Skip to content

Commit f5f0b14

Browse files
committed
[clang-tidy] Improve integer comparison by matching valid expressions outside implicitCastExpr
1 parent 79210fe commit f5f0b14

File tree

3 files changed

+41
-3
lines changed

3 files changed

+41
-3
lines changed

clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ intCastExpression(bool IsSigned,
3939
// std::cmp_{} functions trigger a compile-time error if either LHS or RHS
4040
// is a non-integer type, char, enum or bool
4141
// (unsigned char/ signed char are Ok and can be used).
42-
auto IntTypeExpr = expr(hasType(hasCanonicalType(qualType(
42+
const auto HasIntegerType = hasType(hasCanonicalType(qualType(
4343
isInteger(), IsSigned ? isSignedInteger() : isUnsignedInteger(),
44-
unless(isActualChar()), unless(booleanType()), unless(enumType())))));
44+
unless(isActualChar()), unless(booleanType()), unless(enumType()))));
45+
46+
auto IntTypeExpr = expr(HasIntegerType);
4547

4648
const auto ImplicitCastExpr =
4749
CastBindName.empty() ? implicitCastExpr(hasSourceExpression(IntTypeExpr))
@@ -52,8 +54,14 @@ intCastExpression(bool IsSigned,
5254
const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
5355
const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
5456

57+
// Match function calls or variable references not directly wrapped by an
58+
// implicit cast
59+
const auto CallIntExpr = CastBindName.empty()
60+
? callExpr(HasIntegerType)
61+
: callExpr(HasIntegerType).bind(CastBindName);
62+
5563
return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
56-
FunctionalCastExpr));
64+
FunctionalCastExpr, CallIntExpr));
5765
}
5866

5967
static StringRef parseOpCode(BinaryOperator::Opcode Code) {

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,10 @@ Changes in existing checks
197197
<clang-tidy/checks/modernize/use-designated-initializers>` check by avoiding
198198
diagnosing designated initializers for ``std::array`` initializations.
199199

200+
- Improved :doc:`modernize-use-integer-sign-comparison
201+
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check by matching
202+
valid integer expressions not directly wrapped around an implicit cast.
203+
200204
- Improved :doc:`modernize-use-ranges
201205
<clang-tidy/checks/modernize/use-ranges>` check by updating suppress
202206
warnings logic for ``nullptr`` in ``std::find``.

clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,3 +121,29 @@ int AllComparisons() {
121121

122122
return 0;
123123
}
124+
125+
namespace PR127471 {
126+
int getSignedValue();
127+
unsigned int getUnsignedValue();
128+
129+
void callExprTest() {
130+
131+
if (getSignedValue() < getUnsignedValue())
132+
return;
133+
// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
134+
// CHECK-FIXES: if (std::cmp_less(getSignedValue() , getUnsignedValue()))
135+
136+
int sVar = 0;
137+
if (getUnsignedValue() > sVar)
138+
return;
139+
// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
140+
// CHECK-FIXES: if (std::cmp_greater(getUnsignedValue() , sVar))
141+
142+
unsigned int uVar = 0;
143+
if (getSignedValue() > uVar)
144+
return;
145+
// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
146+
// CHECK-FIXES: if (std::cmp_greater(getSignedValue() , uVar))
147+
148+
}
149+
} // namespace PR127471

0 commit comments

Comments
 (0)