Revert "[clang-tidy] Improve integer comparison by matching valid expressions outside implicitCastExpr"

[RiverDave]

Reverts #134188
related: #143927

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: David Rivera (RiverDave)

Changes

Reverts llvm/llvm-project#134188
related: #143927

---

Full diff: https://github.com/llvm/llvm-project/pull/143944.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp (+7-14)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (-4)
• (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp (-78)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index c02c5dfa8756d..eeba5cce80da5 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -39,28 +39,21 @@ 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).
-  const auto HasIntegerType = hasType(hasCanonicalType(qualType(
+  auto IntTypeExpr = expr(hasType(hasCanonicalType(qualType(
       isInteger(), IsSigned ? isSignedInteger() : isUnsignedInteger(),
-      unless(isActualChar()), unless(booleanType()), unless(enumType()))));
-
-  const auto IntTypeExpr = expr(HasIntegerType);
+      unless(isActualChar()), unless(booleanType()), unless(enumType())))));
 
   const auto ImplicitCastExpr =
       CastBindName.empty() ? implicitCastExpr(hasSourceExpression(IntTypeExpr))
                            : implicitCastExpr(hasSourceExpression(IntTypeExpr))
                                  .bind(CastBindName);
 
-  const auto ExplicitCastExpr =
-      anyOf(explicitCastExpr(has(ImplicitCastExpr)),
-            ignoringImpCasts(explicitCastExpr(has(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);
+  const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
+  const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
+  const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
 
-  return expr(anyOf(ImplicitCastExpr, ExplicitCastExpr, CallIntExpr));
+  return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
+                    FunctionalCastExpr));
 }
 
 static StringRef parseOpCode(BinaryOperator::Opcode Code) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 882ee0015df17..19ccd1790e757 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -237,10 +237,6 @@ 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 d93a05ac38050..e0a84ef5aed26 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,81 +121,3 @@ 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 T1>
-    void templateFunctionTest(T1 value) {
-        if (value() < getUnsignedValue())
-            return;
-
-        if (value() < (getSignedValue() || getUnsignedValue()))
-          return;
-    }
-} // namespace PR127471