Skip to content

Commit 1910be5

Browse files
bjosvmahesh-attarde
bjosv
authored and
mahesh-attarde
committed
[clang-tidy] Fix false positives on C23 enums in bugprone-signed-char-misuse (llvm#149790)
Ignore false positives on C23 enums which allows setting the fixed underlying type to signed char. The AST tree for C enums includes a ImplicitCastExp (that was matched) but this is not the case for C++ enums. Fixes llvm#145651 --------- Signed-off-by: Björn Svensson <[email protected]>
1 parent fc25ed4 commit 1910be5

File tree

3 files changed

+218
-5
lines changed

3 files changed

+218
-5
lines changed

‎clang-tools-extra/clang-tidy/bugprone/SignedCharMisuseCheck.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,17 +74,25 @@ void SignedCharMisuseCheck::registerMatchers(MatchFinder *Finder) {
7474
charCastExpression(true, IntegerType, "signedCastExpression");
7575
const auto UnSignedCharCastExpr =
7676
charCastExpression(false, IntegerType, "unsignedCastExpression");
77+
const bool IsC23 = getLangOpts().C23;
7778

78-
// Catch assignments with signed char -> integer conversion.
79+
// Catch assignments with signed char -> integer conversion. Ignore false
80+
// positives on C23 enums with the fixed underlying type of signed char.
7981
const auto AssignmentOperatorExpr =
8082
expr(binaryOperator(hasOperatorName("="), hasLHS(hasType(IntegerType)),
81-
hasRHS(SignedCharCastExpr)));
83+
hasRHS(SignedCharCastExpr)),
84+
IsC23 ? unless(binaryOperator(
85+
hasLHS(hasType(hasCanonicalType(enumType())))))
86+
: Matcher<Stmt>(anything()));
8287

8388
Finder->addMatcher(AssignmentOperatorExpr, this);
8489

85-
// Catch declarations with signed char -> integer conversion.
86-
const auto Declaration = varDecl(isDefinition(), hasType(IntegerType),
87-
hasInitializer(SignedCharCastExpr));
90+
// Catch declarations with signed char -> integer conversion. Ignore false
91+
// positives on C23 enums with the fixed underlying type of signed char.
92+
const auto Declaration = varDecl(
93+
isDefinition(), hasType(IntegerType), hasInitializer(SignedCharCastExpr),
94+
IsC23 ? unless(hasType(hasCanonicalType(enumType())))
95+
: Matcher<VarDecl>(anything()));
8896

8997
Finder->addMatcher(Declaration, this);
9098

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,10 @@ Changes in existing checks
106106
<clang-tidy/checks/bugprone/infinite-loop>` check by adding detection for
107107
variables introduced by structured bindings.
108108

109+
- Improved :doc:`bugprone-signed-char-misuse
110+
<clang-tidy/checks/bugprone/signed-char-misuse>` check by fixing
111+
false positives on C23 enums with the fixed underlying type of signed char.
112+
109113
- Improved :doc:`bugprone-unhandled-self-assignment
110114
<clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
111115
an additional matcher that generalizes the copy-and-swap idiom pattern

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/signed-char-misuse-c23.c

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
// RUN: %check_clang_tidy %s bugprone-signed-char-misuse %t -- -- -std=c23
2+
3+
///////////////////////////////////////////////////////////////////
4+
/// Test cases correctly caught by the check.
5+
6+
int SimpleVarDeclaration() {
7+
signed char CCharacter = -5;
8+
int NCharacter = CCharacter;
9+
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
10+
11+
return NCharacter;
12+
}
13+
14+
int SimpleAssignment() {
15+
signed char CCharacter = -5;
16+
int NCharacter;
17+
NCharacter = CCharacter;
18+
// CHECK-MESSAGES: [[@LINE-1]]:16: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
19+
20+
return NCharacter;
21+
}
22+
23+
int NegativeConstValue() {
24+
const signed char CCharacter = -5;
25+
int NCharacter = CCharacter;
26+
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
27+
28+
return NCharacter;
29+
}
30+
31+
int CharPointer(signed char *CCharacter) {
32+
int NCharacter = *CCharacter;
33+
// CHECK-MESSAGES: [[@LINE-1]]:20: warning: 'signed char' to 'int' conversion; consider casting to 'unsigned char' first. [bugprone-signed-char-misuse]
34+
35+
return NCharacter;
36+
}
37+
38+
int SignedUnsignedCharEquality(signed char SCharacter) {
39+
unsigned char USCharacter = 'a';
40+
if (SCharacter == USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
41+
return 1;
42+
return 0;
43+
}
44+
45+
int SignedUnsignedCharIneqiality(signed char SCharacter) {
46+
unsigned char USCharacter = 'a';
47+
if (SCharacter != USCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
48+
return 1;
49+
return 0;
50+
}
51+
52+
int CompareWithNonAsciiConstant(unsigned char USCharacter) {
53+
const signed char SCharacter = -5;
54+
if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
55+
return 1;
56+
return 0;
57+
}
58+
59+
int CompareWithUnsignedNonAsciiConstant(signed char SCharacter) {
60+
const unsigned char USCharacter = 128;
61+
if (USCharacter == SCharacter) // CHECK-MESSAGES: [[@LINE]]:7: warning: comparison between 'signed char' and 'unsigned char' [bugprone-signed-char-misuse]
62+
return 1;
63+
return 0;
64+
}
65+
66+
///////////////////////////////////////////////////////////////////
67+
/// Test cases correctly ignored by the check.
68+
69+
// Enum with a fixed underlying type of signed char.
70+
enum EType1 : signed char {
71+
EType1_M128 = -128,
72+
EType1_1 = 1,
73+
};
74+
75+
enum EType1 es1_1 = EType1_M128;
76+
enum EType1 es1_2 = EType1_1;
77+
enum EType1 es1_3 = -128;
78+
79+
void assign(enum EType1 *es) {
80+
*es = EType1_M128;
81+
}
82+
83+
// Type aliased enum with a fixed underlying type of signed char.
84+
typedef signed char int8_t;
85+
typedef enum EType2 : int8_t {
86+
EType2_M128 = -128,
87+
EType2_1 = 1,
88+
} EType2_t;
89+
90+
EType2_t es2_1 = EType2_M128;
91+
EType2_t es2_2 = EType2_1;
92+
EType2_t es2_3 = -128;
93+
94+
// Enum with a fixed underlying type of unsigned char.
95+
enum EType3 : unsigned char {
96+
EType3_1 = 1,
97+
EType3_128 = 128,
98+
};
99+
100+
enum EType3 es3_1 = EType3_1;
101+
enum EType3 es3_2 = EType3_128;
102+
enum EType3 es3_3 = 128;
103+
104+
105+
int UnsignedCharCast() {
106+
unsigned char CCharacter = 'a';
107+
int NCharacter = CCharacter;
108+
109+
return NCharacter;
110+
}
111+
112+
int PositiveConstValue() {
113+
const signed char CCharacter = 5;
114+
int NCharacter = CCharacter;
115+
116+
return NCharacter;
117+
}
118+
119+
// signed char -> integer cast is not the direct child of declaration expression.
120+
int DescendantCast() {
121+
signed char CCharacter = 'a';
122+
int NCharacter = 10 + CCharacter;
123+
124+
return NCharacter;
125+
}
126+
127+
// signed char -> integer cast is not the direct child of assignment expression.
128+
int DescendantCastAssignment() {
129+
signed char CCharacter = 'a';
130+
int NCharacter;
131+
NCharacter = 10 + CCharacter;
132+
133+
return NCharacter;
134+
}
135+
136+
// bool is an integer type in clang; make sure to ignore it.
137+
bool BoolVarDeclaration() {
138+
signed char CCharacter = 'a';
139+
bool BCharacter = CCharacter == 'b';
140+
141+
return BCharacter;
142+
}
143+
144+
// bool is an integer type in clang; make sure to ignore it.
145+
bool BoolAssignment() {
146+
signed char CCharacter = 'a';
147+
bool BCharacter;
148+
BCharacter = CCharacter == 'b';
149+
150+
return BCharacter;
151+
}
152+
153+
// char is an integer type in clang; make sure to ignore it.
154+
unsigned char CharToCharCast() {
155+
signed char SCCharacter = 'a';
156+
unsigned char USCharacter;
157+
USCharacter = SCCharacter;
158+
159+
return USCharacter;
160+
}
161+
162+
int SameCharTypeComparison(signed char SCharacter) {
163+
signed char SCharacter2 = 'a';
164+
if (SCharacter == SCharacter2)
165+
return 1;
166+
return 0;
167+
}
168+
169+
int SameCharTypeComparison2(unsigned char USCharacter) {
170+
unsigned char USCharacter2 = 'a';
171+
if (USCharacter == USCharacter2)
172+
return 1;
173+
return 0;
174+
}
175+
176+
int CharIntComparison(signed char SCharacter) {
177+
int ICharacter = 10;
178+
if (SCharacter == ICharacter)
179+
return 1;
180+
return 0;
181+
}
182+
183+
int CompareWithAsciiLiteral(unsigned char USCharacter) {
184+
if (USCharacter == 'x')
185+
return 1;
186+
return 0;
187+
}
188+
189+
int CompareWithAsciiConstant(unsigned char USCharacter) {
190+
const signed char SCharacter = 'a';
191+
if (USCharacter == SCharacter)
192+
return 1;
193+
return 0;
194+
}
195+
196+
int CompareWithUnsignedAsciiConstant(signed char SCharacter) {
197+
const unsigned char USCharacter = 'a';
198+
if (USCharacter == SCharacter)
199+
return 1;
200+
return 0;
201+
}

0 commit comments

Comments
 (0)