Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 5a13000

Browse filesBrowse files
lcarteymbaluda
lcartey
and
mbaluda
authored
A5-2-2: Exclude results in uninstantiated templates, explain limitations (#160)
* A5-2-2: Clarify c-style casts scope, exclude templates Clarify what `CStyleCast` does and does not cover by adding a comment, expanding the test case and providing an implementation scope. In addition, exclude casts on template parameters to avoid unnecessary false positives. * A5-2-2: Exclude uninstantiated templates Any cast in an uninstantiated template that is related to the template parameter may be converted to a `ConstructorCall` when the template is instantiated. To avoid the common false positive case where the functional cast notation is used to call a constructor, we exclude all results in uninstantiated templates and instead rely on reporting results in template instantiations instead. --------- Co-authored-by: Mauro Baluda <mbaluda@github.com>
1 parent ea27dfa commit 5a13000
Copy full SHA for 5a13000

File tree

7 files changed

+119
-3
lines changed
Filter options

7 files changed

+119
-3
lines changed

‎change_notes/2023-01-09-cstylecasts-template-parameters.md

Copy file name to clipboard
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- `A5-2-2`
2+
- `CStyleCasts.ql` - exclude template parameters to avoid false positives when using the "functional notation" syntax. In addition, provide a greater explanation on limitations of this query.

‎cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql

Copy file name to clipboardExpand all lines: cpp/autosar/src/rules/A5-2-2/TraditionalCStyleCastsUsed.ql
+32Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,11 +44,43 @@ class LibraryMacro extends Macro {
4444
LibraryMacro() { not this instanceof UserProvidedMacro }
4545
}
4646

47+
/*
48+
* In theory this query should exclude casts using the "functional notation" syntax, e.g.
49+
* ```
50+
* int(x);
51+
* ```
52+
* This is because this is not a C-style cast, as it is not legitimate C code. However, our database
53+
* schema does not distinguish between C-style casts and functional casts, so we cannot exclude just
54+
* those.
55+
*
56+
* In addition, we do not get `CStyleCasts` in cases where the cast is converted to a `ConstructorCall`.
57+
* This holds for both the "functional notation" syntax and the "c-style" syntax, e.g. both of these
58+
* are represented in our model as `ConstructorCall`s only:
59+
* ```
60+
* class A { public: A(int); };
61+
* void create() {
62+
* (A)1;
63+
* A(1);
64+
* }
65+
* ```
66+
*
67+
* As a consequence this query:
68+
* - Produces false positives when primitive types are cast using the "functional notation" syntax.
69+
* - Produces false negatives when a C-style cast is converted to a `ConstructorCall` e.g. when the
70+
* argument type is compatible with a single-argument constructor.
71+
*/
72+
4773
from CStyleCast c, string extraMessage, Locatable l, string supplementary
4874
where
4975
not isExcluded(c, BannedSyntaxPackage::traditionalCStyleCastsUsedQuery()) and
5076
not c.isImplicit() and
5177
not c.getType() instanceof UnknownType and
78+
// For casts in templates that occur on types related to a template parameter, the copy of th
79+
// cast in the uninstantiated template is represented as a `CStyleCast` even if in practice all
80+
// the instantiations represent it as a `ConstructorCall`. To avoid the common false positive case
81+
// of using the functional cast notation to call a constructor we exclude all `CStyleCast`s on
82+
// uninstantiated templates, and instead rely on reporting results within instantiations.
83+
not c.isFromUninstantiatedTemplate(_) and
5284
// Exclude casts created from macro invocations of macros defined by third parties
5385
not getGeneratedFrom(c) instanceof LibraryMacro and
5486
// If the cast was generated from a user-provided macro, then report the macro that generated the

‎cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected

Copy file name to clipboardExpand all lines: cpp/autosar/test/rules/A5-2-2/TraditionalCStyleCastsUsed.expected
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,5 @@
55
| test.cpp:79:3:79:18 | (int)... | Use of explicit c-style cast to int generated from macro $@. | test.cpp:71:1:71:36 | #define NESTED_ADD_ONE(x) ADD_ONE(x) | NESTED_ADD_ONE |
66
| test.cpp:85:19:85:26 | (int)... | Use of explicit c-style cast to int. | test.cpp:85:19:85:26 | (int)... | |
77
| test.cpp:86:27:86:34 | (int)... | Use of explicit c-style cast to int. | test.cpp:86:27:86:34 | (int)... | |
8+
| test.cpp:114:10:114:13 | (int)... | Use of explicit c-style cast to int. | test.cpp:114:10:114:13 | (int)... | |
9+
| test.cpp:149:12:149:26 | (unsigned int)... | Use of explicit c-style cast to unsigned int. | test.cpp:149:12:149:26 | (unsigned int)... | |

‎cpp/autosar/test/rules/A5-2-2/options.clang

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-I../../../../common/test/includes/custom-library

‎cpp/autosar/test/rules/A5-2-2/options.gcc

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-I../../../../common/test/includes/custom-library

‎cpp/autosar/test/rules/A5-2-2/test.cpp

Copy file name to clipboardExpand all lines: cpp/autosar/test/rules/A5-2-2/test.cpp
+73-2Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ int foo() { return 1; }
66
void test_c_style_cast() {
77
double f = 3.14;
88
std::uint32_t n1 = (std::uint32_t)f; // NON_COMPLIANT - C-style cast
9-
std::uint32_t n2 = unsigned(f); // NON_COMPLIANT - functional cast
9+
std::uint32_t n2 = unsigned(f); // COMPLIANT[FALSE_POSITIVE]
1010

1111
std::uint8_t n3 = 1;
1212
std::uint8_t n4 = 1;
@@ -86,4 +86,75 @@ void test_macro_cast() {
8686
LIBRARY_NO_CAST_ADD_TWO((int)1.0); // NON_COMPLIANT - library macro with
8787
// c-style cast in argument, written by
8888
// user so should be reported
89-
}
89+
}
90+
91+
class D {
92+
public:
93+
D(int x) : fx(x), fy(0) {}
94+
D(int x, int y) : fx(x), fy(y) {}
95+
96+
private:
97+
int fx;
98+
int fy;
99+
};
100+
101+
D testNonFunctionalCast() {
102+
return (D)1; // NON_COMPLIANT[FALSE_NEGATIVE]
103+
}
104+
105+
D testFunctionalCast() {
106+
return D(1); // COMPLIANT
107+
}
108+
109+
D testFunctionalCastMulti() {
110+
return D(1, 2); // COMPLIANT
111+
}
112+
113+
template <typename T> T testFunctionalCastTemplate() {
114+
return T(1); // COMPLIANT[FALSE_POSITIVE]
115+
}
116+
117+
template <typename T> T testFunctionalCastTemplateMulti() {
118+
return T(1, 2); // COMPLIANT
119+
}
120+
121+
void testFunctionalCastTemplateUse() {
122+
testFunctionalCastTemplate<D>();
123+
testFunctionalCastTemplate<int>();
124+
testFunctionalCastTemplateMulti<D>();
125+
}
126+
127+
template <typename T> class E {
128+
public:
129+
class F {
130+
public:
131+
F(int x) : fx(x), fy(0) {}
132+
F(int x, int y) : fx(x), fy(y) {}
133+
134+
private:
135+
int fx;
136+
int fy;
137+
};
138+
139+
F f() {
140+
return F(1); // COMPLIANT
141+
}
142+
143+
D d() {
144+
return D(1); // COMPLIANT
145+
}
146+
147+
int i() {
148+
double f = 3.14;
149+
return (unsigned int)f; // NON_COMPLIANT
150+
}
151+
};
152+
153+
class G {};
154+
155+
void testE() {
156+
E<G> e;
157+
e.f();
158+
e.d();
159+
e.i();
160+
}

‎rule_packages/cpp/BannedSyntax.json

Copy file name to clipboardExpand all lines: rule_packages/cpp/BannedSyntax.json
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,14 @@
141141
"tags": [
142142
"correctness",
143143
"scope/single-translation-unit"
144-
]
144+
],
145+
"implementation_scope": {
146+
"description": "This query has the following limitations:",
147+
"items": [
148+
"It erroneously reports functional notation casts on primitive types (e.g. int(x)) as traditional C-style casts.",
149+
"It will not report C-Style casts that result in a direct initialization via a constructor call with the given argument."
150+
]
151+
}
145152
}
146153
],
147154
"title": "Traditional C-style casts shall not be used."

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.