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.boris...@qt.io> Date: Thu, 2 Jan 2025 18:08:26 +0100 Subject: [PATCH 1/3] [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.boris...@qt.io> Date: Mon, 6 Jan 2025 12:53:55 +0100 Subject: [PATCH 2/3] [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`. >From 9d2fda4d1f07cd0cf07211c518dfe83d05eb64d7 Mon Sep 17 00:00:00 2001 From: Tatiana Borisova <tatiana.boris...@qt.io> Date: Tue, 14 Jan 2025 16:09:36 +0100 Subject: [PATCH 3/3] [clang-tidy] Fix modernize-use-integer-sign-comparison comparison - Delete unneccessary code, rename option to ``CheckIntegerSize``, update ReleaseNotes. --- .../clang-tidy/modernize/ModernizeTidyModule.cpp | 8 -------- .../modernize/UseIntegerSignComparisonCheck.cpp | 13 ++++++------- .../modernize/UseIntegerSignComparisonCheck.h | 2 +- clang-tools-extra/docs/ReleaseNotes.rst | 4 ++-- .../modernize/use-integer-sign-comparison.rst | 7 ++++--- 5 files changed, 13 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index dc7c9c551d668d..fc46c72982fdce 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -126,14 +126,6 @@ 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 9b385702cecdf6..0faeaacafda5b4 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp @@ -81,12 +81,12 @@ UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck( IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", utils::IncludeSorter::IS_LLVM), areDiagsSelfContained()), - ConsideringIntSize(Options.get("ConsideringIntegerSize", true)) {} + CheckIntegerSize(Options.get("CheckIntegerSize", true)) {} void UseIntegerSignComparisonCheck::storeOptions( ClangTidyOptions::OptionMap &Opts) { Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); - Options.store(Opts, "ConsideringIntegerSize", ConsideringIntSize); + Options.store(Opts, "CheckIntegerSize", CheckIntegerSize); } void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) { @@ -141,11 +141,10 @@ void UseIntegerSignComparisonCheck::check( if (LHS == nullptr || RHS == nullptr) return; - if (ConsideringIntSize && - (Result.Context->getTypeSize( - SignedCastExpression->getSubExpr()->getType()) > - Result.Context->getTypeSize( - UnSignedCastExpression->getSubExpr()->getType()))) + if (CheckIntegerSize && (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 0ec34a207e1bbb..67ff1800bbcc92 100644 --- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h +++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h @@ -35,7 +35,7 @@ class UseIntegerSignComparisonCheck : public ClangTidyCheck { private: utils::IncludeInserter IncludeInserter; - const bool ConsideringIntSize; + const bool CheckIntegerSize; }; } // namespace clang::tidy::modernize diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6abd8acb606d88..85c9f35987294e 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -303,8 +303,8 @@ Changes in existing checks - 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. + ignore comparisons between signed wide and unsigned narrow integer types, + configurable via the ``CheckIntegerSize`` option. - Improved :doc:`modernize-use-nullptr <clang-tidy/checks/modernize/use-nullptr>` check to also recognize 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 cf57e55a8f4fa0..4304cada48e886 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 @@ -35,6 +35,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`. +.. option:: CheckIntegerSize + + Ignore comparisons between signed wide and unsigned narrow integer + types. Default is `true`. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits