https://github.com/RiverDave updated
https://github.com/llvm/llvm-project/pull/134188
>From daab8b4ccbce1f15758378c8229207a871b40683 Mon Sep 17 00:00:00 2001
From: David Rivera <davidriv...@gmail.com>
Date: Wed, 2 Apr 2025 21:02:00 -0400
Subject: [PATCH 1/2] [clang-tidy] Improve integer comparison by matching valid
expressions outside implicitCastExpr for use-integer-sign-comparison
---
.../UseIntegerSignComparisonCheck.cpp | 21 +++--
clang-tools-extra/docs/ReleaseNotes.rst | 4 +
.../modernize/use-integer-sign-comparison.cpp | 83 +++++++++++++++++++
3 files changed, 101 insertions(+), 7 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index eeba5cce80da5..ce43d8ed2ce4d 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -39,21 +39,28 @@ intCastExpression(bool IsSigned,
// std::cmp_{} functions trigger a compile-time error if either LHS or RHS
// is a non-integer type, char, enum or bool
// (unsigned char/ signed char are Ok and can be used).
- auto IntTypeExpr = expr(hasType(hasCanonicalType(qualType(
+ const auto HasIntegerType = hasType(hasCanonicalType(qualType(
isInteger(), IsSigned ? isSignedInteger() : isUnsignedInteger(),
- unless(isActualChar()), unless(booleanType()), unless(enumType())))));
+ unless(isActualChar()), unless(booleanType()), unless(enumType()))));
+
+ const auto IntTypeExpr = expr(HasIntegerType);
const auto ImplicitCastExpr =
CastBindName.empty() ? implicitCastExpr(hasSourceExpression(IntTypeExpr))
: implicitCastExpr(hasSourceExpression(IntTypeExpr))
.bind(CastBindName);
- const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
- const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
- const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
+ const auto ExplicitCastExpr = anyOf(
+ explicitCastExpr(hasDescendant(ImplicitCastExpr)),
+ ignoringImpCasts(explicitCastExpr(hasDescendant(ImplicitCastExpr))));
+
+ // Match function calls or variable references not directly wrapped by an
+ // implicit cast
+ const auto CallIntExpr = CastBindName.empty()
+ ? callExpr(HasIntegerType)
+ : callExpr(HasIntegerType).bind(CastBindName);
- return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
- FunctionalCastExpr));
+ return expr(anyOf(ImplicitCastExpr, ExplicitCastExpr, CallIntExpr));
}
static StringRef parseOpCode(BinaryOperator::Opcode Code) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index e0f81a032c38d..a2d67f6644eaa 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -223,6 +223,10 @@ Changes in existing checks
<clang-tidy/checks/modernize/use-designated-initializers>` check by avoiding
diagnosing designated initializers for ``std::array`` initializations.
+- Improved :doc:`modernize-use-integer-sign-comparison
+ <clang-tidy/checks/modernize/use-integer-sign-comparison>` check by matching
+ valid integer expressions not directly wrapped around an implicit cast.
+
- Improved :doc:`modernize-use-ranges
<clang-tidy/checks/modernize/use-ranges>` check by updating suppress
warnings logic for ``nullptr`` in ``std::find``.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
index e0a84ef5aed26..5af74a39bd67e 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
@@ -121,3 +121,86 @@ int AllComparisons() {
return 0;
}
+
+namespace PR127471 {
+ int getSignedValue();
+ unsigned int getUnsignedValue();
+
+ void callExprTest() {
+
+ if (getSignedValue() < getUnsignedValue())
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_less(getSignedValue() , getUnsignedValue()))
+
+ int sVar = 0;
+ if (getUnsignedValue() > sVar)
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_greater(getUnsignedValue() , sVar))
+
+ unsigned int uVar = 0;
+ if (getSignedValue() > uVar)
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_greater(getSignedValue() , uVar))
+
+ }
+
+ // Add a class with member functions for testing member function calls
+ class TestClass {
+ public:
+ int getSignedValue() { return -5; }
+ unsigned int getUnsignedValue() { return 5; }
+ };
+
+ void memberFunctionTests() {
+ TestClass obj;
+
+ if (obj.getSignedValue() < obj.getUnsignedValue())
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_less(obj.getSignedValue() ,
obj.getUnsignedValue()))
+ }
+
+ void castFunctionTests() {
+ // C-style casts with function calls
+ if ((int)getUnsignedValue() < (unsigned int)getSignedValue())
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_less(getUnsignedValue(),getSignedValue()))
+
+
+ // Static casts with function calls
+ if (static_cast<int>(getUnsignedValue()) < static_cast<unsigned
int>(getSignedValue()))
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_less(getUnsignedValue(),getSignedValue()))
+ }
+
+ // Define tests
+ #define SIGNED_FUNC getSignedValue()
+ #define UNSIGNED_FUNC getUnsignedValue()
+
+ void defineTests() {
+ if (SIGNED_FUNC < UNSIGNED_FUNC)
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:13: warning: comparison between 'signed' and
'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_less(SIGNED_FUNC , UNSIGNED_FUNC))
+ }
+
+ // Template tests (should not warn)
+ template <typename T>
+ void templateFunctionTest(T value) {
+ unsigned int uVar = 42;
+ if (value < uVar)
+ return;
+// CHECK-MESSAGES-NOT: warning:
+ }
+
+ void runTemplateTest() {
+ int sVar = -42;
+ templateFunctionTest(sVar); // This should not trigger warnings
+ }
+
+} // namespace PR127471
>From 5bf659d27f2831dab5997d4f24ee110d1572e87b Mon Sep 17 00:00:00 2001
From: David Rivera <davidriv...@gmail.com>
Date: Thu, 5 Jun 2025 09:39:51 -0400
Subject: [PATCH 2/2] Fix format
---
.../modernize/UseIntegerSignComparisonCheck.cpp | 6 +++---
.../modernize/use-integer-sign-comparison.cpp | 15 +++++----------
2 files changed, 8 insertions(+), 13 deletions(-)
diff --git
a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index ce43d8ed2ce4d..c02c5dfa8756d 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -50,9 +50,9 @@ intCastExpression(bool IsSigned,
: implicitCastExpr(hasSourceExpression(IntTypeExpr))
.bind(CastBindName);
- const auto ExplicitCastExpr = anyOf(
- explicitCastExpr(hasDescendant(ImplicitCastExpr)),
- ignoringImpCasts(explicitCastExpr(hasDescendant(ImplicitCastExpr))));
+ const auto ExplicitCastExpr =
+ anyOf(explicitCastExpr(has(ImplicitCastExpr)),
+ ignoringImpCasts(explicitCastExpr(has(ImplicitCastExpr))));
// Match function calls or variable references not directly wrapped by an
// implicit cast
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
index 5af74a39bd67e..d93a05ac38050 100644
---
a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
+++
b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
@@ -190,17 +190,12 @@ namespace PR127471 {
}
// Template tests (should not warn)
- template <typename T>
- void templateFunctionTest(T value) {
- unsigned int uVar = 42;
- if (value < uVar)
+ template <typename T1>
+ void templateFunctionTest(T1 value) {
+ if (value() < getUnsignedValue())
return;
-// CHECK-MESSAGES-NOT: warning:
- }
- void runTemplateTest() {
- int sVar = -42;
- templateFunctionTest(sVar); // This should not trigger warnings
+ if (value() < (getSignedValue() || getUnsignedValue()))
+ return;
}
-
} // namespace PR127471
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
