Skip to content

Commit 6b5021a

Browse files
lcarteylcartey
committed
Scope: Add getParentScope testing
 - Expose the internal getParentScope for testing. - Add test cases
1 parent 90d4127 commit 6b5021a

File tree

4 files changed

+195
-50
lines changed

4 files changed

+195
-50
lines changed

‎cpp/common/src/codingstandards/cpp/Scope.qll

Lines changed: 57 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -5,54 +5,59 @@
55
import cpp
66

77
/**
8-
* Gets the parent scope of this `Element`, if any.
9-
* A scope is a `Type` (`Class` / `Enum`), a `Namespace`, a `Block`, a `Function`,
10-
* or certain kinds of `Statement`.
11-
* Differs from `Element::getParentScope` when `e` is a `LoopControlVariable`
8+
* Internal module, exposed for testing.
129
*/
13-
private Element getParentScope(Element e) {
14-
/*
15-
* A `Block` cannot have a `ForStmt` as its parent scope so we have to special case
16-
* for loop bodies to ensure that identifiers inside the loop bodies have the for stmt as a parent scope.
17-
* If this is not the case then `i2` in the following example cannot be in `i1`'s potential scope, because
18-
* the parent scope of `i1` is the `ForStmt` while the transitive closure of the parent scope for `i2` doesn't include
19-
* outer scope. Blocks can only have blocks as parent scopes.
20-
* void f() {
21-
* for( int i1; ... ) {
22-
* for (int i2; ...) {
23-
* }
24-
* }
25-
* }
10+
module Internal {
11+
/**
12+
* Gets the parent scope of this `Element`, if any.
13+
* A scope is a `Type` (`Class` / `Enum`), a `Namespace`, a `Block`, a `Function`,
14+
* or certain kinds of `Statement`.
15+
* Differs from `Element::getParentScope` when `e` is a `LoopControlVariable`
2616
*/
27-
28-
exists(Loop loop | loop.getStmt() = e and result = loop)
29-
or
30-
exists(IfStmt ifStmt |
31-
(ifStmt.getThen() = e or ifStmt.getElse() = e) and
32-
result = ifStmt
33-
)
34-
or
35-
exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
36-
or
37-
not result.(Loop).getStmt() = e and
38-
not result.(IfStmt).getThen() = e and
39-
not result.(IfStmt).getElse() = e and
40-
not result.(SwitchStmt).getStmt() = e and
41-
if exists(e.getParentScope())
42-
then result = e.getParentScope()
43-
else (
44-
// Statements do no have a parent scope, so return the enclosing block.
45-
result = e.(Stmt).getEnclosingBlock()
17+
Element getParentScope(Element e) {
18+
/*
19+
* A `Block` cannot have a `ForStmt` as its parent scope so we have to special case
20+
* for loop bodies to ensure that identifiers inside the loop bodies have the for stmt as a parent scope.
21+
* If this is not the case then `i2` in the following example cannot be in `i1`'s potential scope, because
22+
* the parent scope of `i1` is the `ForStmt` while the transitive closure of the parent scope for `i2` doesn't include
23+
* outer scope. Blocks can only have blocks as parent scopes.
24+
* void f() {
25+
* for( int i1; ... ) {
26+
* for (int i2; ...) {
27+
* }
28+
* }
29+
* }
30+
*/
31+
32+
exists(Loop loop | loop.getStmt() = e and result = loop)
33+
or
34+
exists(IfStmt ifStmt |
35+
(ifStmt.getThen() = e or ifStmt.getElse() = e) and
36+
result = ifStmt
37+
)
4638
or
47-
result = e.(Expr).getEnclosingBlock()
39+
exists(SwitchStmt switchStmt | switchStmt.getStmt() = e and result = switchStmt)
4840
or
49-
// Catch block parameters don't have an enclosing scope, so attach them to the
50-
// the block itself
51-
exists(CatchBlock cb |
52-
e = cb.getParameter() and
53-
result = cb
41+
not result.(Loop).getStmt() = e and
42+
not result.(IfStmt).getThen() = e and
43+
not result.(IfStmt).getElse() = e and
44+
not result.(SwitchStmt).getStmt() = e and
45+
if exists(e.getParentScope())
46+
then result = e.getParentScope()
47+
else (
48+
// Statements do no have a parent scope, so return the enclosing block.
49+
result = e.(Stmt).getEnclosingBlock()
50+
or
51+
result = e.(Expr).getEnclosingBlock()
52+
or
53+
// Catch block parameters don't have an enclosing scope, so attach them to the
54+
// the block itself
55+
exists(CatchBlock cb |
56+
e = cb.getParameter() and
57+
result = cb
58+
)
5459
)
55-
)
60+
}
5661
}
5762

5863
/** A variable which is defined by the user, rather than being from a third party or compiler generated. */
@@ -68,19 +73,19 @@ class UserVariable extends Variable {
6873

6974
/** An element which is the parent scope of at least one other element in the program. */
7075
class Scope extends Element {
71-
Scope() { this = getParentScope(_) }
76+
Scope() { this = Internal::getParentScope(_) }
7277

73-
UserVariable getAVariable() { getParentScope(result) = this }
78+
UserVariable getAVariable() { Internal::getParentScope(result) = this }
7479

7580
int getNumberOfVariables() { result = count(getAVariable()) }
7681

7782
Scope getAnAncestor() { result = this.getStrictParent+() }
7883

79-
Scope getStrictParent() { result = getParentScope(this) }
84+
Scope getStrictParent() { result = Internal::getParentScope(this) }
8085

81-
Declaration getADeclaration() { getParentScope(result) = this }
86+
Declaration getADeclaration() { Internal::getParentScope(result) = this }
8287

83-
Expr getAnExpr() { this = getParentScope(result) }
88+
Expr getAnExpr() { this = Internal::getParentScope(result) }
8489

8590
private predicate getLocationInfo(
8691
PreprocessorBranchDirective pbd, string file1, string file2, int startline1, int startline2
@@ -112,9 +117,11 @@ class Scope extends Element {
112117
predicate isGenerated() { this instanceof GeneratedBlockStmt }
113118

114119
int getDepth() {
115-
exists(Scope parent | parent = getParentScope(this) and result = 1 + parent.getDepth())
120+
exists(Scope parent |
121+
parent = Internal::getParentScope(this) and result = 1 + parent.getDepth()
122+
)
116123
or
117-
not exists(getParentScope(this)) and result = 0
124+
not exists(Internal::getParentScope(this)) and result = 0
118125
}
119126
}
120127

‎cpp/common/test/library/codingstandards/cpp/scope/ParentScope.expected

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
| file://:0:0:0:0 | (unnamed parameter 0) | file://:0:0:0:0 | operator= |
2+
| file://:0:0:0:0 | (unnamed parameter 0) | file://:0:0:0:0 | operator= |
3+
| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:8:7:8:7 | operator= |
4+
| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:8:7:8:7 | operator= |
5+
| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | (unnamed constructor) |
6+
| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | (unnamed constructor) |
7+
| file://:0:0:0:0 | (unnamed parameter 0) | test.cpp:27:28:27:28 | operator= |
8+
| file://:0:0:0:0 | __va_list_tag | file://:0:0:0:0 | (global namespace) |
9+
| file://:0:0:0:0 | fp_offset | file://:0:0:0:0 | __va_list_tag |
10+
| file://:0:0:0:0 | gp_offset | file://:0:0:0:0 | __va_list_tag |
11+
| file://:0:0:0:0 | operator= | file://:0:0:0:0 | __va_list_tag |
12+
| file://:0:0:0:0 | operator= | file://:0:0:0:0 | __va_list_tag |
13+
| file://:0:0:0:0 | overflow_arg_area | file://:0:0:0:0 | __va_list_tag |
14+
| file://:0:0:0:0 | reg_save_area | file://:0:0:0:0 | __va_list_tag |
15+
| test.cpp:1:5:1:7 | id1 | file://:0:0:0:0 | (global namespace) |
16+
| test.cpp:3:11:3:13 | ns1 | file://:0:0:0:0 | (global namespace) |
17+
| test.cpp:4:5:4:7 | id1 | test.cpp:3:11:3:13 | ns1 |
18+
| test.cpp:6:11:6:13 | ns1::ns2 | test.cpp:3:11:3:13 | ns1 |
19+
| test.cpp:7:5:7:7 | id1 | test.cpp:6:11:6:13 | ns1::ns2 |
20+
| test.cpp:8:7:8:7 | C1 | test.cpp:8:7:8:8 | C1 |
21+
| test.cpp:8:7:8:7 | operator= | test.cpp:8:7:8:8 | C1 |
22+
| test.cpp:8:7:8:7 | operator= | test.cpp:8:7:8:8 | C1 |
23+
| test.cpp:8:7:8:8 | C1 | test.cpp:6:11:6:13 | ns1::ns2 |
24+
| test.cpp:9:7:9:9 | id1 | test.cpp:8:7:8:8 | C1 |
25+
| test.cpp:10:8:10:17 | test_scope | test.cpp:8:7:8:8 | C1 |
26+
| test.cpp:10:23:10:25 | id1 | test.cpp:10:8:10:17 | test_scope |
27+
| test.cpp:10:28:34:3 | { ... } | test.cpp:10:8:10:17 | test_scope |
28+
| test.cpp:11:5:33:5 | for(...;...;...) ... | test.cpp:10:28:34:3 | { ... } |
29+
| test.cpp:11:10:11:17 | declaration | test.cpp:10:28:34:3 | { ... } |
30+
| test.cpp:11:14:11:16 | id1 | test.cpp:11:5:33:5 | for(...;...;...) ... |
31+
| test.cpp:11:19:11:21 | id1 | test.cpp:10:28:34:3 | { ... } |
32+
| test.cpp:11:19:11:25 | ... < ... | test.cpp:10:28:34:3 | { ... } |
33+
| test.cpp:11:25:11:25 | 1 | test.cpp:10:28:34:3 | { ... } |
34+
| test.cpp:11:28:11:30 | id1 | test.cpp:10:28:34:3 | { ... } |
35+
| test.cpp:11:28:11:32 | ... ++ | test.cpp:10:28:34:3 | { ... } |
36+
| test.cpp:11:35:33:5 | { ... } | test.cpp:10:28:34:3 | { ... } |
37+
| test.cpp:11:35:33:5 | { ... } | test.cpp:11:5:33:5 | for(...;...;...) ... |
38+
| test.cpp:12:7:32:7 | for(...;...;...) ... | test.cpp:11:35:33:5 | { ... } |
39+
| test.cpp:12:12:12:19 | declaration | test.cpp:11:35:33:5 | { ... } |
40+
| test.cpp:12:16:12:18 | id1 | test.cpp:12:7:32:7 | for(...;...;...) ... |
41+
| test.cpp:12:21:12:23 | id1 | test.cpp:11:35:33:5 | { ... } |
42+
| test.cpp:12:21:12:27 | ... < ... | test.cpp:11:35:33:5 | { ... } |
43+
| test.cpp:12:27:12:27 | 1 | test.cpp:11:35:33:5 | { ... } |
44+
| test.cpp:12:30:12:32 | id1 | test.cpp:11:35:33:5 | { ... } |
45+
| test.cpp:12:30:12:34 | ... ++ | test.cpp:11:35:33:5 | { ... } |
46+
| test.cpp:12:37:32:7 | { ... } | test.cpp:11:35:33:5 | { ... } |
47+
| test.cpp:12:37:32:7 | { ... } | test.cpp:12:7:32:7 | for(...;...;...) ... |
48+
| test.cpp:13:9:31:9 | { ... } | test.cpp:12:37:32:7 | { ... } |
49+
| test.cpp:14:11:14:18 | declaration | test.cpp:13:9:31:9 | { ... } |
50+
| test.cpp:14:15:14:17 | id1 | test.cpp:13:9:31:9 | { ... } |
51+
| test.cpp:16:11:20:11 | if (...) ... | test.cpp:13:9:31:9 | { ... } |
52+
| test.cpp:16:15:16:17 | id1 | test.cpp:13:9:31:9 | { ... } |
53+
| test.cpp:16:15:16:22 | ... == ... | test.cpp:13:9:31:9 | { ... } |
54+
| test.cpp:16:22:16:22 | 0 | test.cpp:13:9:31:9 | { ... } |
55+
| test.cpp:16:25:18:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
56+
| test.cpp:16:25:18:11 | { ... } | test.cpp:16:11:20:11 | if (...) ... |
57+
| test.cpp:17:13:17:20 | declaration | test.cpp:16:25:18:11 | { ... } |
58+
| test.cpp:17:17:17:19 | id1 | test.cpp:16:25:18:11 | { ... } |
59+
| test.cpp:18:18:20:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
60+
| test.cpp:18:18:20:11 | { ... } | test.cpp:16:11:20:11 | if (...) ... |
61+
| test.cpp:19:13:19:20 | declaration | test.cpp:18:18:20:11 | { ... } |
62+
| test.cpp:19:17:19:19 | id1 | test.cpp:18:18:20:11 | { ... } |
63+
| test.cpp:21:11:25:11 | switch (...) ... | test.cpp:13:9:31:9 | { ... } |
64+
| test.cpp:21:19:21:21 | id1 | test.cpp:13:9:31:9 | { ... } |
65+
| test.cpp:21:24:25:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
66+
| test.cpp:21:24:25:11 | { ... } | test.cpp:21:11:25:11 | switch (...) ... |
67+
| test.cpp:22:11:22:17 | case ...: | test.cpp:21:24:25:11 | { ... } |
68+
| test.cpp:22:16:22:16 | 0 | test.cpp:21:24:25:11 | { ... } |
69+
| test.cpp:23:13:23:20 | declaration | test.cpp:21:24:25:11 | { ... } |
70+
| test.cpp:23:17:23:19 | id1 | test.cpp:21:24:25:11 | { ... } |
71+
| test.cpp:24:13:24:18 | break; | test.cpp:21:24:25:11 | { ... } |
72+
| test.cpp:25:11:25:11 | label ...: | test.cpp:13:9:31:9 | { ... } |
73+
| test.cpp:26:11:28:11 | try { ... } | test.cpp:13:9:31:9 | { ... } |
74+
| test.cpp:26:15:28:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
75+
| test.cpp:27:13:27:53 | declaration | test.cpp:26:15:28:11 | { ... } |
76+
| test.cpp:27:18:27:24 | lambda1 | test.cpp:26:15:28:11 | { ... } |
77+
| test.cpp:27:27:27:52 | [...](...){...} | test.cpp:26:15:28:11 | { ... } |
78+
| test.cpp:27:27:27:52 | {...} | test.cpp:26:15:28:11 | { ... } |
79+
| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) |
80+
| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) |
81+
| test.cpp:27:28:27:28 | (unnamed constructor) | file://:0:0:0:0 | decltype([...](...){...}) |
82+
| test.cpp:27:28:27:28 | operator= | file://:0:0:0:0 | decltype([...](...){...}) |
83+
| test.cpp:27:29:27:29 | id1 | file://:0:0:0:0 | decltype([...](...){...}) |
84+
| test.cpp:27:29:27:31 | id1 | test.cpp:26:15:28:11 | { ... } |
85+
| test.cpp:27:33:27:33 | operator() | file://:0:0:0:0 | decltype([...](...){...}) |
86+
| test.cpp:27:36:27:52 | { ... } | test.cpp:27:33:27:33 | operator() |
87+
| test.cpp:27:38:27:50 | declaration | test.cpp:27:36:27:52 | { ... } |
88+
| test.cpp:27:42:27:44 | id1 | test.cpp:27:36:27:52 | { ... } |
89+
| test.cpp:27:47:27:49 | 10 | test.cpp:27:36:27:52 | { ... } |
90+
| test.cpp:27:52:27:52 | return ... | test.cpp:27:36:27:52 | { ... } |
91+
| test.cpp:28:24:28:26 | id1 | test.cpp:28:29:30:11 | { ... } |
92+
| test.cpp:28:29:30:11 | <handler> | test.cpp:13:9:31:9 | { ... } |
93+
| test.cpp:28:29:30:11 | { ... } | test.cpp:13:9:31:9 | { ... } |
94+
| test.cpp:29:13:29:20 | declaration | test.cpp:28:29:30:11 | { ... } |
95+
| test.cpp:29:17:29:19 | id1 | test.cpp:28:29:30:11 | { ... } |
96+
| test.cpp:34:3:34:3 | return ... | test.cpp:10:28:34:3 | { ... } |

‎cpp/common/test/library/codingstandards/cpp/scope/ParentScope.ql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import codingstandards.cpp.Scope
2+
3+
from Element e, Element parent
4+
where Internal::getParentScope(e) = parent
5+
select e, parent

‎cpp/common/test/library/codingstandards/cpp/scope/test.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
int id1;
2+
3+
namespace ns1 {
4+
int id1; // COMPLIANT
5+
6+
namespace ns2 {
7+
int id1; // COMPLIANT
8+
class C1 {
9+
int id1;
10+
void test_scope(int id1) {
11+
for (int id1; id1 < 1; id1++) {
12+
for (int id1; id1 < 1; id1++) {
13+
{
14+
int id1;
15+
16+
if (id1 == 0) {
17+
int id1;
18+
} else {
19+
int id1;
20+
}
21+
switch (id1) {
22+
case 0:
23+
int id1;
24+
break;
25+
}
26+
try {
27+
auto lambda1 = [id1]() { int id1 = 10; };
28+
} catch (int id1) {
29+
int id1;
30+
}
31+
}
32+
}
33+
}
34+
}
35+
};
36+
} // namespace ns2
37+
} // namespace ns1

0 commit comments

Comments
 (0)