https://github.com/qt-tatiana updated
https://github.com/llvm/llvm-project/pull/113144
>From a52c06e25a919f5d84f9ac4c373f4e0f9246b503 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.boris...@qt.io>
Date: Thu, 17 Oct 2024 18:00:08 +0200
Subject: [PATCH] [clang-tidy] Create a check for signed and unsigned integers
comparison
- modernize-use-integer-sign-comparison - replace comparisons between
signed and unsigned integers with their safe C++20 ``std::cmp_*``
alternative, if available.
---
.../clang-tidy/modernize/CMakeLists.txt | 1 +
.../modernize/ModernizeTidyModule.cpp | 3 +
.../UseIntegerSignComparisonCheck.cpp | 171 ++++++++++++++++++
.../modernize/UseIntegerSignComparisonCheck.h | 42 +++++
clang-tools-extra/docs/ReleaseNotes.rst | 8 +-
.../docs/clang-tidy/checks/list.rst | 1 +
.../modernize/use-integer-sign-comparison.rst | 36 ++++
.../modernize/use-integer-sign-comparison.cpp | 122 +++++++++++++
8 files changed, 383 insertions(+), 1 deletion(-)
create mode 100644
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
create mode 100644
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
create mode 100644
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
create mode 100644
clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
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 e8148e06b6af28..c8a419a88c2670 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,10 +123,16 @@ New checks
Gives warnings for tagged unions, where the number of tags is
different 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 0082234f5ed31b..7e6f9180b320b4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -300,6 +300,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
