Skip to content

Commit 82a8ce3

Browse files
authored
Merge pull request #1043 from github/michaelrfairhurst/package-deadcode-9-rule-0-2-3
Michaelrfairhurst/package deadcode 9 rule 0-2-3
2 parents 86caf28 + f66d898 commit 82a8ce3

File tree

17 files changed

+707
-4
lines changed

17 files changed

+707
-4
lines changed

c/common/test/rules/unusedtypedeclarations/UnusedTypeDeclarations.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22
| test.c:7:18:7:18 | D | Type declaration D is not used. |
33
| test.c:28:11:28:11 | R | Type declaration R is not used. |
44
| test.c:41:12:41:12 | (unnamed class/struct/union) | Type declaration (unnamed class/struct/union) is not used. |
5+
| test.c:56:3:56:4 | T2 | Type declaration T2 is not used. |

c/common/test/rules/unusedtypedeclarations/test.c

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,13 @@ void test_nested_struct() {
4949
s.f1;
5050
s.f3;
5151
s.f5;
52-
}
52+
}
53+
54+
typedef struct {
55+
int m1;
56+
} T2; // NON_COMPLIANT
57+
typedef struct {
58+
int m1;
59+
} T1; // COMPLIANT - used in function below
60+
61+
void test_typedef() { T1 t1; }
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
- `RULE-2-3`, `A0-1-6` - `UnusedTypeDeclarations.ql`:
2+
- Type usage analysis has been improved to find more possible type usages, including:
3+
- Previous behavior considered anonymous types in variable declarations to be considered used by the variable definition itself. This has been improved to require that a field of the anonymous type is accessed for the type to be considered used.
4+
- Usages of a template type inside a specialization of that template are no longer considered usages of the template type.
5+
- Hidden friend declarations are no longer considered usages of the class they are declaring friendship for.
6+
- Improved exclusions generally, for cases such as nested types and functions within functions. These previously were a source of incorrectly identified type uses.
7+
- Additional case added to detect `template <Enum = Enum::Value>` as a usage of `Enum`, without an explicit `tpl<Enum::Value>` usage.
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import cpp
2+
3+
/**
4+
* The last declaration in a class by location order.
5+
*
6+
* This may fail to capture certain cases such as hidden friends (see HiddenFriend.qll).
7+
*/
8+
class LastClassDeclaration extends Declaration {
9+
Class cls;
10+
11+
pragma[nomagic]
12+
LastClassDeclaration() {
13+
cls.getADeclaration() = this and
14+
getLocation().getEndLine() = getLastLineOfClassDeclaration(cls)
15+
}
16+
}
17+
18+
/**
19+
* Gets the line number of the last line of the declaration of `cls`.
20+
*
21+
* This is often more performant to use than `LastClassDeclaration.getLocation().getEndLine()`.
22+
*/
23+
int getLastLineOfClassDeclaration(Class cls) {
24+
result =
25+
max(int endLine |
26+
endLine = pragma[only_bind_out](cls).getADeclaration().getLocation().getEndLine()
27+
)
28+
}
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
/**
2+
* The C++ extractor does not support hidden friends, which are friend functions defined within a
3+
* class, rather than declared:
4+
*
5+
* ```cpp
6+
* struct A {
7+
* friend void hidden_friend(A) {} // Definition: this is a hidden friend
8+
* friend void not_hidden_friend(A); // Declaration: this is not a hidden friend
9+
* };
10+
* ```
11+
*
12+
* In the database, a `FriendDecl` is not created for the hidden friend. The hidden friend function
13+
* is created as a `TopLevel` function with no enclosing element. However, we can identify it as a
14+
* hidden friend by its location.
15+
*/
16+
17+
import cpp
18+
import codingstandards.cpp.ast.Class
19+
20+
/**
21+
* A class that, by our best logic, appears to possibly be a hidden friend.
22+
*
23+
* Hidden friends are not directly represented in the database. Instances of this class have been
24+
* found to have a location "within" the "body" of a class, and to satisfy other criteria that
25+
* indicates it may be a hidden friend.
26+
*/
27+
class PossibleHiddenFriend extends HiddenFriendCandidate {
28+
ClassCandidate cls;
29+
30+
PossibleHiddenFriend() { hidesFriend(cls, this) }
31+
32+
Class getFriendClass() { result = cls }
33+
}
34+
35+
/**
36+
* Begin by limiting the number of candidate functions to consider.
37+
*
38+
* Only inline top level functions can be hidden friends.
39+
*/
40+
private class HiddenFriendCandidate extends TopLevelFunction {
41+
HiddenFriendCandidate() { this.isInline() }
42+
}
43+
44+
/**
45+
* Only consider files which contain hidden friend candidates.
46+
*/
47+
private class FileCandidate extends File {
48+
FileCandidate() { exists(HiddenFriendCandidate c | c.getFile() = this) }
49+
}
50+
51+
/**
52+
* Only consider classes in candidate files, that include hidden friend candidates.
53+
*/
54+
private class ClassCandidate extends Class {
55+
ClassCandidate() { getFile() instanceof FileCandidate }
56+
57+
/**
58+
* Find the next declaration after this class that shares an enclosing element.
59+
*
60+
* This may be the next declaration after this class, or `getNextOrphanedDeclaration` may find the
61+
* true next declaration after this class. These are split for performance reasons.
62+
*/
63+
Declaration getNextSiblingDeclaration() {
64+
result =
65+
min(Declaration decl |
66+
decl.getEnclosingElement() = this.getEnclosingElement() and
67+
pragma[only_bind_out](decl.getFile()) = pragma[only_bind_out](this.getFile()) and
68+
decl.getLocation().getStartLine() > getLastLineOfClassDeclaration(this)
69+
|
70+
decl order by decl.getLocation().getStartLine(), decl.getLocation().getStartColumn()
71+
)
72+
}
73+
74+
/**
75+
* Get the next declaration after this class that does not have an enclosing element.
76+
*
77+
* This may be the next declaration after this class, or `getNextSiblingDeclaration` may find the
78+
* true next declaration after this class. These are split for performance reasons.
79+
*
80+
* Note that `OrphanedDeclaration` excludes hidden friend candidates, so this will find the next
81+
* orphan that is definitely not a hidden friend.
82+
*/
83+
Declaration getNextOrphanedDeclaration() {
84+
result =
85+
min(OrphanedDeclaration decl, int startLine, int startColumn |
86+
orphanHasLocation(decl, this.getFile(), startLine, startColumn) and
87+
startLine > getLastLineOfClassDeclaration(this)
88+
|
89+
decl order by startLine, startColumn
90+
)
91+
}
92+
93+
/**
94+
* Get the first declaration definitely after this class, and not a hidden friend declaration, to
95+
* determine the "end" location of this class.
96+
*/
97+
Declaration getFirstNonClassDeclaration() {
98+
result =
99+
min(Declaration decl |
100+
decl = getNextSiblingDeclaration() or decl = getNextOrphanedDeclaration()
101+
|
102+
decl order by decl.getLocation().getStartLine(), decl.getLocation().getStartColumn()
103+
)
104+
}
105+
}
106+
107+
/**
108+
* Helper predicate to improve join performance.
109+
*/
110+
pragma[nomagic]
111+
private predicate orphanHasLocation(
112+
OrphanedDeclaration orphan, FileCandidate file, int startLine, int startColumn
113+
) {
114+
orphan.getFile() = file and
115+
orphan.getLocation().getEndLine() = startLine and
116+
orphan.getLocation().getEndColumn() = startColumn
117+
}
118+
119+
/**
120+
* Orphaned declarations to be found by `getNextOrphanedDeclaration`.
121+
*
122+
* These are declarations with no enclosing element. Note that we exclude hidden friend candidates,
123+
* as this class is used to find the declarations that are definitely not part of some class. This
124+
* is done so we can detect if hidden friends may be within that class definition. Therefore we must
125+
* exclude hidden friend candidates, even though those are also orphaned.
126+
*/
127+
private class OrphanedDeclaration extends Declaration {
128+
OrphanedDeclaration() {
129+
not exists(getEnclosingElement()) and
130+
not this instanceof HiddenFriendCandidate and
131+
getFile() instanceof FileCandidate and
132+
not isFromTemplateInstantiation(_)
133+
}
134+
}
135+
136+
/**
137+
* Helper predicate to improve join performance.
138+
*/
139+
pragma[nomagic]
140+
private predicate classCandidateHasFile(ClassCandidate c, FileCandidate f) { c.getFile() = f }
141+
142+
/**
143+
* Helper predicate to improve join performance.
144+
*/
145+
pragma[nomagic]
146+
private predicate hiddenFriendCandidateHasFile(HiddenFriendCandidate h, FileCandidate f) {
147+
h.getFile() = f
148+
}
149+
150+
/**
151+
* Find the class locations that have declarations that could be hidden friend declarations, by
152+
* comparing the locations of the candidate hidden friend functions to the location of the first
153+
* declaration that clearly is outside that class.
154+
*/
155+
pragma[nomagic]
156+
private predicate hidesFriend(ClassCandidate c, HiddenFriendCandidate f) {
157+
exists(FileCandidate file, Location cloc, Location floc |
158+
classCandidateHasFile(c, file) and
159+
hiddenFriendCandidateHasFile(f, file) and
160+
cloc = c.getLocation() and
161+
floc = f.getLocation() and
162+
cloc.getEndLine() < floc.getStartLine() and
163+
floc.getEndLine() < c.getFirstNonClassDeclaration().getLocation().getStartLine()
164+
)
165+
}
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype DeadCode9Query = TUnusedTypeWithLimitedVisibilityQuery()
7+
8+
predicate isDeadCode9QueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `unusedTypeWithLimitedVisibility` query
11+
DeadCode9Package::unusedTypeWithLimitedVisibilityQuery() and
12+
queryId =
13+
// `@id` for the `unusedTypeWithLimitedVisibility` query
14+
"cpp/misra/unused-type-with-limited-visibility" and
15+
ruleId = "RULE-0-2-3" and
16+
category = "advisory"
17+
}
18+
19+
module DeadCode9Package {
20+
Query unusedTypeWithLimitedVisibilityQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `unusedTypeWithLimitedVisibility` query
24+
TQueryCPP(TDeadCode9PackageQuery(TUnusedTypeWithLimitedVisibilityQuery()))
25+
}
26+
}

cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import DeadCode5
2525
import DeadCode6
2626
import DeadCode7
2727
import DeadCode8
28+
import DeadCode9
2829
import Declarations
2930
import ExceptionSafety
3031
import Exceptions1
@@ -105,6 +106,7 @@ newtype TCPPQuery =
105106
TDeadCode6PackageQuery(DeadCode6Query q) or
106107
TDeadCode7PackageQuery(DeadCode7Query q) or
107108
TDeadCode8PackageQuery(DeadCode8Query q) or
109+
TDeadCode9PackageQuery(DeadCode9Query q) or
108110
TDeclarationsPackageQuery(DeclarationsQuery q) or
109111
TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or
110112
TExceptions1PackageQuery(Exceptions1Query q) or
@@ -185,6 +187,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat
185187
isDeadCode6QueryMetadata(query, queryId, ruleId, category) or
186188
isDeadCode7QueryMetadata(query, queryId, ruleId, category) or
187189
isDeadCode8QueryMetadata(query, queryId, ruleId, category) or
190+
isDeadCode9QueryMetadata(query, queryId, ruleId, category) or
188191
isDeclarationsQueryMetadata(query, queryId, ruleId, category) or
189192
isExceptionSafetyQueryMetadata(query, queryId, ruleId, category) or
190193
isExceptions1QueryMetadata(query, queryId, ruleId, category) or

