Skip to content

Commit 74482eb

Browse files
MichaelRFairhurstMichaelRFairhurst
committed
Address feedback
1 parent 752056e commit 74482eb

File tree

5 files changed

+77
-48
lines changed

5 files changed

+77
-48
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ predicate isTemplateSpecialization(Declaration d) {
5454
d instanceof ClassTemplateSpecialization
5555
}
5656

57-
private newtype TIndentifierIntroduction =
57+
private newtype TIdentifierIntroduction =
5858
TSomeIdentifierIntroduction(
5959
string ident, Location loc, IdentifierIntroductionImpl::IdentifierIntroductionBase element
6060
) {
@@ -77,7 +77,7 @@ private newtype TIndentifierIntroduction =
7777
* which introduces both the macro name and its parameter names, and therefore plural `getAnIdent()`
7878
* is used instead of singular `getIdent()`.
7979
*/
80-
class IdentifierIntroduction extends TIndentifierIntroduction {
80+
class IdentifierIntroduction extends TIdentifierIntroduction {
8181
string getIdent() { this = TSomeIdentifierIntroduction(result, _, _) }
8282

8383
Element getElement() { this = TSomeIdentifierIntroduction(_, _, result) }
@@ -207,7 +207,7 @@ private module IdentifierIntroductionImpl {
207207
* This has to be treated specially. The member predicate `getName()` on a `FriendDecl` returns the
208208
* string "foo's friend", which is not an identifier in the program.
209209
*
210-
* The elements returned by the `getFriend()` member predicate often do not have a correspending
210+
* The elements returned by the `getFriend()` member predicate often do not have a corresponding
211211
* `DeclarationEntry`, and therefore these element identifiers are not matched by the other classes
212212
* defined here that extend `IdentifierIntroductionBase`.
213213
*
@@ -365,7 +365,7 @@ private module IdentifierIntroductionImpl {
365365
}
366366

367367
/**
368-
* An identifier introduced as a template function name or as a parameter of a function-like macro.
368+
* An identifier introduced as a macro name or as a parameter of a function-like macro.
369369
*/
370370
class MacroIdentifier extends IdentifierIntroductionBase, Macro {
371371
override string getAnIdent() {

‎cpp/misra/src/rules/RULE-5-10-1/PoorlyFormedIdentifier.ql‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @id cpp/misra/poorly-formed-identifier
33
* @name RULE-5-10-1: User-defined identifiers shall have an appropriate form
4-
* @description Identifiers shall not conflict with keywords, reserved name, or otherwise be poorly
4+
* @description Identifiers shall not conflict with keywords, reserved names, or otherwise be poorly
55
* formed.
66
* @kind problem
77
* @precision very-high
@@ -32,11 +32,18 @@ predicate hasDoubleUnderscore(string s) { s.matches("%\\_\\_%") }
3232
bindingset[s]
3333
predicate startsWithUnderscore(string s) { s.matches("\\_%") }
3434

35-
predicate isUserDefinedLiteralSuffixNonCompliant(Function f) {
36-
f.getName().matches("operator\"\"%") and
37-
(
38-
not f.getName().matches("operator\"\"\\_%") or
39-
f.getName().matches("operator\"\" \\_%")
35+
predicate isReservedLiteralSuffix(Function f, IdentifierIntroduction intro) {
36+
// `operator"" _km` is reserved while `operator ""_km` is not. We don't have adequate information
37+
// from the extractor to distinguish this, however, we can use offset information to detect
38+
// `operator "" _km` as a best effort guess.
39+
f = intro.getElement().(FunctionDeclarationEntry).getFunction() and
40+
f.getName().matches("operator%\"\"%") and
41+
startsWithUnderscore(intro.getIdent()) and
42+
exists(Location loc, int reservedLength, int actualLength | loc = intro.getLocation() |
43+
loc.getStartLine() = loc.getEndLine() and
44+
reservedLength = ("operator \"\" " + intro).length() and
45+
actualLength = loc.getEndColumn() - loc.getStartColumn() + 1 and
46+
reservedLength = actualLength
4047
)
4148
}
4249

@@ -69,7 +76,7 @@ where
6976
or
7077
intro.isFromMacro() and
7178
not ident.regexpMatch("^[a-zA-Z0-9_]+$") and
72-
message = "Identifier '" + ident + "' contains invalid characters. "
79+
message = "Identifier '" + ident + "' contains invalid characters."
7380
or
7481
intro.isFromMacro() and
7582
containsLowercase(ident) and
@@ -95,8 +102,10 @@ where
95102
message = "Identifier '" + ident + "' is defined in reserved namespace."
96103
or
97104
exists(Function func | func = intro.getElement().(FunctionDeclarationEntry).getFunction() |
98-
isUserDefinedLiteralSuffixNonCompliant(func) and
99-
message = "User-defined literal suffix '" + ident + "' is malformed."
105+
isReservedLiteralSuffix(func, intro) and
106+
message =
107+
"User-defined literal suffix '" + ident +
108+
"' is reserved due to leading underscore and space."
100109
)
101110
)
102111
select intro, message

‎cpp/misra/test/rules/RULE-5-10-1/PoorlyFormedIdentifier.expected‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,35 +14,38 @@
1414
| test.cpp:52:7:52:13 | __end__ | Identifier '__end__' starts with underscore. |
1515
| test.cpp:57:7:57:12 | _local | Identifier '_local' starts with underscore. |
1616
| test.cpp:68:15:68:28 | mil | User-defined literal suffix 'mil' does not start with underscore. |
17-
| test.cpp:92:1:92:24 | invalid_macro | Identifier 'invalid_macro' contains lowercase characters. |
18-
| test.cpp:93:1:93:21 | Mixed_Case | Identifier 'Mixed_Case' contains lowercase characters. |
19-
| test.cpp:95:1:95:27 | MACRO_with_lower | Identifier 'MACRO_with_lower' contains lowercase characters. |
20-
| test.cpp:96:1:96:26 | MACRO_123_lower | Identifier 'MACRO_123_lower' contains lowercase characters. |
21-
| test.cpp:97:1:97:25 | macro_ALL_CAPS | Identifier 'macro_ALL_CAPS' contains lowercase characters. |
22-
| test.cpp:98:1:98:23 | MACRO$DOLLAR | Identifier 'MACRO$DOLLAR' contains invalid characters. |
23-
| test.cpp:99:1:100:11 | x | Identifier 'x' contains lowercase characters. |
24-
| test.cpp:104:1:104:38 | __Y | Identifier '__Y' contains double underscores. |
25-
| test.cpp:104:1:104:38 | __Y | Identifier '__Y' starts with underscore. |
26-
| test.cpp:117:5:117:6 | l2 | Identifier 'l2' is defined in reserved namespace. |
27-
| test.cpp:121:5:121:6 | l3 | Identifier 'l3' is defined in reserved namespace. |
28-
| test.cpp:125:5:125:6 | l4 | Identifier 'l4' is defined in reserved namespace. |
29-
| test.cpp:130:5:130:6 | l5 | Identifier 'l5' is defined in reserved namespace. |
30-
| test.cpp:148:7:148:11 | final | Identifier 'final' is a reserved name. |
31-
| test.cpp:149:7:149:14 | override | Identifier 'override' is a reserved name. |
32-
| test.cpp:150:7:150:13 | defined | Identifier 'defined' is a reserved name. |
33-
| test.cpp:151:7:151:12 | size_t | Identifier 'size_t' is a reserved name. |
34-
| test.cpp:152:7:152:10 | FILE | Identifier 'FILE' is a reserved name. |
35-
| test.cpp:153:7:153:12 | time_t | Identifier 'time_t' is a reserved name. |
36-
| test.cpp:154:7:154:15 | ptrdiff_t | Identifier 'ptrdiff_t' is a reserved name. |
37-
| test.cpp:155:7:155:13 | clock_t | Identifier 'clock_t' is a reserved name. |
38-
| test.cpp:156:7:156:11 | div_t | Identifier 'div_t' is a reserved name. |
39-
| test.cpp:157:7:157:12 | fpos_t | Identifier 'fpos_t' is a reserved name. |
40-
| test.cpp:158:7:158:11 | lconv | Identifier 'lconv' is a reserved name. |
41-
| test.cpp:159:7:159:12 | ldiv_t | Identifier 'ldiv_t' is a reserved name. |
42-
| test.cpp:160:7:160:15 | mbstate_t | Identifier 'mbstate_t' is a reserved name. |
43-
| test.cpp:161:7:161:18 | sig_atomic_t | Identifier 'sig_atomic_t' is a reserved name. |
44-
| test.cpp:162:7:162:8 | tm | Identifier 'tm' is a reserved name. |
45-
| test.cpp:163:7:163:13 | va_list | Identifier 'va_list' is a reserved name. |
46-
| test.cpp:164:7:164:15 | wctrans_t | Identifier 'wctrans_t' is a reserved name. |
47-
| test.cpp:165:7:165:14 | wctype_t | Identifier 'wctype_t' is a reserved name. |
48-
| test.cpp:166:7:166:12 | wint_t | Identifier 'wint_t' is a reserved name. |
17+
| test.cpp:80:15:80:37 | _micrometer | User-defined literal suffix '_micrometer' is reserved due to leading underscore and space. |
18+
| test.cpp:84:15:84:36 | _picometer | User-defined literal suffix '_picometer' is reserved due to leading underscore and space. |
19+
| test.cpp:87:15:87:34 | angstrom | User-defined literal suffix 'angstrom' does not start with underscore. |
20+
| test.cpp:109:1:109:24 | invalid_macro | Identifier 'invalid_macro' contains lowercase characters. |
21+
| test.cpp:110:1:110:21 | Mixed_Case | Identifier 'Mixed_Case' contains lowercase characters. |
22+
| test.cpp:112:1:112:27 | MACRO_with_lower | Identifier 'MACRO_with_lower' contains lowercase characters. |
23+
| test.cpp:113:1:113:26 | MACRO_123_lower | Identifier 'MACRO_123_lower' contains lowercase characters. |
24+
| test.cpp:114:1:114:25 | macro_ALL_CAPS | Identifier 'macro_ALL_CAPS' contains lowercase characters. |
25+
| test.cpp:115:1:115:23 | MACRO$DOLLAR | Identifier 'MACRO$DOLLAR' contains invalid characters. |
26+
| test.cpp:116:1:117:11 | x | Identifier 'x' contains lowercase characters. |
27+
| test.cpp:121:1:121:38 | __Y | Identifier '__Y' contains double underscores. |
28+
| test.cpp:121:1:121:38 | __Y | Identifier '__Y' starts with underscore. |
29+
| test.cpp:134:5:134:6 | l2 | Identifier 'l2' is defined in reserved namespace. |
30+
| test.cpp:138:5:138:6 | l3 | Identifier 'l3' is defined in reserved namespace. |
31+
| test.cpp:142:5:142:6 | l4 | Identifier 'l4' is defined in reserved namespace. |
32+
| test.cpp:147:5:147:6 | l5 | Identifier 'l5' is defined in reserved namespace. |
33+
| test.cpp:165:7:165:11 | final | Identifier 'final' is a reserved name. |
34+
| test.cpp:166:7:166:14 | override | Identifier 'override' is a reserved name. |
35+
| test.cpp:167:7:167:13 | defined | Identifier 'defined' is a reserved name. |
36+
| test.cpp:168:7:168:12 | size_t | Identifier 'size_t' is a reserved name. |
37+
| test.cpp:169:7:169:10 | FILE | Identifier 'FILE' is a reserved name. |
38+
| test.cpp:170:7:170:12 | time_t | Identifier 'time_t' is a reserved name. |
39+
| test.cpp:171:7:171:15 | ptrdiff_t | Identifier 'ptrdiff_t' is a reserved name. |
40+
| test.cpp:172:7:172:13 | clock_t | Identifier 'clock_t' is a reserved name. |
41+
| test.cpp:173:7:173:11 | div_t | Identifier 'div_t' is a reserved name. |
42+
| test.cpp:174:7:174:12 | fpos_t | Identifier 'fpos_t' is a reserved name. |
43+
| test.cpp:175:7:175:11 | lconv | Identifier 'lconv' is a reserved name. |
44+
| test.cpp:176:7:176:12 | ldiv_t | Identifier 'ldiv_t' is a reserved name. |
45+
| test.cpp:177:7:177:15 | mbstate_t | Identifier 'mbstate_t' is a reserved name. |
46+
| test.cpp:178:7:178:18 | sig_atomic_t | Identifier 'sig_atomic_t' is a reserved name. |
47+
| test.cpp:179:7:179:8 | tm | Identifier 'tm' is a reserved name. |
48+
| test.cpp:180:7:180:13 | va_list | Identifier 'va_list' is a reserved name. |
49+
| test.cpp:181:7:181:15 | wctrans_t | Identifier 'wctrans_t' is a reserved name. |
50+
| test.cpp:182:7:182:14 | wctype_t | Identifier 'wctype_t' is a reserved name. |
51+
| test.cpp:183:7:183:12 | wint_t | Identifier 'wint_t' is a reserved name. |

‎cpp/misra/test/rules/RULE-5-10-1/test.cpp‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,9 +67,26 @@ void test_user_defined_literals() {
6767
// NON_COMPLIANT - user-defined literal suffix doesn't start with underscore
6868
long double operator"" mil(long double value) { return value; }
6969

70-
// NON_COMPLIANT - space before underscore makes it reserved
70+
// Space before underscore makes it reserved, however, we can't detect this in
71+
// the extractor. NON_COMPLIANT[False negative]
7172
long double operator"" _meter(long double value) { return value; }
7273

74+
// COMPLIANT - space before quotes does not affect compliance.
75+
long double operator""_nanometer(long double value) { return value; }
76+
77+
// clang-format off
78+
// NON_COMPLIANT - space before underscore makes it reserved, this we can
79+
// guess with offsets.
80+
long double operator "" _micrometer(long double value) { return value; }
81+
82+
// COMPLIANT[False positive] - This confuses our logic that uses spaces to
83+
// guess offsets.
84+
long double operator ""_picometer(long double value) { return value; }
85+
86+
// NON_COMPLIANT -- not reserved, but required to start with an underscore.
87+
long double operator "" angstrom(long double value) { return value; }
88+
// clang-format on
89+
7390
// COMPLIANT - proper user-defined literal suffix
7491
int operator""_count(unsigned long long value) {
7592
return static_cast<int>(value);
@@ -99,7 +116,7 @@ class C1 {
99116
#define FUNCTION_LIKE_MACRO(x) \
100117
((x) + 1) // NON_COMPLIANT - lower case argument name
101118
#define FUNCTION_LIKE_MACRO2(X) \
102-
((X) + 1) // NON_COMPLIANT - lower case argument name
119+
((X) + 1) // COMPLIANT - upper case argument name
103120
#define VARIADIC_MACRO(X, __VA_ARGS__...) __VA_ARGS__ // COMPLIANT - variadic
104121
#define NOT_VARIADIC_MACRO(X, __Y) __Y // NON_COMPLIANT -- double underscore
105122

‎rule_packages/cpp/Naming2.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
},
88
"queries": [
99
{
10-
"description": "Identifiers shall not conflict with keywords, reserved name, or otherwise be poorly formed.",
10+
"description": "Identifiers shall not conflict with keywords, reserved names, or otherwise be poorly formed.",
1111
"kind": "problem",
1212
"name": "User-defined identifiers shall have an appropriate form",
1313
"precision": "very-high",

0 commit comments

Comments
 (0)