[clang-tools-extra] [clang-tidy] Fix modernize-use-integer-sign-comparison comparison (PR #121506)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 7 10:06:03 PST 2025
https://github.com/qt-tatiana updated https://github.com/llvm/llvm-project/pull/121506
>From 0591e8b7be49299e2b73634a6ad4f2330557b37a Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 2 Jan 2025 18:08:26 +0100
Subject: [PATCH 1/2] [clang-tidy] Fix modernize-use-integer-sign-comparison
comparison
- modernize-use-integer-sign-comparison should ignore a comparison between
the signed wide type and the unsigned narrow type, see #120867
---
.../UseIntegerSignComparisonCheck.cpp | 14 ++++++++++-
.../modernize/use-integer-sign-comparison.cpp | 24 +++++++++++++++++--
2 files changed, 35 insertions(+), 3 deletions(-)
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);
+}
>From 60cc85f4693a9164159754248313119067037f3c Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Mon, 6 Jan 2025 12:53:55 +0100
Subject: [PATCH 2/2] [clang-tidy] Fix modernize-use-integer-sign-comparison
comparison
- add ``ConsideringIntegerSize`` option, that ignores a comparison
between a signed wide and an unsigned narrow integer types, add
documentation.
---
.../clang-tidy/modernize/ModernizeTidyModule.cpp | 8 ++++++++
.../modernize/UseIntegerSignComparisonCheck.cpp | 13 ++++++++-----
.../modernize/UseIntegerSignComparisonCheck.h | 1 +
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++
.../modernize/use-integer-sign-comparison.rst | 4 ++++
5 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
index fc46c72982fdce..dc7c9c551d668d 100644
--- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
@@ -126,6 +126,14 @@ class ModernizeModule : public ClangTidyModule {
"modernize-use-uncaught-exceptions");
CheckFactories.registerCheck<UseUsingCheck>("modernize-use-using");
}
+
+ ClangTidyOptions getModuleOptions() override {
+ ClangTidyOptions Options;
+ Options.CheckOptions
+ ["modernize-use-integer-sign-comparison.ConsideringIntegerSize"] =
+ "true";
+ return Options;
+ }
};
// Register the ModernizeTidyModule using this statically initialized variable.
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index ebcfafcd391145..9b385702cecdf6 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -80,11 +80,13 @@ UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck(
: ClangTidyCheck(Name, Context),
IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
utils::IncludeSorter::IS_LLVM),
- areDiagsSelfContained()) {}
+ areDiagsSelfContained()),
+ ConsideringIntSize(Options.get("ConsideringIntegerSize", true)) {}
void UseIntegerSignComparisonCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
+ Options.store(Opts, "ConsideringIntegerSize", ConsideringIntSize);
}
void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
@@ -139,10 +141,11 @@ void UseIntegerSignComparisonCheck::check(
if (LHS == nullptr || RHS == nullptr)
return;
- if (Result.Context->getTypeSize(
- SignedCastExpression->getSubExpr()->getType()) >
- Result.Context->getTypeSize(
- UnSignedCastExpression->getSubExpr()->getType()))
+ if (ConsideringIntSize &&
+ (Result.Context->getTypeSize(
+ SignedCastExpression->getSubExpr()->getType()) >
+ Result.Context->getTypeSize(
+ UnSignedCastExpression->getSubExpr()->getType())))
return;
const Expr *SubExprLHS = nullptr;
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
index a1074829d6eca5..0ec34a207e1bbb 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
@@ -35,6 +35,7 @@ class UseIntegerSignComparisonCheck : public ClangTidyCheck {
private:
utils::IncludeInserter IncludeInserter;
+ const bool ConsideringIntSize;
};
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 1fd9b6077be5f5..6abd8acb606d88 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -301,6 +301,11 @@ Changes in existing checks
<clang-tidy/checks/modernize/use-designated-initializers>` check to fix a
crash when a class is declared but not defined.
+- Improved :doc:`modernize-use-integer-sign-comparison
+ <clang-tidy/checks/modernize/use-integer-sign-comparison>` check to
+ add an option ``ConsideringIntegerSize``, that ignores a comparison between
+ a signed wide and an unsigned narrow integer types.
+
- Improved :doc:`modernize-use-nullptr
<clang-tidy/checks/modernize/use-nullptr>` check to also recognize
``NULL``/``__null`` (but not ``0``) when used with a templated type.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
index 7e2c13b782694f..cf57e55a8f4fa0 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
@@ -34,3 +34,7 @@ Options
A string specifying which include-style is used, `llvm` or `google`.
Default is `llvm`.
+
+.. option:: ConsideringIntegerSize
+ Ignores a comparison between a signed wide and an unsigned narrow
+ integer types. Default is `true`.
More information about the cfe-commits
mailing list