Skip to content

Commit ad71fd3

Browse files
Exclude templates, improve performance
1 parent 5d8e899 commit ad71fd3

File tree

7 files changed

+235
-60
lines changed

7 files changed

+235
-60
lines changed

change_notes/2026-02-18-unused-type-improvements.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@
33
- 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.
44
- Usages of a template type inside a specialization of that template are no longer considered usages of the template type.
55
- 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.
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.

cpp/common/src/codingstandards/cpp/ast/Class.qll

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,19 @@ class LastClassDeclaration extends Declaration {
1010

1111
pragma[nomagic]
1212
LastClassDeclaration() {
13-
this =
14-
max(Declaration decl, Location l |
15-
decl = cls.getADeclaration() and
16-
l = decl.getLocation()
17-
|
18-
decl order by l.getEndLine(), l.getEndColumn()
19-
)
13+
cls.getADeclaration() = this and
14+
getLocation().getEndLine() = getLastLineOfClassDeclaration(cls)
2015
}
2116
}
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+
}

cpp/common/src/codingstandards/cpp/ast/HiddenFriend.qll

Lines changed: 75 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@
1717
import cpp
1818
import codingstandards.cpp.ast.Class
1919

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+
*/
2027
class PossibleHiddenFriend extends HiddenFriendCandidate {
2128
ClassCandidate cls;
2229

@@ -47,53 +54,76 @@ private class FileCandidate extends File {
4754
private class ClassCandidate extends Class {
4855
ClassCandidate() { getFile() instanceof FileCandidate }
4956

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+
*/
5063
Declaration getNextSiblingDeclaration() {
51-
exists(LastClassDeclaration last |
52-
last.getDeclaringType() = this and
53-
result =
54-
min(Declaration decl |
55-
decl.getEnclosingElement() = this.getEnclosingElement() and
56-
pragma[only_bind_out](decl.getFile()) = pragma[only_bind_out](this.getFile()) and
57-
decl.getLocation().getStartLine() > last.getLocation().getEndLine()
58-
|
59-
decl order by decl.getLocation().getStartLine(), decl.getLocation().getStartColumn()
60-
)
61-
)
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+
)
6272
}
6373

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+
*/
6483
Declaration getNextOrphanedDeclaration() {
65-
exists(LastClassDeclaration last |
66-
last.getDeclaringType() = this and
67-
result =
68-
min(OrphanedDeclaration decl, Location locLast, Location locDecl |
69-
orphanHasFile(decl, this.getFile()) and
70-
locDecl = decl.getLocation() and
71-
locLast = last.getLocation() and
72-
locLast.getStartLine() < locDecl.getEndLine()
73-
|
74-
decl order by locDecl.getStartLine(), locDecl.getStartColumn()
75-
)
76-
)
84+
result =
85+
min(OrphanedDeclaration decl, int startLine, int startColumn | // Location locDecl | // Location locLast, Location locDecl |
86+
orphanHasLocation(decl, this.getFile(), startLine, startColumn) and
87+
startLine > getLastLineOfClassDeclaration(this)
88+
|
89+
decl order by startLine, startColumn
90+
)
7791
}
7892

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+
*/
7997
Declaration getFirstNonClassDeclaration() {
80-
exists(LastClassDeclaration last |
81-
last.getDeclaringType() = this and
82-
result =
83-
min(Declaration decl |
84-
decl = getNextSiblingDeclaration() or decl = getNextOrphanedDeclaration()
85-
|
86-
decl order by decl.getLocation().getStartLine(), decl.getLocation().getStartColumn()
87-
)
88-
)
98+
result =
99+
min(Declaration decl |
100+
decl = getNextSiblingDeclaration() or decl = getNextOrphanedDeclaration()
101+
|
102+
decl order by decl.getLocation().getStartLine(), decl.getLocation().getStartColumn()
103+
)
89104
}
90105
}
91106

107+
/**
108+
* Helper predicate to improve join performance.
109+
*/
92110
pragma[nomagic]
93-
private predicate orphanHasFile(OrphanedDeclaration orphan, FileCandidate file) {
94-
orphan.getFile() = file
111+
private predicate orphanHasLocation(
112+
OrphanedDeclaration orphan, FileCandidate file, int endLine, int endColumn
113+
) {
114+
orphan.getFile() = file and
115+
orphan.getLocation().getEndLine() = endLine and
116+
orphan.getLocation().getEndColumn() = endColumn
95117
}
96118

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+
*/
97127
private class OrphanedDeclaration extends Declaration {
98128
OrphanedDeclaration() {
99129
not exists(getEnclosingElement()) and
@@ -103,14 +133,25 @@ private class OrphanedDeclaration extends Declaration {
103133
}
104134
}
105135

136+
/**
137+
* Helper predicate to improve join performance.
138+
*/
106139
pragma[nomagic]
107140
private predicate classCandidateHasFile(ClassCandidate c, FileCandidate f) { c.getFile() = f }
108141

142+
/**
143+
* Helper predicate to improve join performance.
144+
*/
109145
pragma[nomagic]
110146
private predicate hiddenFriendCandidateHasFile(HiddenFriendCandidate h, FileCandidate f) {
111147
h.getFile() = f
112148
}
113149

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+
*/
114155
pragma[nomagic]
115156
private predicate hidesFriend(ClassCandidate c, HiddenFriendCandidate f) {
116157
exists(FileCandidate file, Location cloc, Location floc |

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,6 @@ predicate isWithinTypeDefinition(Locatable loc, Type type) {
8989
tpl = spec.getPrimaryTemplate() and
9090
isWithinTypeDefinition(loc, spec)
9191
)
92-
//exists(TemplateClass tpl, ClassTemplateSpecialization spec |
93-
// tpl.getAnInstantiation() = type and
94-
// tpl = spec.getPrimaryTemplate() and
95-
// isWithinTypeDefinition(loc, spec)
96-
//)
97-
//exists(ClassTemplateSpecialization spec |
98-
// specializationSharesType(type, spec) and
99-
// isWithinTypeDefinition(loc, spec)
100-
//)
10192
}
10293

10394
private Locatable getATypeUse_i(Type type, string reason) {
@@ -181,6 +172,10 @@ private Locatable getATypeUse_i(Type type, string reason) {
181172
// Temporary object creation of type `type`
182173
exists(TemporaryObjectExpr toe | result = toe | type = toe.getType()) and
183174
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"
184179
)
185180
or
186181
// Recursive case - used by a used type

cpp/misra/src/rules/RULE-0-2-3/UnusedTypeWithLimitedVisibility.ql

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,31 @@ import cpp
1818
import codingstandards.cpp.misra
1919
import codingstandards.cpp.types.Uses
2020

21+
class UninstantiatedTemplate extends Element {
22+
UninstantiatedTemplate() {
23+
this instanceof TemplateClass and
24+
not exists(this.(TemplateClass).getAnInstantiation())
25+
or
26+
this instanceof TemplateFunction and
27+
not exists(this.(TemplateFunction).getAnInstantiation())
28+
}
29+
30+
predicate canAccessNamespace(Namespace n) {
31+
n = this.(Declaration).getNamespace().getParentNamespace*()
32+
}
33+
}
34+
35+
predicate isExcludedNamespaceDueToTemplate(Namespace n) {
36+
exists(UninstantiatedTemplate t | t.canAccessNamespace(n))
37+
}
38+
39+
predicate maybeUsedInUninstantiatedTemplate(UserType type) {
40+
isExcludedNamespaceDueToTemplate(type.getNamespace())
41+
}
42+
2143
predicate hasLimitedVisibility(UserType type) {
2244
type.isLocal() or
23-
type.getNamespace().isAnonymous()
45+
type.getNamespace().getParentNamespace*().isAnonymous()
2446
}
2547

2648
predicate hasMaybeUnusedAttribute(UserType type) {
@@ -33,6 +55,7 @@ where
3355
hasLimitedVisibility(type) and
3456
not exists(getATypeUse(type)) and
3557
not hasMaybeUnusedAttribute(type) and
58+
not maybeUsedInUninstantiatedTemplate(type) and
3659
not type instanceof TypeTemplateParameter and
3760
not type instanceof ClassTemplateSpecialization
3861
select type, "Type '" + type.getName() + "' with limited visibility is not used."
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// Test types used in templates.
2+
//
3+
// The rule is clear that even a usage in an uninstantiated template should be
4+
// counted. However, we don't have enough information from the CodeQL extractor
5+
// to do this precisely, so we err on the side of false negatives.
6+
//
7+
// The only API to find these type references in uninstantiated templates
8+
// directly is `TypeMention`, which is too disassociated from the AST to easily
9+
// determine whether than mention occurs in or outside of the definition of
10+
// the type itself, inside a template, etc.
11+
namespace {
12+
13+
// The following code is too hard to read without namespace indendation.
14+
// clang-format off
15+
16+
/**
17+
* Test case 1: A type is used inside an instantiated template
18+
*/
19+
namespace instantiated_use {
20+
struct A1 {}; // COMPLIANT
21+
22+
template <typename T> // COMPLIANT
23+
void foo(A1 l1) {}
24+
25+
void f1() { foo<int>(A1{}); }
26+
} // namespace instantiated_use
27+
28+
/**
29+
* Test case 2: A type is used inside an uninstantiated template
30+
*/
31+
namespace uninstantiated_use {
32+
struct A1 {}; // COMPLIANT
33+
34+
template <typename T> // COMPLIANT
35+
void foo(A1 l1) {}
36+
} // namespace uninstantiated_use
37+
38+
/**
39+
* Test case 3: A type is not used by a relevant template that is instantiated
40+
*/
41+
namespace instantiated_unused {
42+
struct A1 {}; // NON_COMPLIANT
43+
44+
template <typename T> // COMPLIANT
45+
void foo(int l1) {}
46+
47+
void f1() { foo<int>(42); }
48+
} // namespace instantiated_unused
49+
50+
/**
51+
* Test case 4: A type that is not used by a relevant template that is not
52+
* instantiated
53+
*/
54+
namespace uninstantiated_unused {
55+
struct A1 {}; // NON_COMPLIANT[False Negative]
56+
57+
template <typename T> // COMPLIANT
58+
void foo(int l1) {}
59+
} // namespace uninstantiated_unused
60+
61+
/**
62+
* Test case 5: A type that is used to instantiate a template
63+
*/
64+
namespace use_by_instantiation {
65+
struct A1 {}; // COMPLIANT
66+
67+
template <typename T> // COMPLIANT
68+
void foo() {
69+
T l1;
70+
}
71+
72+
void f1() { foo<A1>(); }
73+
} // namespace use_by_instantiation
74+
75+
/**
76+
* Test case 6: A type that is used as the type of a template parameter
77+
*/
78+
namespace use_as_template_parameter_type {
79+
enum A1 { a };
80+
81+
template <A1 a = A1::a> void foo() {} // COMPLIANT
82+
83+
// Instantiate the template -- if it isn't instantiated, the alert for A1 is
84+
// suppressed regardless of template parameter type usage
85+
void f1() { foo(); }
86+
} // namespace use_as_template_parameter_type
87+
88+
/**
89+
* Test case 6: A type that is used as the default value of a non-type template
90+
* parameter
91+
*/
92+
namespace use_as_template_parameter_default_value {
93+
class A1 { // COMPLIANT[False positive]
94+
static const int a = 42;
95+
};
96+
97+
// The only API to find this in the database is `TypeMention`, which is too
98+
// disassociated from the AST to easily determine if it is used in or
99+
// outside of the definition of `A1`.
100+
template <int a = A1::a> void foo() {}
101+
102+
// Instantiate the template -- if it isn't instantiated, the alert for A1 is
103+
// suppressed regardless of template parameter default value
104+
void f1() { foo(); }
105+
} // namespace use_as_template_parameter_default_value
106+
107+
} // namespace

0 commit comments

Comments
 (0)