Author: qt-tatiana Date: 2024-12-12T00:38:05+08:00 New Revision: 8b63bfbf6dd2ad0efd221407755300942a7ca35f
URL: https://github.com/llvm/llvm-project/commit/8b63bfbf6dd2ad0efd221407755300942a7ca35f DIFF: https://github.com/llvm/llvm-project/commit/8b63bfbf6dd2ad0efd221407755300942a7ca35f.diff LOG: [clang-tidy] Create a check for signed and unsigned integers comparison (#113144) - modernize-use-integer-sign-comparison replaces comparisons between signed and unsigned integers with their safe C++20 ``std::cmp_*`` alternative, if available. Added: clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp Modified: clang-tools-extra/clang-tidy/modernize/CMakeLists.txt clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt index c919d49b42873a..bab1167fb15ff2 100644 --- a/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/modernize/CMakeLists.txt @@ -36,6 +36,7 @@ add_clang_library(clangTidyModernizeModule STATIC UseEmplaceCheck.cpp UseEqualsDefaultCheck.cpp UseEqualsDeleteCheck.cpp + UseIntegerSignComparisonCheck.cpp UseNodiscardCheck.cpp UseNoexceptCheck.cpp UseNullptrCheck.cpp diff --git a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp index 18607593320635..fc46c72982fdce 100644 --- a/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp @@ -37,6 +37,7 @@ #include "UseEmplaceCheck.h" #include "UseEqualsDefaultCheck.h" #include "UseEqualsDeleteCheck.h" +#include "UseIntegerSignComparisonCheck.h" #include "UseNodiscardCheck.h" #include "UseNoexceptCheck.h" #include "UseNullptrCheck.h" @@ -76,6 +77,8 @@ class ModernizeModule : public ClangTidyModule { CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value"); CheckFactories.registerCheck<UseDesignatedInitializersCheck>( "modernize-use-designated-initializers"); + CheckFactories.registerCheck<UseIntegerSignComparisonCheck>( + "modernize-use-integer-sign-comparison"); CheckFactories.registerCheck<UseRangesCheck>("modernize-use-ranges"); CheckFactories.registerCheck<UseStartsEndsWithCheck>( "modernize-use-starts-ends-with"); diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp new file mode 100644 index 00000000000000..8f807bc0a96d56 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp @@ -0,0 +1,171 @@ +//===--- UseIntegerSignComparisonCheck.cpp - clang-tidy -------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "UseIntegerSignComparisonCheck.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; +using namespace clang::ast_matchers::internal; + +namespace clang::tidy::modernize { + +/// Find if the passed type is the actual "char" type, +/// not applicable to explicit "signed char" or "unsigned char" types. +static bool isActualCharType(const clang::QualType &Ty) { + using namespace clang; + const Type *DesugaredType = Ty->getUnqualifiedDesugaredType(); + if (const auto *BT = llvm::dyn_cast<BuiltinType>(DesugaredType)) + return (BT->getKind() == BuiltinType::Char_U || + BT->getKind() == BuiltinType::Char_S); + return false; +} + +namespace { +AST_MATCHER(clang::QualType, isActualChar) { + return clang::tidy::modernize::isActualCharType(Node); +} +} // namespace + +static BindableMatcher<clang::Stmt> +intCastExpression(bool IsSigned, + const std::string &CastBindName = std::string()) { + // 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(), + unless(isActualChar()), unless(booleanType()), unless(enumType()))))); + + const auto ImplicitCastExpr = + CastBindName.empty() ? implicitCastExpr(hasSourceExpression(IntTypeExpr)) + : implicitCastExpr(hasSourceExpression(IntTypeExpr)) + .bind(CastBindName); + + const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr)); + const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr)); + const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr)); + + return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr, + FunctionalCastExpr)); +} + +static StringRef parseOpCode(BinaryOperator::Opcode Code) { + switch (Code) { + case BO_LT: + return "cmp_less"; + case BO_GT: + return "cmp_greater"; + case BO_LE: + return "cmp_less_equal"; + case BO_GE: + return "cmp_greater_equal"; + case BO_EQ: + return "cmp_equal"; + case BO_NE: + return "cmp_not_equal"; + default: + return ""; + } +} + +UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + IncludeInserter(Options.getLocalOrGlobal("IncludeStyle", + utils::IncludeSorter::IS_LLVM), + areDiagsSelfContained()) {} + +void UseIntegerSignComparisonCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle()); +} + +void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) { + const auto SignedIntCastExpr = intCastExpression(true, "sIntCastExpression"); + const auto UnSignedIntCastExpr = intCastExpression(false); + + // Flag all operators "==", "<=", ">=", "<", ">", "!=" + // that are used between signed/unsigned + const auto CompareOperator = + binaryOperator(hasAnyOperatorName("==", "<=", ">=", "<", ">", "!="), + hasOperands(SignedIntCastExpr, UnSignedIntCastExpr), + unless(isInTemplateInstantiation())) + .bind("intComparison"); + + Finder->addMatcher(CompareOperator, this); +} + +void UseIntegerSignComparisonCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + IncludeInserter.registerPreprocessor(PP); +} + +void UseIntegerSignComparisonCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *SignedCastExpression = + Result.Nodes.getNodeAs<ImplicitCastExpr>("sIntCastExpression"); + assert(SignedCastExpression); + + // Ignore the match if we know that the signed int value is not negative. + Expr::EvalResult EVResult; + if (!SignedCastExpression->isValueDependent() && + SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult, + *Result.Context)) { + const llvm::APSInt SValue = EVResult.Val.getInt(); + if (SValue.isNonNegative()) + return; + } + + const auto *BinaryOp = + Result.Nodes.getNodeAs<BinaryOperator>("intComparison"); + if (BinaryOp == nullptr) + return; + + const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode(); + + 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 = SourceRange(LHS->getBeginLoc()); + SourceRange R2 = SourceRange(BinaryOp->getOperatorLoc()); + SourceRange R3 = SourceRange(Lexer::getLocForEndOfToken( + 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)); + R2.setBegin(Lexer::getLocForEndOfToken( + SubExprLHS->getEndLoc(), 0, *Result.SourceManager, getLangOpts())); + } + if (const auto *RHSCast = llvm::dyn_cast<ExplicitCastExpr>(RHS)) { + SubExprRHS = RHSCast->getSubExpr(); + R2.setEnd(SubExprRHS->getBeginLoc().getLocWithOffset(-1)); + } + DiagnosticBuilder Diag = + diag(BinaryOp->getBeginLoc(), + "comparison between 'signed' and 'unsigned' integers"); + const std::string CmpNamespace = ("std::" + parseOpCode(OpCode)).str(); + const std::string CmpHeader = "<utility>"; + // Prefer modernize-use-integer-sign-comparison when C++20 is available! + Diag << FixItHint::CreateReplacement( + CharSourceRange(R1, SubExprLHS != nullptr), + llvm::Twine(CmpNamespace + "(").str()); + Diag << FixItHint::CreateReplacement(R2, ","); + Diag << FixItHint::CreateReplacement(CharSourceRange::getCharRange(R3), ")"); + + // If there is no include for cmp_{*} functions, we'll add it. + Diag << IncludeInserter.createIncludeInsertion( + Result.SourceManager->getFileID(BinaryOp->getBeginLoc()), CmpHeader); +} + +} // namespace clang::tidy::modernize diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h new file mode 100644 index 00000000000000..a1074829d6eca5 --- /dev/null +++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h @@ -0,0 +1,42 @@ +//===--- UseIntegerSignComparisonCheck.h - clang-tidy -----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINTEGERSIGNCOMPARISONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINTEGERSIGNCOMPARISONCHECK_H + +#include "../ClangTidyCheck.h" +#include "../utils/IncludeInserter.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +namespace clang::tidy::modernize { + +/// Replace comparisons between signed and unsigned integers with their safe +/// C++20 ``std::cmp_*`` alternative, if available. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-integer-sign-comparison.html +class UseIntegerSignComparisonCheck : public ClangTidyCheck { +public: + UseIntegerSignComparisonCheck(StringRef Name, ClangTidyContext *Context); + + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus20; + } + +private: + utils::IncludeInserter IncludeInserter; +}; + +} // namespace clang::tidy::modernize + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINTEGERSIGNCOMPARISONCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index b2b66dca6ccf85..8603c1d8bf1167 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -136,10 +136,16 @@ New checks Gives warnings for tagged unions, where the number of tags is diff erent from the number of data members inside the union. +- New :doc:`modernize-use-integer-sign-comparison + <clang-tidy/checks/modernize/use-integer-sign-comparison>` check. + + Replace comparisons between signed and unsigned integers with their safe + C++20 ``std::cmp_*`` alternative, if available. + - New :doc:`portability-template-virtual-member-function <clang-tidy/checks/portability/template-virtual-member-function>` check. - Finds cases when an uninstantiated virtual member function in a template class + Finds cases when an uninstantiated virtual member function in a template class causes cross-compiler incompatibility. New check aliases diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index d731b13fc0df44..41f8f958e9e181 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -301,6 +301,7 @@ Clang-Tidy Checks :doc:`modernize-use-emplace <modernize/use-emplace>`, "Yes" :doc:`modernize-use-equals-default <modernize/use-equals-default>`, "Yes" :doc:`modernize-use-equals-delete <modernize/use-equals-delete>`, "Yes" + :doc:`modernize-use-integer-sign-comparison <modernize/use-integer-sign-comparison>`, "Yes" :doc:`modernize-use-nodiscard <modernize/use-nodiscard>`, "Yes" :doc:`modernize-use-noexcept <modernize/use-noexcept>`, "Yes" :doc:`modernize-use-nullptr <modernize/use-nullptr>`, "Yes" 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 new file mode 100644 index 00000000000000..7e2c13b782694f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst @@ -0,0 +1,36 @@ +.. title:: clang-tidy - modernize-use-integer-sign-comparison + +modernize-use-integer-sign-comparison +===================================== + +Replace comparisons between signed and unsigned integers with their safe +C++20 ``std::cmp_*`` alternative, if available. + +The check provides a replacement only for C++20 or later, otherwise +it highlights the problem and expects the user to fix it manually. + +Examples of fixes created by the check: + +.. code-block:: c++ + + unsigned int func(int a, unsigned int b) { + return a == b; + } + +becomes + +.. code-block:: c++ + + #include <utility> + + unsigned int func(int a, unsigned int b) { + return std::cmp_equal(a, b); + } + +Options +------- + +.. option:: IncludeStyle + + A string specifying which include-style is used, `llvm` or `google`. + Default is `llvm`. 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 new file mode 100644 index 00000000000000..99f00444c2d3f3 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp @@ -0,0 +1,122 @@ +// CHECK-FIXES: #include <utility> +// RUN: %check_clang_tidy -std=c++20 %s modernize-use-integer-sign-comparison %t + +// The code that triggers the check +#define MAX_MACRO(a, b) (a < b) ? b : a + +unsigned int FuncParameters(int bla) { + unsigned int result = 0; + if (result == bla) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_equal(result , bla)) + + return 1; +} + +template <typename T> +void TemplateFuncParameter(T val) { + unsigned long uL = 0; + if (val >= uL) + return; +// CHECK-MESSAGES-NOT: warning: +} + +template <typename T1, typename T2> +int TemplateFuncParameters(T1 val1, T2 val2) { + if (val1 >= val2) + return 0; +// CHECK-MESSAGES-NOT: warning: + return 1; +} + +int AllComparisons() { + unsigned int uVar = 42; + unsigned short uArray[7] = {0, 1, 2, 3, 9, 7, 9}; + + int sVar = -42; + short sArray[7] = {-1, -2, -8, -94, -5, -4, -6}; + + enum INT_TEST { + VAL1 = 0, + VAL2 = -1 + }; + + char ch = 'a'; + unsigned char uCh = 'a'; + signed char sCh = 'a'; + bool bln = false; + + if (bln == sVar) + return 0; +// CHECK-MESSAGES-NOT: warning: + + if (ch > uCh) + return 0; +// CHECK-MESSAGES-NOT: warning: + + if (sVar <= INT_TEST::VAL2) + return 0; +// CHECK-MESSAGES-NOT: warning: + + if (uCh < sCh) + return -1; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_less(uCh , sCh)) + + if ((int)uVar < sVar) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_less(uVar, sVar)) + + (uVar != sVar) ? uVar = sVar + : sVar = uVar; +// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: (std::cmp_not_equal(uVar , sVar)) ? uVar = sVar + + while (uArray[0] <= sArray[0]) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:12: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: while (std::cmp_less_equal(uArray[0] , sArray[0])) + + if (uArray[1] > sArray[1]) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_greater(uArray[1] , sArray[1])) + + MAX_MACRO(uVar, sArray[0]); +// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] + + if (static_cast<unsigned int>(uArray[2]) < static_cast<int>(sArray[2])) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_less(uArray[2],sArray[2])) + + if ((unsigned int)uArray[3] < (int)sArray[3]) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_less(uArray[3],sArray[3])) + + if ((unsigned int)(uArray[4]) < (int)(sArray[4])) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_less((uArray[4]),(sArray[4]))) + + if (uArray[5] > sArray[5]) + return 0; +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison] +// CHECK-FIXES: if (std::cmp_greater(uArray[5] , sArray[5])) + + #define VALUE sArray[6] + if (uArray[6] > VALUE) + return 0; +// 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; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
