[clang-tools-extra] [clang-tidy][NFC] Clean up and slightly optimize `modernize-use-integer-sign-comparison` (PR #163492)

Victor Chernyakin via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 14 21:21:03 PDT 2025


https://github.com/localspook created https://github.com/llvm/llvm-project/pull/163492

None

>From 314636a3bb35d22cc8166a5d8e51527b00f055b5 Mon Sep 17 00:00:00 2001
From: Victor Chernyakin <chernyakin.victor.j at outlook.com>
Date: Tue, 14 Oct 2025 20:21:25 -0700
Subject: [PATCH] [clang-tidy][NFC] Clean up and slightly optimize
 `modernize-use-integer-sign-comparison`

---
 .../UseIntegerSignComparisonCheck.cpp         | 35 +++++++------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 0003429c62890..a3d3f0fa0ec93 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -34,13 +34,12 @@ AST_MATCHER(clang::QualType, isActualChar) {
 } // namespace
 
 static BindableMatcher<clang::Stmt>
-intCastExpression(bool IsSigned,
-                  const std::string &CastBindName = std::string()) {
+intCastExpression(bool IsSigned, StringRef CastBindName = {}) {
   // 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(
-      isInteger(), IsSigned ? isSignedInteger() : isUnsignedInteger(),
+      IsSigned ? isSignedInteger() : isUnsignedInteger(),
       unless(isActualChar()), unless(booleanType()), unless(enumType())))));
 
   const auto ImplicitCastExpr =
@@ -71,7 +70,7 @@ static StringRef parseOpCode(BinaryOperator::Opcode Code) {
   case BO_NE:
     return "cmp_not_equal";
   default:
-    return "";
+    llvm_unreachable("invalid opcode");
   }
 }
 
@@ -119,23 +118,16 @@ void UseIntegerSignComparisonCheck::check(
   Expr::EvalResult EVResult;
   if (!SignedCastExpression->isValueDependent() &&
       SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
-                                                        *Result.Context)) {
-    const llvm::APSInt SValue = EVResult.Val.getInt();
-    if (SValue.isNonNegative())
-      return;
-  }
+                                                        *Result.Context) &&
+      EVResult.Val.getInt().isNonNegative())
+    return;
 
   const auto *BinaryOp =
       Result.Nodes.getNodeAs<BinaryOperator>("intComparison");
-  if (BinaryOp == nullptr)
-    return;
-
-  const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode();
+  assert(BinaryOp);
 
   const Expr *LHS = BinaryOp->getLHS()->IgnoreImpCasts();
   const Expr *RHS = BinaryOp->getRHS()->IgnoreImpCasts();
-  if (LHS == nullptr || RHS == nullptr)
-    return;
   const Expr *SubExprLHS = nullptr;
   const Expr *SubExprRHS = nullptr;
   SourceRange R1(LHS->getBeginLoc());
@@ -144,8 +136,7 @@ void UseIntegerSignComparisonCheck::check(
       RHS->getEndLoc(), 0, *Result.SourceManager, getLangOpts()));
   if (const auto *LHSCast = llvm::dyn_cast<ExplicitCastExpr>(LHS)) {
     SubExprLHS = LHSCast->getSubExpr();
-    R1 = SourceRange(LHS->getBeginLoc(),
-                     SubExprLHS->getBeginLoc().getLocWithOffset(-1));
+    R1.setEnd(SubExprLHS->getBeginLoc().getLocWithOffset(-1));
     R2.setBegin(Lexer::getLocForEndOfToken(
         SubExprLHS->getEndLoc(), 0, *Result.SourceManager, getLangOpts()));
   }
@@ -156,21 +147,21 @@ void UseIntegerSignComparisonCheck::check(
   DiagnosticBuilder Diag =
       diag(BinaryOp->getBeginLoc(),
            "comparison between 'signed' and 'unsigned' integers");
-  std::string CmpNamespace;
-  llvm::StringRef CmpHeader;
+  StringRef CmpNamespace;
+  StringRef CmpHeader;
 
   if (getLangOpts().CPlusPlus20) {
     CmpHeader = "<utility>";
-    CmpNamespace = llvm::Twine("std::" + parseOpCode(OpCode)).str();
+    CmpNamespace = "std::";
   } else if (getLangOpts().CPlusPlus17 && EnableQtSupport) {
     CmpHeader = "<QtCore/q20utility.h>";
-    CmpNamespace = llvm::Twine("q20::" + parseOpCode(OpCode)).str();
+    CmpNamespace = "q20::";
   }
 
   // Prefer modernize-use-integer-sign-comparison when C++20 is available!
   Diag << FixItHint::CreateReplacement(
       CharSourceRange(R1, SubExprLHS != nullptr),
-      llvm::Twine(CmpNamespace + "(").str());
+      Twine(CmpNamespace + parseOpCode(BinaryOp->getOpcode()) + "(").str());
   Diag << FixItHint::CreateReplacement(R2, ",");
   Diag << FixItHint::CreateReplacement(CharSourceRange::getCharRange(R3), ")");
 



More information about the cfe-commits mailing list