cpp/common/src/codingstandards/cpp/types/Uses.qll

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import cpp
15+
import codingstandards.cpp.ast.HiddenFriend
1516

1617
/**
1718
* Gets a typedef with the same qualified name and declared at the same location.
@@ -35,7 +36,8 @@ private TypedefType getAnEquivalentTypeDef(TypedefType type) {
3536
* is from within the function signature or field declaration of the type itself.
3637
*/
3738
Locatable getATypeUse(Type type) {
38-
result = getATypeUse_i(type, _)
39+
result = getATypeUse_i(type, _) and
40+
not isWithinTypeDefinition(result, type)
3941
or
4042
// Identify `TypeMention`s of typedef types, where the underlying type is used.
4143
//
@@ -65,6 +67,30 @@ Locatable getATypeUse(Type type) {
6567
)
6668
}
6769

70+
predicate isWithinTypeDefinition(Locatable loc, Type type) {
71+
loc = type
72+
or
73+
loc.getEnclosingElement*() = type
74+
or
75+
isWithinTypeDefinition(loc.(Function).getDeclaringType(), type)
76+
or
77+
isWithinTypeDefinition(loc.(MemberVariable).getDeclaringType(), type)
78+
or
79+
isWithinTypeDefinition(loc.(PossibleHiddenFriend).getFriendClass(), type)
80+
or
81+
isWithinTypeDefinition(loc.(Parameter).getFunction(), type)
82+
or
83+
isWithinTypeDefinition(loc.(Expr).getEnclosingFunction(), type)
84+
or
85+
isWithinTypeDefinition(loc.(LocalVariable).getFunction(), type)
86+
or
87+
exists(TemplateClass tpl, ClassTemplateSpecialization spec |
88+
tpl = type and
89+
tpl = spec.getPrimaryTemplate() and
90+
isWithinTypeDefinition(loc, spec)
91+
)
92+
}
93+
6894
private Locatable getATypeUse_i(Type type, string reason) {
6995
(
7096
// Restrict to uses within the source checkout root
@@ -81,6 +107,7 @@ private Locatable getATypeUse_i(Type type, string reason) {
81107
type = v.getType() and
82108
// Ignore self referential variables and parameters
83109
not v.getDeclaringType().refersTo(type) and
110+
not type.(UserType).isAnonymous() and
84111
not type = v.(Parameter).getFunction().getDeclaringType()
85112
) and
86113
reason = "used as a variable type"
@@ -145,6 +172,10 @@ private Locatable getATypeUse_i(Type type, string reason) {
145172
// Temporary object creation of type `type`
146173
exists(TemporaryObjectExpr toe | result = toe | type = toe.getType()) and
147174
reason = "used in temporary object expr"
175+
or
176+
// template<Type t> ...
177+
exists(Declaration decl | result = decl | type = decl.getATemplateArgumentKind()) and
178+
reason = "used as a non-type template parameter"
148179
)
149180
or
150181
// Recursive case - used by a used type

cpp/common/test/rules/unusedtypedeclarations/UnusedTypeDeclarations.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,7 @@
44
| test.cpp:77:11:77:11 | R | Type declaration R is not used. |
55
| test.cpp:90:12:90:12 | (unnamed class/struct/union) | Type declaration (unnamed class/struct/union) is not used. |
66
| test.cpp:111:29:111:30 | AA | Type declaration AA is not used. |
7+
| test.cpp:126:7:126:12 | Nested | Type declaration Nested is not used. |
8+
| test.cpp:135:9:135:20 | UnusedNested | Type declaration UnusedNested is not used. |
9+
| test.cpp:138:7:138:22 | NestedBlockScope | Type declaration NestedBlockScope is not used. |
10+
| test.cpp:149:11:149:16 | Unused | Type declaration Unused is not used. |

cpp/common/test/rules/unusedtypedeclarations/test.cpp

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,4 +121,31 @@ void test_temporary_object_creation() {
121121

122122
return C1{p1};
123123
};
124-
}
124+
}
125+
126+
class Nested { // NON_COMPLIANT - only used within itself
127+
public:
128+
class NestedNested { // COMPLIANT - used by class `Nested`
129+
public:
130+
Nested f();
131+
};
132+
133+
NestedNested g();
134+
135+
class UnusedNested {}; // NON_COMPLIANT - never used by class `Nested`
136+
};
137+
138+
class NestedBlockScope { // NON_COMPLIANT - only used within itself
139+
public:
140+
void f() {
141+
class NestedFunction { // COMPLIANT - used in function below
142+
void g() {
143+
NestedBlockScope l1; // Doesn't count as use of `NestedBlockScope`.
144+
}
145+
};
146+
147+
NestedFunction l1;
148+
149+
class Unused {}; // NON_COMPLIANT - never used by function `f`
150+
}
151+
};

0 commit comments

Comments
 (0)