[clang-tools-extra] [clang-tidy] Improve integer comparison by matching valid expressions outside implicitCastExpr (PR #134188)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: David Rivera (RiverDave)

<details>
<summary>Changes</summary>

Covered the edge case where an int expression is not necessarily directly wrapped around an ImplicitCastExpr which seemed to be a requirement in this check to trigger.

**For instance**:

```cpp
#include <vector>

bool f() {
  std::vector<int> v;
  unsigned int i = 0;

  return i >= v.size();
}
```
**See AST:**
![image](https://github.com/user-attachments/assets/9a52928a-f5a7-4857-aa69-90d978874578)

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


3 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp (+13-3) 
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp (+26) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index eeba5cce80da5..089df7ece3f82 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -39,9 +39,11 @@ 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()))));
+
+  auto IntTypeExpr = expr(HasIntegerType);
 
   const auto ImplicitCastExpr =
       CastBindName.empty() ? implicitCastExpr(hasSourceExpression(IntTypeExpr))
@@ -52,8 +54,16 @@ intCastExpression(bool IsSigned,
   const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
   const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
 
+  // Match function calls or variable references not wrapped by an implicit cast
+  const auto CallIntExpr = CastBindName.empty()
+                               ? callExpr(HasIntegerType)
+                               : callExpr(HasIntegerType).bind(CastBindName);
+  const auto DeclRefIntExpr =
+      CastBindName.empty() ? declRefExpr(HasIntegerType)
+                           : declRefExpr(HasIntegerType).bind(CastBindName);
+
   return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
-                    FunctionalCastExpr));
+                    FunctionalCastExpr, CallIntExpr));
 }
 
 static StringRef parseOpCode(BinaryOperator::Opcode Code) {
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6cb8d572d3a78..b5f2d8e8fcbd7 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -177,6 +177,10 @@ Changes in existing checks
   matched scenarios of ``find`` and ``rfind`` methods and fixing false
   positives when those methods were called with 3 arguments.
 
+- Improved :doc:`modernize-use-integer-sign-comparison
+  <clang-tidy/checks/modernize/use-integer-sign-comparison>` check by matching
+  valid integer expressions not wrapped around an Implicit Cast.
+
 - Improved :doc:`modernize-use-std-numbers
   <clang-tidy/checks/modernize/use-std-numbers>` check to support math
   functions of different precisions.
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 99f00444c2d3f..1d2f64a359a2c 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
@@ -120,3 +120,29 @@ 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))
+
+    }
+} // namespace PR127471

``````````

</details>


https://github.com/llvm/llvm-project/pull/134188