[clang-tools-extra] [clang-tidy] Fix modernize-use-integer-sign-comparison comparison (PR #121506)

via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 3 02:59:04 PST 2025


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-tidy

Author: None (qt-tatiana)

<details>
<summary>Changes</summary>

- modernize-use-integer-sign-comparison should ignore a comparison between the signed wide type and the unsigned narrow type, see #<!-- -->120867

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


2 Files Affected:

- (modified) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp (+13-1) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp (+22-2) 


``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 8f807bc0a96d56..ebcfafcd391145 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -89,7 +89,8 @@ void UseIntegerSignComparisonCheck::storeOptions(
 
 void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
   const auto SignedIntCastExpr = intCastExpression(true, "sIntCastExpression");
-  const auto UnSignedIntCastExpr = intCastExpression(false);
+  const auto UnSignedIntCastExpr =
+      intCastExpression(false, "uIntCastExpression");
 
   // Flag all operators "==", "<=", ">=", "<", ">", "!="
   // that are used between signed/unsigned
@@ -111,7 +112,10 @@ void UseIntegerSignComparisonCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *SignedCastExpression =
       Result.Nodes.getNodeAs<ImplicitCastExpr>("sIntCastExpression");
+  const auto *UnSignedCastExpression =
+      Result.Nodes.getNodeAs<ImplicitCastExpr>("uIntCastExpression");
   assert(SignedCastExpression);
+  assert(UnSignedCastExpression);
 
   // Ignore the match if we know that the signed int value is not negative.
   Expr::EvalResult EVResult;
@@ -134,6 +138,13 @@ void UseIntegerSignComparisonCheck::check(
   const Expr *RHS = BinaryOp->getRHS()->IgnoreImpCasts();
   if (LHS == nullptr || RHS == nullptr)
     return;
+
+  if (Result.Context->getTypeSize(
+          SignedCastExpression->getSubExpr()->getType()) >
+      Result.Context->getTypeSize(
+          UnSignedCastExpression->getSubExpr()->getType()))
+    return;
+
   const Expr *SubExprLHS = nullptr;
   const Expr *SubExprRHS = nullptr;
   SourceRange R1 = SourceRange(LHS->getBeginLoc());
@@ -151,6 +162,7 @@ void UseIntegerSignComparisonCheck::check(
     SubExprRHS = RHSCast->getSubExpr();
     R2.setEnd(SubExprRHS->getBeginLoc().getLocWithOffset(-1));
   }
+
   DiagnosticBuilder Diag =
       diag(BinaryOp->getBeginLoc(),
            "comparison between 'signed' and 'unsigned' integers");
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 99f00444c2d3f3..85eda925d5903c 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
@@ -32,7 +32,7 @@ int TemplateFuncParameters(T1 val1, T2 val2) {
 
 int AllComparisons() {
     unsigned int uVar = 42;
-    unsigned short uArray[7] = {0, 1, 2, 3, 9, 7, 9};
+    unsigned int uArray[7] = {0, 1, 2, 3, 9, 7, 9};
 
     int sVar = -42;
     short sArray[7] = {-1, -2, -8, -94, -5, -4, -6};
@@ -113,10 +113,30 @@ int AllComparisons() {
 // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
 // CHECK-FIXES: if (std::cmp_greater(uArray[6] , VALUE))
 
-
     FuncParameters(uVar);
     TemplateFuncParameter(sVar);
     TemplateFuncParameters(uVar, sVar);
 
     return 0;
 }
+
+bool foo1(int x, unsigned char y) {
+    return x == y;
+// CHECK-MESSAGES-NOT: warning:
+}
+
+bool foo2(int x, unsigned short y) {
+    return x == y;
+// CHECK-MESSAGES-NOT: warning:
+}
+
+bool bar1(unsigned int x, char y) {
+    return x == y;
+// CHECK-MESSAGES-NOT: warning:
+}
+
+bool bar2(unsigned int x, short y) {
+    return x == y;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: std::cmp_equal(x , y);
+}

``````````

</details>


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


More information about the cfe-commits mailing list