[clang-tools-extra] [clang-tidy] Create a check for signed and unsigned integers comparison (PR #113144)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 25 09:22:21 PST 2024
https://github.com/qt-tatiana updated https://github.com/llvm/llvm-project/pull/113144
>From 34ee6f5836efe3acddbdd1c9810af358b8c4f981 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 17 Oct 2024 18:00:08 +0200
Subject: [PATCH 01/12] [clang-tidy] Create a check for signed and unsigned
integers comparison
- modernize-use-integer-sign-comparison check performs comparisons between
signed and unsigned integer types mathematically correct.
If C++20 is supported the check replaces integers comparisons to
std::cmp_{} functions and add <utility> include.
- qt-integer-sign-comparison check works like an alias of
modernize-use-integer-sign-comparison for C++20.
If only C++17 is supported the check replaces integers comparisons to
q20::cmp_{} functions and add <QtCore/q20utility.h> include.
The check assumes the analysed code is Qt-based code.
- add qt module for qt-related checks.
---
clang-tools-extra/clang-tidy/CMakeLists.txt | 2 +
.../clang-tidy/ClangTidyForceLinker.h | 5 +
.../clang-tidy/modernize/CMakeLists.txt | 1 +
.../UseIntegerSignComparisonCheck.cpp | 169 ++++++++++++++++++
.../modernize/UseIntegerSignComparisonCheck.h | 46 +++++
.../clang-tidy/qt/CMakeLists.txt | 26 +++
.../clang-tidy/qt/QtTidyModule.cpp | 46 +++++
clang-tools-extra/docs/ReleaseNotes.rst | 17 ++
.../docs/clang-tidy/checks/list.rst | 3 +
.../modernize/use-integer-sign-comparison.rst | 43 +++++
.../checks/qt/IntegerCrossSignComparison.rst | 64 +++++++
clang-tools-extra/docs/clang-tidy/index.rst | 1 +
.../modernize/use-integer-sign-comparison.cpp | 61 +++++++
.../qt/qt-integer-sign-comparison.cpp | 61 +++++++
14 files changed, 545 insertions(+)
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/clang-tidy/qt/CMakeLists.txt
create mode 100644 clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
create mode 100644 clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 83a3236131dc93..56ef16a8fb37d6 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -75,6 +75,7 @@ add_subdirectory(objc)
add_subdirectory(openmp)
add_subdirectory(performance)
add_subdirectory(portability)
+add_subdirectory(qt)
add_subdirectory(readability)
add_subdirectory(zircon)
set(ALL_CLANG_TIDY_CHECKS
@@ -99,6 +100,7 @@ set(ALL_CLANG_TIDY_CHECKS
clangTidyOpenMPModule
clangTidyPerformanceModule
clangTidyPortabilityModule
+ clangTidyQtModule
clangTidyReadabilityModule
clangTidyZirconModule
)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
index adde9136ff1dd5..3c777f42520223 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -127,6 +127,11 @@ extern volatile int PortabilityModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED PortabilityModuleAnchorDestination =
PortabilityModuleAnchorSource;
+// This anchor is used to force the linker to link the QtClangTidyModule.
+extern volatile int QtClangTidyModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED QtClangTidyModuleAnchorDestination =
+ QtClangTidyModuleAnchorSource;
+
// This anchor is used to force the linker to link the ReadabilityModule.
extern volatile int ReadabilityModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination =
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/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
new file mode 100644
index 00000000000000..8f394a14a9b0c4
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -0,0 +1,169 @@
+//===--- 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 {
+
+UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()),
+ IsQtApplication(Options.get("IsQtApplication", false)) {}
+
+void UseIntegerSignComparisonCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IsQtApplication", IsQtApplication);
+}
+
+void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
+ const auto SignedIntCastExpr = intCastExpression(true, "sIntCastExpression");
+ const auto UnSignedIntCastExpr =
+ intCastExpression(false, "uIntCastExpression");
+
+ // Flag all operators "==", "<=", ">=", "<", ">", "!="
+ // that are used between signed/unsigned
+ const auto CompareOperator =
+ expr(binaryOperator(hasAnyOperatorName("==", "<=", ">=", "<", ">", "!="),
+ anyOf(allOf(hasLHS(SignedIntCastExpr),
+ hasRHS(UnSignedIntCastExpr)),
+ allOf(hasLHS(UnSignedIntCastExpr),
+ hasRHS(SignedIntCastExpr)))))
+ .bind("intComparison");
+
+ Finder->addMatcher(CompareOperator, this);
+}
+
+BindableMatcher<clang::Stmt> UseIntegerSignComparisonCheck::intCastExpression(
+ bool IsSigned, const std::string &CastBindName) const {
+ auto IntTypeExpr = expr();
+ if (IsSigned) {
+ IntTypeExpr = expr(hasType(qualType(isInteger(), isSignedInteger())));
+ } else {
+ IntTypeExpr =
+ expr(hasType(qualType(isInteger(), unless(isSignedInteger()))));
+ }
+
+ const auto ImplicitCastExpr =
+ implicitCastExpr(hasSourceExpression(IntTypeExpr)).bind(CastBindName);
+
+ const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
+ const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
+ const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
+
+ return traverse(TK_AsIs, expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
+ StaticCastExpr, FunctionalCastExpr)));
+}
+
+std::string
+UseIntegerSignComparisonCheck::parseOpCode(BinaryOperator::Opcode code) const {
+ switch (code) {
+ case BO_LT:
+ return std::string("cmp_less");
+ case BO_GT:
+ return std::string("cmp_greater");
+ case BO_LE:
+ return std::string("cmp_less_equal");
+ case BO_GE:
+ return std::string("cmp_greater_equal");
+ case BO_EQ:
+ return std::string("cmp_equal");
+ case BO_NE:
+ return std::string("cmp_not_equal");
+ default:
+ return std::string();
+ }
+}
+
+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");
+ 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;
+ if (!SignedCastExpression->isValueDependent() &&
+ SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
+ *Result.Context)) {
+ llvm::APSInt SValue = EVResult.Val.getInt();
+ if (SValue.isNonNegative())
+ return;
+ }
+
+ const auto *BinaryOp =
+ Result.Nodes.getNodeAs<BinaryOperator>("intComparison");
+ if (BinaryOp == nullptr)
+ return;
+
+ auto OpCode = BinaryOp->getOpcode();
+ const auto *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
+ const auto *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
+ if (LHS == nullptr || RHS == nullptr)
+ return;
+
+ StringRef LHSString(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(LHS->getSourceRange()),
+ *Result.SourceManager, getLangOpts()));
+
+ StringRef RHSString(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(RHS->getSourceRange()),
+ *Result.SourceManager, getLangOpts()));
+
+ DiagnosticBuilder Diag =
+ diag(BinaryOp->getBeginLoc(),
+ "comparison between 'signed' and 'unsigned' integers");
+
+ if (!(getLangOpts().CPlusPlus17 && IsQtApplication) &&
+ !getLangOpts().CPlusPlus20)
+ return;
+
+ std::string CmpNamespace;
+ std::string CmpInclude;
+ if (getLangOpts().CPlusPlus17 && IsQtApplication) {
+ CmpInclude = "<QtCore/q20utility.h>";
+ CmpNamespace = std::string("q20::") + parseOpCode(OpCode);
+ }
+
+ if (getLangOpts().CPlusPlus20) {
+ CmpInclude = "<utility>";
+ CmpNamespace = std::string("std::") + parseOpCode(OpCode);
+ }
+
+ // Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
+ // apps. Prefer modernize-use-integer-sign-comparison when C++20 is available!
+ Diag << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(BinaryOp->getBeginLoc(),
+ BinaryOp->getEndLoc()),
+ StringRef(std::string(CmpNamespace) + std::string("(") +
+ std::string(LHSString) + std::string(", ") +
+ std::string(RHSString) + std::string(")")));
+
+ // If there is no include for cmp_{*} functions, we'll add it.
+ Diag << IncludeInserter.createIncludeInsertion(
+ Result.SourceManager->getFileID(BinaryOp->getBeginLoc()),
+ StringRef(CmpInclude));
+}
+
+} // 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..13c6ea9f74ec32
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
@@ -0,0 +1,46 @@
+//===--- 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 {
+
+/// Class detects comparisons between signed and unsigned integers
+///
+/// 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.CPlusPlus;
+ }
+
+private:
+ ast_matchers::internal::BindableMatcher<clang::Stmt>
+ intCastExpression(bool IsSigned, const std::string &CastBindName) const;
+ std::string parseOpCode(BinaryOperator::Opcode code) const;
+
+ utils::IncludeInserter IncludeInserter;
+ bool IsQtApplication = false;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINTEGERSIGNCOMPARISONCHECK_H
diff --git a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
new file mode 100644
index 00000000000000..5133c47b3031ee
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+ FrontendOpenMP
+ Support
+)
+
+add_clang_library(clangTidyQtModule
+ QtTidyModule.cpp
+
+ LINK_LIBS
+ clangTidy
+ clangTidyModernizeModule
+ clangTidyUtils
+
+ DEPENDS
+ omp_gen
+ ClangDriverOptions
+ )
+
+clang_target_link_libraries(clangTidyQtModule
+ PRIVATE
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTooling
+ )
diff --git a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
new file mode 100644
index 00000000000000..3f2c5ec9a4fa21
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
@@ -0,0 +1,46 @@
+//===-- QtTidyModule.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 "../ClangTidy.h"
+#include "../ClangTidyModule.h"
+#include "../ClangTidyModuleRegistry.h"
+#include "../modernize/UseIntegerSignComparisonCheck.h"
+
+namespace clang::tidy {
+namespace qt {
+
+/// A module containing checks of the C++ Core Guidelines
+class QtClangTidyModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<modernize::UseIntegerSignComparisonCheck>(
+ "qt-integer-sign-comparison");
+ }
+
+ ClangTidyOptions getModuleOptions() override {
+ ClangTidyOptions Options;
+ ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
+
+ Opts["qt-integer-sign-comparison."
+ "IsQtApplication"] = "true";
+
+ return Options;
+ }
+};
+
+// Register the LLVMTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<QtClangTidyModule>
+ X("qt-module", "Adds checks for the Qt framework Guidelines.");
+
+} // namespace qt
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the QtClangTidyModule.
+volatile int QtClangTidyModuleAnchorSource = 0;
+
+} // namespace clang::tidy
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index e8148e06b6af28..ba9922b835d230 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -129,6 +129,15 @@ New checks
Finds cases when an uninstantiated virtual member function in a template class
causes cross-compiler incompatibility.
+- New :doc:`modernize-use-integer-sign-comparison
+ <clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
+
+ Performs comparisons between signed and unsigned integer types
+ mathematically correct. If C++20 is supported a fix-it replaces
+ integers comparisons to
+ std::cmp_{equal,not_equal,{less,greater}{,_equal}} functions.
+
+
New check aliases
^^^^^^^^^^^^^^^^^
@@ -136,6 +145,14 @@ New check aliases
:doc:`bugprone-sizeof-expression
<clang-tidy/checks/bugprone/sizeof-expression>` was added.
+- New alias :doc:`qt-integer-sign-comparison
+ <clang-tidy/checks/qt/integer-sign-comparison>` to
+ doc:`modernize-use-integer-sign-comparison
+ <clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
+ If C++17 is supported, the fix-it replaces integers comparisons to
+ q20::cmp_{equal,not_equal,{less,greater}{,_equal}} functions.
+ The check assumes the analysed code is Qt-based code.
+
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 0082234f5ed31b..f1b9f032c76615 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -30,6 +30,7 @@ Clang-Tidy Checks
openmp/*
performance/*
portability/*
+ qt/*
readability/*
zircon/*
@@ -300,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"
@@ -348,6 +350,7 @@ Clang-Tidy Checks
:doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes"
:doc:`portability-simd-intrinsics <portability/simd-intrinsics>`,
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
+ :doc:`qt-integer-sign-comparison <qt/integer-sign-comparison>`, "Yes"
:doc:`portability-template-virtual-member-function <portability/template-virtual-member-function>`,
:doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes"
:doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`,
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..21bee8ec16b17d
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
@@ -0,0 +1,43 @@
+.. title:: clang-tidy - modernize-use-integer-sign-comparison
+
+modernize-use-integer-sign-comparison
+=====================================
+
+The check detects comparison between signed and unsigned integer values.
+If C++20 is supported, the check suggests a fix-it.
+
+Examples of fixes created by the check:
+
+.. code-block:: c++
+
+ uint func(int bla)
+ {
+ uint result;
+ if (result == bla)
+ return 0;
+
+ return 1;
+ }
+
+becomes
+
+.. code-block:: c++
+
+ #include <utility>
+
+ uint func(int bla)
+ {
+ uint result;
+ if (std::cmp_equal(result, bla))
+ return 0;
+
+ return 1;
+ }
+
+Options
+-------
+
+.. option:: IncludeStyle
+
+ A string specifying which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst b/clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst
new file mode 100644
index 00000000000000..1f9e1089826ce9
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst
@@ -0,0 +1,64 @@
+.. title:: clang-tidy - qt-integer-sign-comparison
+
+qt-integer-sign-comparison
+=============================
+
+The check detects comparison between signed and unsigned integer values.
+If C++20 is supported, the check suggests a std related fix-it.
+If C++17 is supported, the check suggests a Qt related fix-it.
+
+Examples of fixes created by the check:
+
+.. code-block:: c++
+
+ uint func(int bla)
+ {
+ uint result;
+ if (result == bla)
+ return 0;
+
+ return 1;
+ }
+
+in C++17 becomes
+
+.. code-block:: c++
+
+ <QtCore/q20utility.h>
+
+ uint func(int bla)
+ {
+ uint result;
+ if (q20::cmp_equal(result, bla))
+ return 0;
+
+ return 1;
+ }
+
+in C++20 becomes
+
+.. code-block:: c++
+
+ #include <utility>
+
+ uint func(int bla)
+ {
+ uint result;
+ if (std::cmp_equal(result, bla))
+ return 0;
+
+ return 1;
+ }
+
+Options
+-------
+
+.. option:: IncludeStyle
+
+ A string specifying which include-style is used, `llvm` or `google`. Default
+ is `llvm`.
+
+.. option:: IsQtApplication
+
+ When `true` (default `false`), then it is assumed that the code being analyzed
+ is the Qt-based code.
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index a4233d5d8e2694..9663ed03c8734d 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -84,6 +84,7 @@ Name prefix Description
``performance-`` Checks that target performance-related issues.
``portability-`` Checks that target portability-related issues that don't
relate to any particular coding style.
+``qt-`` Checks related to Qt framework.
``readability-`` Checks that target readability-related issues that don't
relate to any particular coding style.
``zircon-`` Checks related to Zircon kernel coding conventions.
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..b580e2dfc6731d
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
@@ -0,0 +1,61 @@
+// 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
+namespace std {
+
+unsigned int FuncParameters(int bla) {
+ unsigned int result;
+ 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 TemplateFuncParameters(T val) {
+ unsigned long uL = 0;
+ if (val >= uL)
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
+// CHECK-FIXES: if (std::cmp_greater_equal(val, uL))
+}
+
+int AllComparisons() {
+ unsigned int uVar = 42;
+ unsigned short uArray[2] = {0, 1};
+
+ int sVar = -42;
+ short sArray[2] = {-1, -2};
+
+ if (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]
+
+ FuncParameters(uVar);
+ TemplateFuncParameters(sVar);
+
+ return 0;
+}
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
new file mode 100644
index 00000000000000..bb9e691e7cb797
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
@@ -0,0 +1,61 @@
+// RUN: %check_clang_tidy -std=c++17 %s qt-integer-sign-comparison %t
+
+// The code that triggers the check
+#define MAX_MACRO(a, b) (a < b) ? b : a
+namespace std {
+
+unsigned int FuncParameters(int bla) {
+ unsigned int result;
+ if (result == bla)
+ return 0;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
+// CHECK-FIXES: if (q20::cmp_equal(result, bla))
+
+ return 1;
+}
+
+template <typename T>
+void TemplateFuncParameters(T val) {
+ unsigned long uL = 0;
+ if (val >= uL)
+ return;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
+// CHECK-FIXES: if (q20::cmp_greater_equal(val, uL))
+}
+
+int AllComparisons() {
+ unsigned int uVar = 42;
+ unsigned short uArray[2] = {0, 1};
+
+ int sVar = -42;
+ short sArray[2] = {-1, -2};
+
+ if (uVar < sVar)
+ return 0;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
+// CHECK-FIXES: if (q20::cmp_less(uVar, sVar))
+
+ (uVar != sVar) ? uVar = sVar
+ : sVar = uVar;
+// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
+// CHECK-FIXES: (q20::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 [qt-integer-sign-comparison]
+// CHECK-FIXES: while (q20::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 [qt-integer-sign-comparison]
+// CHECK-FIXES: if (q20::cmp_greater(uArray[1], sArray[1]))
+
+ MAX_MACRO(uVar, sArray[0]);
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
+
+ FuncParameters(uVar);
+ TemplateFuncParameters(sVar);
+
+ return 0;
+}
+}
>From b013b2c5ee3db7e963fb12f99c0750ca6b4ab36a Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 24 Oct 2024 11:59:23 +0200
Subject: [PATCH 02/12] fixup! [clang-tidy] Create a check for signed and
unsigned integers comparison
---
.../modernize/ModernizeTidyModule.cpp | 3 ++
.../UseIntegerSignComparisonCheck.cpp | 33 +++++++++----------
.../modernize/UseIntegerSignComparisonCheck.h | 1 +
.../clang-tidy/qt/CMakeLists.txt | 4 +--
.../clang-tidy/qt/QtTidyModule.cpp | 8 +++--
clang-tools-extra/docs/ReleaseNotes.rst | 21 ++++++------
.../modernize/use-integer-sign-comparison.rst | 15 +++------
...arison.rst => integer-sign-comparison.rst} | 10 ++++--
8 files changed, 50 insertions(+), 45 deletions(-)
rename clang-tools-extra/docs/clang-tidy/checks/qt/{IntegerCrossSignComparison.rst => integer-sign-comparison.rst} (76%)
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
index 8f394a14a9b0c4..444e3de0e787b9 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -22,11 +22,13 @@ UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck(
IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
utils::IncludeSorter::IS_LLVM),
areDiagsSelfContained()),
- IsQtApplication(Options.get("IsQtApplication", false)) {}
+ IsQtApplication(Options.get("IsQtApplication", false)),
+ StringsMatchHeader(Options.get("StringsMatchHeader", "<utility>")) {}
void UseIntegerSignComparisonCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IsQtApplication", IsQtApplication);
+ Options.store(Opts, "StringsMatchHeader", StringsMatchHeader);
}
void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
@@ -84,7 +86,7 @@ UseIntegerSignComparisonCheck::parseOpCode(BinaryOperator::Opcode code) const {
case BO_NE:
return std::string("cmp_not_equal");
default:
- return std::string();
+ return {};
}
}
@@ -107,7 +109,7 @@ void UseIntegerSignComparisonCheck::check(
if (!SignedCastExpression->isValueDependent() &&
SignedCastExpression->getSubExpr()->EvaluateAsInt(EVResult,
*Result.Context)) {
- llvm::APSInt SValue = EVResult.Val.getInt();
+ const llvm::APSInt SValue = EVResult.Val.getInt();
if (SValue.isNonNegative())
return;
}
@@ -117,18 +119,18 @@ void UseIntegerSignComparisonCheck::check(
if (BinaryOp == nullptr)
return;
- auto OpCode = BinaryOp->getOpcode();
- const auto *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
- const auto *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
- if (LHS == nullptr || RHS == nullptr)
+ const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode();
+ const auto *Lhs = BinaryOp->getLHS()->IgnoreParenImpCasts();
+ const auto *Rhs = BinaryOp->getRHS()->IgnoreParenImpCasts();
+ if (Lhs == nullptr || Rhs == nullptr)
return;
- StringRef LHSString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(LHS->getSourceRange()),
+ const StringRef LhsString(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Lhs->getSourceRange()),
*Result.SourceManager, getLangOpts()));
- StringRef RHSString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(RHS->getSourceRange()),
+ const StringRef RhsString(Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Rhs->getSourceRange()),
*Result.SourceManager, getLangOpts()));
DiagnosticBuilder Diag =
@@ -140,14 +142,11 @@ void UseIntegerSignComparisonCheck::check(
return;
std::string CmpNamespace;
- std::string CmpInclude;
if (getLangOpts().CPlusPlus17 && IsQtApplication) {
- CmpInclude = "<QtCore/q20utility.h>";
CmpNamespace = std::string("q20::") + parseOpCode(OpCode);
}
if (getLangOpts().CPlusPlus20) {
- CmpInclude = "<utility>";
CmpNamespace = std::string("std::") + parseOpCode(OpCode);
}
@@ -157,13 +156,13 @@ void UseIntegerSignComparisonCheck::check(
CharSourceRange::getTokenRange(BinaryOp->getBeginLoc(),
BinaryOp->getEndLoc()),
StringRef(std::string(CmpNamespace) + std::string("(") +
- std::string(LHSString) + std::string(", ") +
- std::string(RHSString) + std::string(")")));
+ std::string(LhsString) + std::string(", ") +
+ std::string(RhsString) + std::string(")")));
// If there is no include for cmp_{*} functions, we'll add it.
Diag << IncludeInserter.createIncludeInsertion(
Result.SourceManager->getFileID(BinaryOp->getBeginLoc()),
- StringRef(CmpInclude));
+ StringsMatchHeader);
}
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
index 13c6ea9f74ec32..44668c0656072f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
@@ -39,6 +39,7 @@ class UseIntegerSignComparisonCheck : public ClangTidyCheck {
utils::IncludeInserter IncludeInserter;
bool IsQtApplication = false;
+ const StringRef StringsMatchHeader;
};
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
index 5133c47b3031ee..9b252515bba3e6 100644
--- a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
@@ -14,7 +14,7 @@ add_clang_library(clangTidyQtModule
DEPENDS
omp_gen
ClangDriverOptions
- )
+)
clang_target_link_libraries(clangTidyQtModule
PRIVATE
@@ -23,4 +23,4 @@ clang_target_link_libraries(clangTidyQtModule
clangBasic
clangLex
clangTooling
- )
+)
diff --git a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
index 3f2c5ec9a4fa21..3f1362129ca9b2 100644
--- a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
@@ -1,4 +1,4 @@
-//===-- QtTidyModule.cpp - clang-tidy ----------------------===//
+//===--- QtTidyModule.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.
@@ -26,8 +26,10 @@ class QtClangTidyModule : public ClangTidyModule {
ClangTidyOptions Options;
ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
- Opts["qt-integer-sign-comparison."
- "IsQtApplication"] = "true";
+ Opts["qt-integer-sign-comparison.IncludeStyle"] = "llvm";
+ Opts["qt-integer-sign-comparison.IsQtApplication"] = "true";
+ Opts["qt-integer-sign-comparison.StringsMatchHeader"] =
+ "<QtCore/q20utility.h>";
return Options;
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index ba9922b835d230..89d16df345f2b5 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,20 +123,20 @@ 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:`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
- causes cross-compiler incompatibility.
-
- New :doc:`modernize-use-integer-sign-comparison
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
Performs comparisons between signed and unsigned integer types
mathematically correct. If C++20 is supported a fix-it replaces
- integers comparisons to
- std::cmp_{equal,not_equal,{less,greater}{,_equal}} functions.
+ integers comparisons to std::cmp_equal, std::cmp_not_equal,
+ std::cmp_less, std::cmp_greater, std::cmp_less_equal and
+ std::cmp_greater_equal functions.
+- 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
+ causes cross-compiler incompatibility.
New check aliases
^^^^^^^^^^^^^^^^^
@@ -147,10 +147,11 @@ New check aliases
- New alias :doc:`qt-integer-sign-comparison
<clang-tidy/checks/qt/integer-sign-comparison>` to
- doc:`modernize-use-integer-sign-comparison
+ :doc:`modernize-use-integer-sign-comparison
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
If C++17 is supported, the fix-it replaces integers comparisons to
- q20::cmp_{equal,not_equal,{less,greater}{,_equal}} functions.
+ q20::cmp_equal, q20::cmp_not_equal, q20::cmp_less, q20::cmp_greater,
+ q20::cmp_less_equal and q20::cmp_greater_equal functions.
The check assumes the analysed code is Qt-based code.
Changes in existing checks
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 21bee8ec16b17d..fa7390304437a8 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
@@ -3,8 +3,11 @@
modernize-use-integer-sign-comparison
=====================================
-The check detects comparison between signed and unsigned integer values.
-If C++20 is supported, the check suggests a fix-it.
+Performs comparisons between signed and unsigned integer types
+mathematically correct. If C++20 is supported a fix-it replaces
+integers comparisons to std::cmp_equal, std::cmp_not_equal,
+std::cmp_less, std::cmp_greater, std::cmp_less_equal and
+std::cmp_greater_equal functions.
Examples of fixes created by the check:
@@ -33,11 +36,3 @@ becomes
return 1;
}
-
-Options
--------
-
-.. option:: IncludeStyle
-
- A string specifying which include-style is used, `llvm` or `google`. Default
- is `llvm`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
similarity index 76%
rename from clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst
rename to clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
index 1f9e1089826ce9..a91fe7f714d9b4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
@@ -3,9 +3,9 @@
qt-integer-sign-comparison
=============================
-The check detects comparison between signed and unsigned integer values.
-If C++20 is supported, the check suggests a std related fix-it.
-If C++17 is supported, the check suggests a Qt related fix-it.
+The qt-integer-sign-comparison check is an alias, please see
+:doc:`modernize-use-integer-sign-comparison <../modernize/use-integer-sign-comparison>`
+for more information.
Examples of fixes created by the check:
@@ -62,3 +62,7 @@ Options
When `true` (default `false`), then it is assumed that the code being analyzed
is the Qt-based code.
+
+.. option:: StringsMatchHeader
+ A string specifying a include header file to be added by fix-it. Default
+ is `<utility>`.
>From d9a6540c72515370f1a32a9c74eba22a8f71df34 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 24 Oct 2024 15:14:19 +0200
Subject: [PATCH 03/12] fixup! fixup! [clang-tidy] Create a check for signed
and unsigned integers comparison
---
.../modernize/UseIntegerSignComparisonCheck.cpp | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 444e3de0e787b9..6604f45a557400 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -15,7 +15,6 @@ using namespace clang::ast_matchers;
using namespace clang::ast_matchers::internal;
namespace clang::tidy::modernize {
-
UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
@@ -120,17 +119,17 @@ void UseIntegerSignComparisonCheck::check(
return;
const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode();
- const auto *Lhs = BinaryOp->getLHS()->IgnoreParenImpCasts();
- const auto *Rhs = BinaryOp->getRHS()->IgnoreParenImpCasts();
- if (Lhs == nullptr || Rhs == nullptr)
+ const auto *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
+ const auto *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
+ if (LHS == nullptr || RHS == nullptr)
return;
const StringRef LhsString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(Lhs->getSourceRange()),
+ CharSourceRange::getTokenRange(LHS->getSourceRange()),
*Result.SourceManager, getLangOpts()));
const StringRef RhsString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(Rhs->getSourceRange()),
+ CharSourceRange::getTokenRange(RHS->getSourceRange()),
*Result.SourceManager, getLangOpts()));
DiagnosticBuilder Diag =
@@ -142,12 +141,10 @@ void UseIntegerSignComparisonCheck::check(
return;
std::string CmpNamespace;
- if (getLangOpts().CPlusPlus17 && IsQtApplication) {
- CmpNamespace = std::string("q20::") + parseOpCode(OpCode);
- }
-
if (getLangOpts().CPlusPlus20) {
CmpNamespace = std::string("std::") + parseOpCode(OpCode);
+ } else if (getLangOpts().CPlusPlus17 && IsQtApplication) {
+ CmpNamespace = std::string("q20::") + parseOpCode(OpCode);
}
// Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
>From c4cbfb717c1005c2ca9441758394ce426158a003 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 24 Oct 2024 17:37:24 +0200
Subject: [PATCH 04/12] fixup! fixup! fixup! [clang-tidy] Create a check for
signed and unsigned integers comparison
---
.../modernize/UseIntegerSignComparisonCheck.cpp | 4 ++--
clang-tools-extra/docs/ReleaseNotes.rst | 11 ++++++-----
.../checks/modernize/use-integer-sign-comparison.rst | 6 +++---
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 6604f45a557400..d81d13edf75c83 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -119,8 +119,8 @@ void UseIntegerSignComparisonCheck::check(
return;
const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode();
- const auto *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
- const auto *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
+ const Expr *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
+ const Expr *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
if (LHS == nullptr || RHS == nullptr)
return;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 89d16df345f2b5..9aa602e9a6a989 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -128,9 +128,9 @@ New checks
Performs comparisons between signed and unsigned integer types
mathematically correct. If C++20 is supported a fix-it replaces
- integers comparisons to std::cmp_equal, std::cmp_not_equal,
- std::cmp_less, std::cmp_greater, std::cmp_less_equal and
- std::cmp_greater_equal functions.
+ integers comparisons to ``std::cmp_equal``, ``std::cmp_not_equal``,
+ ``std::cmp_less``, ``std::cmp_greater``, ``std::cmp_less_equal`` and
+ ``std::cmp_greater_equal`` functions.
- New :doc:`portability-template-virtual-member-function
<clang-tidy/checks/portability/template-virtual-member-function>` check.
@@ -150,8 +150,9 @@ New check aliases
:doc:`modernize-use-integer-sign-comparison
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
If C++17 is supported, the fix-it replaces integers comparisons to
- q20::cmp_equal, q20::cmp_not_equal, q20::cmp_less, q20::cmp_greater,
- q20::cmp_less_equal and q20::cmp_greater_equal functions.
+ ``q20::cmp_equal``, ``q20::cmp_not_equal``, ``q20::cmp_less``,
+ ``q20::cmp_greater``, ``q20::cmp_less_equal`` and ``q20::cmp_greater_equal``
+ functions.
The check assumes the analysed code is Qt-based code.
Changes in existing checks
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 fa7390304437a8..e8edc646c8e2f4 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
@@ -5,9 +5,9 @@ modernize-use-integer-sign-comparison
Performs comparisons between signed and unsigned integer types
mathematically correct. If C++20 is supported a fix-it replaces
-integers comparisons to std::cmp_equal, std::cmp_not_equal,
-std::cmp_less, std::cmp_greater, std::cmp_less_equal and
-std::cmp_greater_equal functions.
+integers comparisons to ``std::cmp_equal``, ``std::cmp_not_equal``,
+``std::cmp_less``, ``std::cmp_greater``, ``std::cmp_less_equal`` and
+``std::cmp_greater_equal`` functions.
Examples of fixes created by the check:
>From 47cffa5f95d40e873872684b910fd4d8807f4f57 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Wed, 30 Oct 2024 10:31:46 +0100
Subject: [PATCH 05/12] fixup! fixup! fixup! fixup! [clang-tidy] Create a check
for signed and unsigned integers comparison
---
.../UseIntegerSignComparisonCheck.cpp | 146 ++++++++++--------
.../modernize/UseIntegerSignComparisonCheck.h | 5 -
.../clang-tidy/qt/CMakeLists.txt | 2 +-
.../clang-tidy/qt/QtTidyModule.cpp | 2 -
clang-tools-extra/docs/ReleaseNotes.rst | 7 +-
.../modernize/use-integer-sign-comparison.rst | 7 +-
.../modernize/use-integer-sign-comparison.cpp | 30 ++--
.../qt/qt-integer-sign-comparison.cpp | 30 ++--
8 files changed, 119 insertions(+), 110 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index d81d13edf75c83..1d9014ad84702f 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -15,19 +15,54 @@ using namespace clang::ast_matchers;
using namespace clang::ast_matchers::internal;
namespace clang::tidy::modernize {
+static BindableMatcher<clang::Stmt> intCastExpression(bool IsSigned,
+ const std::string &CastBindName) {
+ auto IntTypeExpr = IsSigned
+ ? expr(hasType(hasCanonicalType(qualType(isInteger(), isSignedInteger()))))
+ : expr(hasType(hasCanonicalType(qualType(isInteger(), unless(isSignedInteger())))));
+
+ const auto ImplicitCastExpr =
+ 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()),
- IsQtApplication(Options.get("IsQtApplication", false)),
- StringsMatchHeader(Options.get("StringsMatchHeader", "<utility>")) {}
+ IsQtApplication(Options.get("IsQtApplication", false)) {}
void UseIntegerSignComparisonCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
Options.store(Opts, "IsQtApplication", IsQtApplication);
- Options.store(Opts, "StringsMatchHeader", StringsMatchHeader);
}
void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
@@ -38,57 +73,15 @@ void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
// Flag all operators "==", "<=", ">=", "<", ">", "!="
// that are used between signed/unsigned
const auto CompareOperator =
- expr(binaryOperator(hasAnyOperatorName("==", "<=", ">=", "<", ">", "!="),
- anyOf(allOf(hasLHS(SignedIntCastExpr),
- hasRHS(UnSignedIntCastExpr)),
- allOf(hasLHS(UnSignedIntCastExpr),
- hasRHS(SignedIntCastExpr)))))
+ binaryOperator(hasAnyOperatorName("==", "<=", ">=", "<", ">", "!="),
+ hasOperands(SignedIntCastExpr,
+ UnSignedIntCastExpr),
+ unless(isInTemplateInstantiation()))
.bind("intComparison");
Finder->addMatcher(CompareOperator, this);
}
-BindableMatcher<clang::Stmt> UseIntegerSignComparisonCheck::intCastExpression(
- bool IsSigned, const std::string &CastBindName) const {
- auto IntTypeExpr = expr();
- if (IsSigned) {
- IntTypeExpr = expr(hasType(qualType(isInteger(), isSignedInteger())));
- } else {
- IntTypeExpr =
- expr(hasType(qualType(isInteger(), unless(isSignedInteger()))));
- }
-
- const auto ImplicitCastExpr =
- implicitCastExpr(hasSourceExpression(IntTypeExpr)).bind(CastBindName);
-
- const auto CStyleCastExpr = cStyleCastExpr(has(ImplicitCastExpr));
- const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
- const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
-
- return traverse(TK_AsIs, expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
- StaticCastExpr, FunctionalCastExpr)));
-}
-
-std::string
-UseIntegerSignComparisonCheck::parseOpCode(BinaryOperator::Opcode code) const {
- switch (code) {
- case BO_LT:
- return std::string("cmp_less");
- case BO_GT:
- return std::string("cmp_greater");
- case BO_LE:
- return std::string("cmp_less_equal");
- case BO_GE:
- return std::string("cmp_greater_equal");
- case BO_EQ:
- return std::string("cmp_equal");
- case BO_NE:
- return std::string("cmp_not_equal");
- default:
- return {};
- }
-}
-
void UseIntegerSignComparisonCheck::registerPPCallbacks(
const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
IncludeInserter.registerPreprocessor(PP);
@@ -124,42 +117,59 @@ void UseIntegerSignComparisonCheck::check(
if (LHS == nullptr || RHS == nullptr)
return;
- const StringRef LhsString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(LHS->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
-
- const StringRef RhsString(Lexer::getSourceText(
- CharSourceRange::getTokenRange(RHS->getSourceRange()),
- *Result.SourceManager, getLangOpts()));
+ const Expr *ExplicitLHS = nullptr;
+ const Expr *ExplicitRHS = nullptr;
+ StringRef ExplicitLhsString, ExplicitLhsRhsString;
- DiagnosticBuilder Diag =
- diag(BinaryOp->getBeginLoc(),
- "comparison between 'signed' and 'unsigned' integers");
+ DiagnosticBuilder Diag = diag(BinaryOp->getBeginLoc(), "comparison between 'signed' and 'unsigned' integers");
+ if (BinaryOp->getLHS() == SignedCastExpression)
+ {
+ ExplicitLHS = SignedCastExpression->isPartOfExplicitCast() ? SignedCastExpression : nullptr;
+ ExplicitRHS = UnSignedCastExpression->isPartOfExplicitCast() ? UnSignedCastExpression : nullptr;
+ } else {
+ ExplicitRHS = SignedCastExpression->isPartOfExplicitCast() ? SignedCastExpression : nullptr;
+ ExplicitLHS = UnSignedCastExpression->isPartOfExplicitCast() ? UnSignedCastExpression : nullptr;
+ }
if (!(getLangOpts().CPlusPlus17 && IsQtApplication) &&
!getLangOpts().CPlusPlus20)
return;
std::string CmpNamespace;
+ std::string CmpHeader;
if (getLangOpts().CPlusPlus20) {
- CmpNamespace = std::string("std::") + parseOpCode(OpCode);
+ CmpNamespace = std::string("std::") + std::string(parseOpCode(OpCode));
+ CmpHeader = std::string("<utility>");
} else if (getLangOpts().CPlusPlus17 && IsQtApplication) {
- CmpNamespace = std::string("q20::") + parseOpCode(OpCode);
+ CmpNamespace = std::string("q20::") + std::string(parseOpCode(OpCode));
+ CmpHeader = std::string("<QtCore/q20utility.h>");
}
// Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
// apps. Prefer modernize-use-integer-sign-comparison when C++20 is available!
- Diag << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(BinaryOp->getBeginLoc(),
- BinaryOp->getEndLoc()),
- StringRef(std::string(CmpNamespace) + std::string("(") +
- std::string(LhsString) + std::string(", ") +
- std::string(RhsString) + std::string(")")));
+ if (ExplicitLHS) {
+ ExplicitLhsString = Lexer::getSourceText(CharSourceRange::getTokenRange(ExplicitLHS->IgnoreCasts()->getSourceRange()), *Result.SourceManager, getLangOpts());
+ Diag << FixItHint::CreateReplacement(LHS->getSourceRange(), llvm::Twine(CmpNamespace + "(" + ExplicitLhsString).str());
+ } else {
+ Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
+ llvm::Twine(CmpNamespace + "(").str());
+ }
+ Diag << FixItHint::CreateReplacement(BinaryOp->getOperatorLoc(), ", ");
+ if (ExplicitRHS) {
+ ExplicitLhsRhsString = Lexer::getSourceText(CharSourceRange::getTokenRange(ExplicitRHS->IgnoreCasts()->getSourceRange()), *Result.SourceManager, getLangOpts());
+ Diag << FixItHint::CreateReplacement(RHS->getSourceRange(), llvm::Twine(ExplicitLhsRhsString + ")").str());
+ } else {
+ Diag << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(
+ Result.SourceManager->getSpellingLoc(RHS->getEndLoc()), 0,
+ *Result.SourceManager, Result.Context->getLangOpts()),
+ ")");
+ }
// If there is no include for cmp_{*} functions, we'll add it.
Diag << IncludeInserter.createIncludeInsertion(
Result.SourceManager->getFileID(BinaryOp->getBeginLoc()),
- StringsMatchHeader);
+ 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
index 44668c0656072f..a49728564ab237 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
@@ -33,13 +33,8 @@ class UseIntegerSignComparisonCheck : public ClangTidyCheck {
}
private:
- ast_matchers::internal::BindableMatcher<clang::Stmt>
- intCastExpression(bool IsSigned, const std::string &CastBindName) const;
- std::string parseOpCode(BinaryOperator::Opcode code) const;
-
utils::IncludeInserter IncludeInserter;
bool IsQtApplication = false;
- const StringRef StringsMatchHeader;
};
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
index 9b252515bba3e6..dc1be2c1aeea16 100644
--- a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
@@ -3,7 +3,7 @@ set(LLVM_LINK_COMPONENTS
Support
)
-add_clang_library(clangTidyQtModule
+add_clang_library(clangTidyQtModule STATIC
QtTidyModule.cpp
LINK_LIBS
diff --git a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
index 3f1362129ca9b2..2f2008157a44c0 100644
--- a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
@@ -28,8 +28,6 @@ class QtClangTidyModule : public ClangTidyModule {
Opts["qt-integer-sign-comparison.IncludeStyle"] = "llvm";
Opts["qt-integer-sign-comparison.IsQtApplication"] = "true";
- Opts["qt-integer-sign-comparison.StringsMatchHeader"] =
- "<QtCore/q20utility.h>";
return Options;
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 9aa602e9a6a989..8a8e4bbc676aef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -126,11 +126,8 @@ New checks
- New :doc:`modernize-use-integer-sign-comparison
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
- Performs comparisons between signed and unsigned integer types
- mathematically correct. If C++20 is supported a fix-it replaces
- integers comparisons to ``std::cmp_equal``, ``std::cmp_not_equal``,
- ``std::cmp_less``, ``std::cmp_greater``, ``std::cmp_less_equal`` and
- ``std::cmp_greater_equal`` functions.
+ Replace comparisons between signed and unsigned integers with their safe
+ ``std::cmp_*`` alternative.
- New :doc:`portability-template-virtual-member-function
<clang-tidy/checks/portability/template-virtual-member-function>` check.
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 e8edc646c8e2f4..fb35cbc54f3555 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
@@ -3,11 +3,8 @@
modernize-use-integer-sign-comparison
=====================================
-Performs comparisons between signed and unsigned integer types
-mathematically correct. If C++20 is supported a fix-it replaces
-integers comparisons to ``std::cmp_equal``, ``std::cmp_not_equal``,
-``std::cmp_less``, ``std::cmp_greater``, ``std::cmp_less_equal`` and
-``std::cmp_greater_equal`` functions.
+Replace comparisons between signed and unsigned integers with their safe
+``std::cmp_*`` alternative.
Examples of fixes created by the check:
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 b580e2dfc6731d..e93ff4f9be8814 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
@@ -2,25 +2,31 @@
// The code that triggers the check
#define MAX_MACRO(a, b) (a < b) ? b : a
-namespace std {
unsigned int FuncParameters(int bla) {
unsigned int result;
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))
+// CHECK-FIXES: if (std::cmp_equal(result , bla))
return 1;
}
template <typename T>
-void TemplateFuncParameters(T val) {
+void TemplateFuncParameter(T val) {
unsigned long uL = 0;
if (val >= uL)
return;
-// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [modernize-use-integer-sign-comparison]
-// CHECK-FIXES: if (std::cmp_greater_equal(val, uL))
+// 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() {
@@ -30,32 +36,32 @@ int AllComparisons() {
int sVar = -42;
short sArray[2] = {-1, -2};
- if (uVar < sVar)
+ 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))
+// 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
+// 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]))
+// 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]))
+// 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]
FuncParameters(uVar);
- TemplateFuncParameters(sVar);
+ TemplateFuncParameter(sVar);
+ TemplateFuncParameters(uVar, sVar);
return 0;
}
-}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
index bb9e691e7cb797..48aa35fa9e73f8 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
@@ -2,25 +2,31 @@
// The code that triggers the check
#define MAX_MACRO(a, b) (a < b) ? b : a
-namespace std {
unsigned int FuncParameters(int bla) {
unsigned int result;
if (result == bla)
return 0;
// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_equal(result, bla))
+// CHECK-FIXES: if (q20::cmp_equal(result , bla))
return 1;
}
template <typename T>
-void TemplateFuncParameters(T val) {
+void TemplateFuncParameter(T val) {
unsigned long uL = 0;
if (val >= uL)
return;
-// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_greater_equal(val, uL))
+// 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() {
@@ -30,32 +36,32 @@ int AllComparisons() {
int sVar = -42;
short sArray[2] = {-1, -2};
- if (uVar < sVar)
+ if ((int)uVar < sVar)
return 0;
// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_less(uVar, sVar))
+// CHECK-FIXES: if (q20::cmp_less(uVar , sVar))
(uVar != sVar) ? uVar = sVar
: sVar = uVar;
// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: (q20::cmp_not_equal(uVar, sVar)) ? uVar = sVar
+// CHECK-FIXES: (q20::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 [qt-integer-sign-comparison]
-// CHECK-FIXES: while (q20::cmp_less_equal(uArray[0], sArray[0]))
+// CHECK-FIXES: while (q20::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 [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_greater(uArray[1], sArray[1]))
+// CHECK-FIXES: if (q20::cmp_greater(uArray[1] , sArray[1]))
MAX_MACRO(uVar, sArray[0]);
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
FuncParameters(uVar);
- TemplateFuncParameters(sVar);
+ TemplateFuncParameter(sVar);
+ TemplateFuncParameters(uVar, sVar);
return 0;
}
-}
>From 0f320e4fea76fd73cbda9139d3567d30cf98278e Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Wed, 30 Oct 2024 16:14:17 +0100
Subject: [PATCH 06/12] fixup! fixup! fixup! fixup! fixup! [clang-tidy] Create
a check for signed and unsigned integers comparison
---
.../UseIntegerSignComparisonCheck.cpp | 61 ++++++++++++-------
.../modernize/use-integer-sign-comparison.rst | 8 +++
.../checks/qt/integer-sign-comparison.rst | 8 +--
3 files changed, 49 insertions(+), 28 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 1d9014ad84702f..cd073f048cc629 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -15,11 +15,12 @@ using namespace clang::ast_matchers;
using namespace clang::ast_matchers::internal;
namespace clang::tidy::modernize {
-static BindableMatcher<clang::Stmt> intCastExpression(bool IsSigned,
- const std::string &CastBindName) {
- auto IntTypeExpr = IsSigned
- ? expr(hasType(hasCanonicalType(qualType(isInteger(), isSignedInteger()))))
- : expr(hasType(hasCanonicalType(qualType(isInteger(), unless(isSignedInteger())))));
+static BindableMatcher<clang::Stmt>
+intCastExpression(bool IsSigned, const std::string &CastBindName) {
+ auto IntTypeExpr = IsSigned ? expr(hasType(hasCanonicalType(
+ qualType(isInteger(), isSignedInteger()))))
+ : expr(hasType(hasCanonicalType(qualType(
+ isInteger(), unless(isSignedInteger())))));
const auto ImplicitCastExpr =
implicitCastExpr(hasSourceExpression(IntTypeExpr)).bind(CastBindName);
@@ -28,8 +29,8 @@ static BindableMatcher<clang::Stmt> intCastExpression(bool IsSigned,
const auto StaticCastExpr = cxxStaticCastExpr(has(ImplicitCastExpr));
const auto FunctionalCastExpr = cxxFunctionalCastExpr(has(ImplicitCastExpr));
- return expr(anyOf(ImplicitCastExpr, CStyleCastExpr,
- StaticCastExpr, FunctionalCastExpr));
+ return expr(anyOf(ImplicitCastExpr, CStyleCastExpr, StaticCastExpr,
+ FunctionalCastExpr));
}
static StringRef parseOpCode(BinaryOperator::Opcode Code) {
@@ -74,8 +75,7 @@ void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
// that are used between signed/unsigned
const auto CompareOperator =
binaryOperator(hasAnyOperatorName("==", "<=", ">=", "<", ">", "!="),
- hasOperands(SignedIntCastExpr,
- UnSignedIntCastExpr),
+ hasOperands(SignedIntCastExpr, UnSignedIntCastExpr),
unless(isInTemplateInstantiation()))
.bind("intComparison");
@@ -121,14 +121,23 @@ void UseIntegerSignComparisonCheck::check(
const Expr *ExplicitRHS = nullptr;
StringRef ExplicitLhsString, ExplicitLhsRhsString;
- DiagnosticBuilder Diag = diag(BinaryOp->getBeginLoc(), "comparison between 'signed' and 'unsigned' integers");
- if (BinaryOp->getLHS() == SignedCastExpression)
- {
- ExplicitLHS = SignedCastExpression->isPartOfExplicitCast() ? SignedCastExpression : nullptr;
- ExplicitRHS = UnSignedCastExpression->isPartOfExplicitCast() ? UnSignedCastExpression : nullptr;
+ DiagnosticBuilder Diag =
+ diag(BinaryOp->getBeginLoc(),
+ "comparison between 'signed' and 'unsigned' integers");
+ if (BinaryOp->getLHS() == SignedCastExpression) {
+ ExplicitLHS = SignedCastExpression->isPartOfExplicitCast()
+ ? SignedCastExpression
+ : nullptr;
+ ExplicitRHS = UnSignedCastExpression->isPartOfExplicitCast()
+ ? UnSignedCastExpression
+ : nullptr;
} else {
- ExplicitRHS = SignedCastExpression->isPartOfExplicitCast() ? SignedCastExpression : nullptr;
- ExplicitLHS = UnSignedCastExpression->isPartOfExplicitCast() ? UnSignedCastExpression : nullptr;
+ ExplicitRHS = SignedCastExpression->isPartOfExplicitCast()
+ ? SignedCastExpression
+ : nullptr;
+ ExplicitLHS = UnSignedCastExpression->isPartOfExplicitCast()
+ ? UnSignedCastExpression
+ : nullptr;
}
if (!(getLangOpts().CPlusPlus17 && IsQtApplication) &&
@@ -148,16 +157,25 @@ void UseIntegerSignComparisonCheck::check(
// Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
// apps. Prefer modernize-use-integer-sign-comparison when C++20 is available!
if (ExplicitLHS) {
- ExplicitLhsString = Lexer::getSourceText(CharSourceRange::getTokenRange(ExplicitLHS->IgnoreCasts()->getSourceRange()), *Result.SourceManager, getLangOpts());
- Diag << FixItHint::CreateReplacement(LHS->getSourceRange(), llvm::Twine(CmpNamespace + "(" + ExplicitLhsString).str());
+ ExplicitLhsString =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(
+ ExplicitLHS->IgnoreCasts()->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+ Diag << FixItHint::CreateReplacement(
+ LHS->getSourceRange(),
+ llvm::Twine(CmpNamespace + "(" + ExplicitLhsString).str());
} else {
Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
llvm::Twine(CmpNamespace + "(").str());
}
Diag << FixItHint::CreateReplacement(BinaryOp->getOperatorLoc(), ", ");
if (ExplicitRHS) {
- ExplicitLhsRhsString = Lexer::getSourceText(CharSourceRange::getTokenRange(ExplicitRHS->IgnoreCasts()->getSourceRange()), *Result.SourceManager, getLangOpts());
- Diag << FixItHint::CreateReplacement(RHS->getSourceRange(), llvm::Twine(ExplicitLhsRhsString + ")").str());
+ ExplicitLhsRhsString =
+ Lexer::getSourceText(CharSourceRange::getTokenRange(
+ ExplicitRHS->IgnoreCasts()->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+ Diag << FixItHint::CreateReplacement(
+ RHS->getSourceRange(), llvm::Twine(ExplicitLhsRhsString + ")").str());
} else {
Diag << FixItHint::CreateInsertion(
Lexer::getLocForEndOfToken(
@@ -168,8 +186,7 @@ void UseIntegerSignComparisonCheck::check(
// If there is no include for cmp_{*} functions, we'll add it.
Diag << IncludeInserter.createIncludeInsertion(
- Result.SourceManager->getFileID(BinaryOp->getBeginLoc()),
- CmpHeader);
+ Result.SourceManager->getFileID(BinaryOp->getBeginLoc()), CmpHeader);
}
} // namespace clang::tidy::modernize
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 fb35cbc54f3555..b61a6c5a079e02 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
@@ -33,3 +33,11 @@ becomes
return 1;
}
+
+Options
+-------
+
+.. option:: IncludeStyle
+
+ A string specifying which include-style is used, `llvm` or `google`.
+ Default is `llvm`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
index a91fe7f714d9b4..753cd952286844 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
@@ -55,14 +55,10 @@ Options
.. option:: IncludeStyle
- A string specifying which include-style is used, `llvm` or `google`. Default
- is `llvm`.
+ A string specifying which include-style is used, `llvm` or `google`.
+ Default is `llvm`.
.. option:: IsQtApplication
When `true` (default `false`), then it is assumed that the code being analyzed
is the Qt-based code.
-
-.. option:: StringsMatchHeader
- A string specifying a include header file to be added by fix-it. Default
- is `<utility>`.
>From be56a624cb600cc76bf4f92ff157a98cd43db3d0 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Wed, 30 Oct 2024 16:19:40 +0100
Subject: [PATCH 07/12] fixup! fixup! fixup! fixup! fixup! fixup! [clang-tidy]
Create a check for signed and unsigned integers comparison
---
.../docs/clang-tidy/checks/qt/integer-sign-comparison.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
index 753cd952286844..7a6e0925beed08 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
@@ -60,5 +60,5 @@ Options
.. option:: IsQtApplication
- When `true` (default `false`), then it is assumed that the code being analyzed
- is the Qt-based code.
+ When `true`, then it is assumed that the code being analyzed is the Qt-based code.
+ Default is `false`.
>From ce5486b66d3daeea70549551648ad8382d5ae7c0 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Sat, 2 Nov 2024 22:40:35 +0100
Subject: [PATCH 08/12] fixup! fixup! fixup! fixup! fixup! fixup! fixup!
[clang-tidy] Create a check for signed and unsigned integers comparison
---
.../UseIntegerSignComparisonCheck.cpp | 94 +++++++++----------
.../clang-tidy/qt/QtTidyModule.cpp | 6 +-
clang-tools-extra/docs/ReleaseNotes.rst | 10 +-
.../modernize/use-integer-sign-comparison.rst | 32 +++----
.../checks/qt/integer-sign-comparison.rst | 38 +++-----
clang-tools-extra/docs/clang-tidy/index.rst | 2 +-
.../modernize/use-integer-sign-comparison.cpp | 2 +-
.../qt/qt-integer-sign-comparison.cpp | 6 +-
8 files changed, 85 insertions(+), 105 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index cd073f048cc629..8f8fcdff465d67 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -17,10 +17,8 @@ using namespace clang::ast_matchers::internal;
namespace clang::tidy::modernize {
static BindableMatcher<clang::Stmt>
intCastExpression(bool IsSigned, const std::string &CastBindName) {
- auto IntTypeExpr = IsSigned ? expr(hasType(hasCanonicalType(
- qualType(isInteger(), isSignedInteger()))))
- : expr(hasType(hasCanonicalType(qualType(
- isInteger(), unless(isSignedInteger())))));
+ auto IntTypeExpr = expr(hasType(hasCanonicalType(qualType(
+ isInteger(), IsSigned ? isSignedInteger() : unless(isSignedInteger())))));
const auto ImplicitCastExpr =
implicitCastExpr(hasSourceExpression(IntTypeExpr)).bind(CastBindName);
@@ -117,28 +115,32 @@ void UseIntegerSignComparisonCheck::check(
if (LHS == nullptr || RHS == nullptr)
return;
- const Expr *ExplicitLHS = nullptr;
- const Expr *ExplicitRHS = nullptr;
- StringRef ExplicitLhsString, ExplicitLhsRhsString;
+ const Expr *SubExprLHS = nullptr;
+ const Expr *SubExprRHS = nullptr;
+ SourceRange R1 = SourceRange(LHS->getBeginLoc());
+ SourceRange R2 = SourceRange(BinaryOp->getOperatorLoc());
+
+ if (const auto *LHSCast = BinaryOp->getLHS() == SignedCastExpression
+ ? SignedCastExpression
+ : UnSignedCastExpression) {
+ if (LHSCast->isPartOfExplicitCast()) {
+ SubExprLHS = LHSCast->getSubExpr();
+ R1 = SourceRange(LHS->getBeginLoc(), SubExprLHS->getBeginLoc());
+ }
+ }
+
+ if (const auto *RHSCast = BinaryOp->getLHS() == SignedCastExpression
+ ? UnSignedCastExpression
+ : SignedCastExpression) {
+ if (RHSCast->isPartOfExplicitCast()) {
+ SubExprRHS = RHSCast->getSubExpr();
+ R2.setEnd(SubExprRHS->getBeginLoc());
+ }
+ }
DiagnosticBuilder Diag =
diag(BinaryOp->getBeginLoc(),
"comparison between 'signed' and 'unsigned' integers");
- if (BinaryOp->getLHS() == SignedCastExpression) {
- ExplicitLHS = SignedCastExpression->isPartOfExplicitCast()
- ? SignedCastExpression
- : nullptr;
- ExplicitRHS = UnSignedCastExpression->isPartOfExplicitCast()
- ? UnSignedCastExpression
- : nullptr;
- } else {
- ExplicitRHS = SignedCastExpression->isPartOfExplicitCast()
- ? SignedCastExpression
- : nullptr;
- ExplicitLHS = UnSignedCastExpression->isPartOfExplicitCast()
- ? UnSignedCastExpression
- : nullptr;
- }
if (!(getLangOpts().CPlusPlus17 && IsQtApplication) &&
!getLangOpts().CPlusPlus20)
@@ -147,42 +149,40 @@ void UseIntegerSignComparisonCheck::check(
std::string CmpNamespace;
std::string CmpHeader;
if (getLangOpts().CPlusPlus20) {
- CmpNamespace = std::string("std::") + std::string(parseOpCode(OpCode));
- CmpHeader = std::string("<utility>");
+ CmpNamespace = llvm::Twine("std::" + parseOpCode(OpCode)).str();
+ CmpHeader = "<utility>";
} else if (getLangOpts().CPlusPlus17 && IsQtApplication) {
- CmpNamespace = std::string("q20::") + std::string(parseOpCode(OpCode));
- CmpHeader = std::string("<QtCore/q20utility.h>");
+ CmpNamespace = llvm::Twine("q20::" + parseOpCode(OpCode)).str();
+ CmpHeader = "<QtCore/q20utility.h>";
}
// Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
// apps. Prefer modernize-use-integer-sign-comparison when C++20 is available!
- if (ExplicitLHS) {
- ExplicitLhsString =
+ if (SubExprLHS) {
+ StringRef ExplicitLhsString =
Lexer::getSourceText(CharSourceRange::getTokenRange(
- ExplicitLHS->IgnoreCasts()->getSourceRange()),
+ SubExprLHS->IgnoreCasts()->getSourceRange()),
*Result.SourceManager, getLangOpts());
Diag << FixItHint::CreateReplacement(
- LHS->getSourceRange(),
- llvm::Twine(CmpNamespace + "(" + ExplicitLhsString).str());
+ R1, llvm::Twine(CmpNamespace + "(" + ExplicitLhsString).str());
} else {
- Diag << FixItHint::CreateInsertion(LHS->getBeginLoc(),
+ Diag << FixItHint::CreateInsertion(R1.getBegin(),
llvm::Twine(CmpNamespace + "(").str());
}
- Diag << FixItHint::CreateReplacement(BinaryOp->getOperatorLoc(), ", ");
- if (ExplicitRHS) {
- ExplicitLhsRhsString =
- Lexer::getSourceText(CharSourceRange::getTokenRange(
- ExplicitRHS->IgnoreCasts()->getSourceRange()),
- *Result.SourceManager, getLangOpts());
- Diag << FixItHint::CreateReplacement(
- RHS->getSourceRange(), llvm::Twine(ExplicitLhsRhsString + ")").str());
- } else {
- Diag << FixItHint::CreateInsertion(
- Lexer::getLocForEndOfToken(
- Result.SourceManager->getSpellingLoc(RHS->getEndLoc()), 0,
- *Result.SourceManager, Result.Context->getLangOpts()),
- ")");
- }
+
+ StringRef ExplicitLhsRhsString =
+ SubExprRHS ? Lexer::getSourceText(
+ CharSourceRange::getTokenRange(
+ SubExprRHS->IgnoreCasts()->getSourceRange()),
+ *Result.SourceManager, getLangOpts())
+ : "";
+ Diag << FixItHint::CreateReplacement(
+ R2, llvm::Twine(", " + ExplicitLhsRhsString).str());
+ Diag << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(
+ Result.SourceManager->getSpellingLoc(RHS->getEndLoc()), 0,
+ *Result.SourceManager, Result.Context->getLangOpts()),
+ ")");
// If there is no include for cmp_{*} functions, we'll add it.
Diag << IncludeInserter.createIncludeInsertion(
diff --git a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
index 2f2008157a44c0..2aabe9ed427cc3 100644
--- a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
@@ -14,7 +14,7 @@
namespace clang::tidy {
namespace qt {
-/// A module containing checks of the C++ Core Guidelines
+/// A module containing checks of the QT
class QtClangTidyModule : public ClangTidyModule {
public:
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
@@ -33,9 +33,9 @@ class QtClangTidyModule : public ClangTidyModule {
}
};
-// Register the LLVMTidyModule using this statically initialized variable.
+// Register the QtClangTidyModule using this statically initialized variable.
static ClangTidyModuleRegistry::Add<QtClangTidyModule>
- X("qt-module", "Adds checks for the Qt framework Guidelines.");
+ X("qt-module", "Adds checks for the QT.");
} // namespace qt
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 8a8e4bbc676aef..b57f63716f48b4 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -127,7 +127,7 @@ New checks
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
Replace comparisons between signed and unsigned integers with their safe
- ``std::cmp_*`` alternative.
+ C++20 ``std::cmp_*`` alternative, if available.
- New :doc:`portability-template-virtual-member-function
<clang-tidy/checks/portability/template-virtual-member-function>` check.
@@ -146,11 +146,9 @@ New check aliases
<clang-tidy/checks/qt/integer-sign-comparison>` to
:doc:`modernize-use-integer-sign-comparison
<clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
- If C++17 is supported, the fix-it replaces integers comparisons to
- ``q20::cmp_equal``, ``q20::cmp_not_equal``, ``q20::cmp_less``,
- ``q20::cmp_greater``, ``q20::cmp_less_equal`` and ``q20::cmp_greater_equal``
- functions.
- The check assumes the analysed code is Qt-based code.
+
+ Replace comparisons between signed and unsigned integers with their safe
+ C++17 ``q20::cmp_*`` or C++20 ``std::cmp_*`` alternatives, if available.
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
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 b61a6c5a079e02..3fab41f7787285 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
@@ -4,35 +4,26 @@ modernize-use-integer-sign-comparison
=====================================
Replace comparisons between signed and unsigned integers with their safe
-``std::cmp_*`` alternative.
+C++20 ``std::cmp_*`` alternative, if available.
+
Examples of fixes created by the check:
.. code-block:: c++
- uint func(int bla)
- {
- uint result;
- if (result == bla)
- return 0;
-
- return 1;
- }
+ uint func(int a, uint b) {
+ return a == b;
+ }
becomes
.. code-block:: c++
- #include <utility>
-
- uint func(int bla)
- {
- uint result;
- if (std::cmp_equal(result, bla))
- return 0;
+ #include <utility>
- return 1;
- }
+ uint func(int a, uint b) {
+ return (std::cmp_equal(result, bla))
+ }
Options
-------
@@ -41,3 +32,8 @@ Options
A string specifying which include-style is used, `llvm` or `google`.
Default is `llvm`.
+
+.. option:: IsQtApplication
+
+ When `true`, it is assumed that the code being analyzed is using the Qt framework.
+ Default is `false`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
index 7a6e0925beed08..fe35b7345f4210 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
@@ -11,44 +11,30 @@ Examples of fixes created by the check:
.. code-block:: c++
- uint func(int bla)
- {
- uint result;
- if (result == bla)
- return 0;
-
- return 1;
- }
+ uint func(int a, uint b) {
+ return a == b;
+ }
in C++17 becomes
.. code-block:: c++
- <QtCore/q20utility.h>
-
- uint func(int bla)
- {
- uint result;
- if (q20::cmp_equal(result, bla))
- return 0;
+ <QtCore/q20utility.h>
- return 1;
- }
+ uint func(int a, uint b) {
+ return (q20::cmp_equal(result, bla))
+ }
in C++20 becomes
.. code-block:: c++
- #include <utility>
+ #include <utility>
- uint func(int bla)
- {
- uint result;
- if (std::cmp_equal(result, bla))
- return 0;
+ uint func(int a, uint b) {
+ return (std::cmp_equal(result, bla))
+ }
- return 1;
- }
Options
-------
@@ -60,5 +46,5 @@ Options
.. option:: IsQtApplication
- When `true`, then it is assumed that the code being analyzed is the Qt-based code.
+ When `true`, it is assumed that the code being analyzed is using the Qt framework.
Default is `false`.
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index 9663ed03c8734d..e0fc8ab73a5931 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -84,7 +84,7 @@ Name prefix Description
``performance-`` Checks that target performance-related issues.
``portability-`` Checks that target portability-related issues that don't
relate to any particular coding style.
-``qt-`` Checks related to Qt framework.
+``qt-`` Checks related to the Qt framework.
``readability-`` Checks that target readability-related issues that don't
relate to any particular coding style.
``zircon-`` Checks related to Zircon kernel coding conventions.
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 e93ff4f9be8814..4626a7f3a8a3bc 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
@@ -4,7 +4,7 @@
#define MAX_MACRO(a, b) (a < b) ? b : a
unsigned int FuncParameters(int bla) {
- unsigned int result;
+ 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]
diff --git a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
index 48aa35fa9e73f8..07e977d5c1b2ee 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
@@ -4,7 +4,7 @@
#define MAX_MACRO(a, b) (a < b) ? b : a
unsigned int FuncParameters(int bla) {
- unsigned int result;
+ unsigned int result = 0;
if (result == bla)
return 0;
// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
@@ -36,10 +36,10 @@ int AllComparisons() {
int sVar = -42;
short sArray[2] = {-1, -2};
- if ((int)uVar < sVar)
+ if (uVar < (unsigned int)sVar)
return 0;
// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_less(uVar , sVar))
+// CHECK-FIXES: if (q20::cmp_less(uVar , sVar))
(uVar != sVar) ? uVar = sVar
: sVar = uVar;
>From 0d35af049a6f73e0af5fd53bcf117cc084c8252e Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Mon, 11 Nov 2024 16:16:08 +0100
Subject: [PATCH 09/12] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup!
[clang-tidy] Create a check for signed and unsigned integers comparison
---
clang-tools-extra/clang-tidy/CMakeLists.txt | 2 -
.../clang-tidy/ClangTidyForceLinker.h | 5 --
.../UseIntegerSignComparisonCheck.cpp | 22 ++----
.../modernize/UseIntegerSignComparisonCheck.h | 1 -
.../clang-tidy/qt/CMakeLists.txt | 26 -------
.../clang-tidy/qt/QtTidyModule.cpp | 46 -------------
clang-tools-extra/docs/ReleaseNotes.rst | 8 ---
.../docs/clang-tidy/checks/list.rst | 2 -
.../modernize/use-integer-sign-comparison.rst | 5 --
.../checks/qt/integer-sign-comparison.rst | 50 --------------
clang-tools-extra/docs/clang-tidy/index.rst | 1 -
.../qt/qt-integer-sign-comparison.cpp | 67 -------------------
12 files changed, 6 insertions(+), 229 deletions(-)
delete mode 100644 clang-tools-extra/clang-tidy/qt/CMakeLists.txt
delete mode 100644 clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
delete mode 100644 clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt
index 56ef16a8fb37d6..83a3236131dc93 100644
--- a/clang-tools-extra/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -75,7 +75,6 @@ add_subdirectory(objc)
add_subdirectory(openmp)
add_subdirectory(performance)
add_subdirectory(portability)
-add_subdirectory(qt)
add_subdirectory(readability)
add_subdirectory(zircon)
set(ALL_CLANG_TIDY_CHECKS
@@ -100,7 +99,6 @@ set(ALL_CLANG_TIDY_CHECKS
clangTidyOpenMPModule
clangTidyPerformanceModule
clangTidyPortabilityModule
- clangTidyQtModule
clangTidyReadabilityModule
clangTidyZirconModule
)
diff --git a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
index 3c777f42520223..adde9136ff1dd5 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
@@ -127,11 +127,6 @@ extern volatile int PortabilityModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED PortabilityModuleAnchorDestination =
PortabilityModuleAnchorSource;
-// This anchor is used to force the linker to link the QtClangTidyModule.
-extern volatile int QtClangTidyModuleAnchorSource;
-static int LLVM_ATTRIBUTE_UNUSED QtClangTidyModuleAnchorDestination =
- QtClangTidyModuleAnchorSource;
-
// This anchor is used to force the linker to link the ReadabilityModule.
extern volatile int ReadabilityModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED ReadabilityModuleAnchorDestination =
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 8f8fcdff465d67..0ecce36eeff65e 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -55,13 +55,11 @@ UseIntegerSignComparisonCheck::UseIntegerSignComparisonCheck(
: ClangTidyCheck(Name, Context),
IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
utils::IncludeSorter::IS_LLVM),
- areDiagsSelfContained()),
- IsQtApplication(Options.get("IsQtApplication", false)) {}
+ areDiagsSelfContained()) {}
void UseIntegerSignComparisonCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
- Options.store(Opts, "IsQtApplication", IsQtApplication);
}
void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
@@ -142,22 +140,14 @@ void UseIntegerSignComparisonCheck::check(
diag(BinaryOp->getBeginLoc(),
"comparison between 'signed' and 'unsigned' integers");
- if (!(getLangOpts().CPlusPlus17 && IsQtApplication) &&
- !getLangOpts().CPlusPlus20)
+ if (!getLangOpts().CPlusPlus20)
return;
- std::string CmpNamespace;
- std::string CmpHeader;
- if (getLangOpts().CPlusPlus20) {
- CmpNamespace = llvm::Twine("std::" + parseOpCode(OpCode)).str();
- CmpHeader = "<utility>";
- } else if (getLangOpts().CPlusPlus17 && IsQtApplication) {
- CmpNamespace = llvm::Twine("q20::" + parseOpCode(OpCode)).str();
- CmpHeader = "<QtCore/q20utility.h>";
- }
+ const std::string CmpNamespace =
+ llvm::Twine("std::" + parseOpCode(OpCode)).str();
+ const std::string CmpHeader = "<utility>";
- // Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
- // apps. Prefer modernize-use-integer-sign-comparison when C++20 is available!
+ // Prefer modernize-use-integer-sign-comparison when C++20 is available!
if (SubExprLHS) {
StringRef ExplicitLhsString =
Lexer::getSourceText(CharSourceRange::getTokenRange(
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
index a49728564ab237..7bc79603d0857b 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
@@ -34,7 +34,6 @@ class UseIntegerSignComparisonCheck : public ClangTidyCheck {
private:
utils::IncludeInserter IncludeInserter;
- bool IsQtApplication = false;
};
} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt b/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
deleted file mode 100644
index dc1be2c1aeea16..00000000000000
--- a/clang-tools-extra/clang-tidy/qt/CMakeLists.txt
+++ /dev/null
@@ -1,26 +0,0 @@
-set(LLVM_LINK_COMPONENTS
- FrontendOpenMP
- Support
-)
-
-add_clang_library(clangTidyQtModule STATIC
- QtTidyModule.cpp
-
- LINK_LIBS
- clangTidy
- clangTidyModernizeModule
- clangTidyUtils
-
- DEPENDS
- omp_gen
- ClangDriverOptions
-)
-
-clang_target_link_libraries(clangTidyQtModule
- PRIVATE
- clangAST
- clangASTMatchers
- clangBasic
- clangLex
- clangTooling
-)
diff --git a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp b/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
deleted file mode 100644
index 2aabe9ed427cc3..00000000000000
--- a/clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp
+++ /dev/null
@@ -1,46 +0,0 @@
-//===--- QtTidyModule.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 "../ClangTidy.h"
-#include "../ClangTidyModule.h"
-#include "../ClangTidyModuleRegistry.h"
-#include "../modernize/UseIntegerSignComparisonCheck.h"
-
-namespace clang::tidy {
-namespace qt {
-
-/// A module containing checks of the QT
-class QtClangTidyModule : public ClangTidyModule {
-public:
- void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
- CheckFactories.registerCheck<modernize::UseIntegerSignComparisonCheck>(
- "qt-integer-sign-comparison");
- }
-
- ClangTidyOptions getModuleOptions() override {
- ClangTidyOptions Options;
- ClangTidyOptions::OptionMap &Opts = Options.CheckOptions;
-
- Opts["qt-integer-sign-comparison.IncludeStyle"] = "llvm";
- Opts["qt-integer-sign-comparison.IsQtApplication"] = "true";
-
- return Options;
- }
-};
-
-// Register the QtClangTidyModule using this statically initialized variable.
-static ClangTidyModuleRegistry::Add<QtClangTidyModule>
- X("qt-module", "Adds checks for the QT.");
-
-} // namespace qt
-
-// This anchor is used to force the linker to link in the generated object file
-// and thus register the QtClangTidyModule.
-volatile int QtClangTidyModuleAnchorSource = 0;
-
-} // namespace clang::tidy
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b57f63716f48b4..c8a419a88c2670 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -142,14 +142,6 @@ New check aliases
:doc:`bugprone-sizeof-expression
<clang-tidy/checks/bugprone/sizeof-expression>` was added.
-- New alias :doc:`qt-integer-sign-comparison
- <clang-tidy/checks/qt/integer-sign-comparison>` to
- :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++17 ``q20::cmp_*`` or C++20 ``std::cmp_*`` alternatives, if available.
-
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index f1b9f032c76615..7e6f9180b320b4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -30,7 +30,6 @@ Clang-Tidy Checks
openmp/*
performance/*
portability/*
- qt/*
readability/*
zircon/*
@@ -350,7 +349,6 @@ Clang-Tidy Checks
:doc:`portability-restrict-system-includes <portability/restrict-system-includes>`, "Yes"
:doc:`portability-simd-intrinsics <portability/simd-intrinsics>`,
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
- :doc:`qt-integer-sign-comparison <qt/integer-sign-comparison>`, "Yes"
:doc:`portability-template-virtual-member-function <portability/template-virtual-member-function>`,
:doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes"
:doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`,
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 3fab41f7787285..54aef3ca948d21 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
@@ -32,8 +32,3 @@ Options
A string specifying which include-style is used, `llvm` or `google`.
Default is `llvm`.
-
-.. option:: IsQtApplication
-
- When `true`, it is assumed that the code being analyzed is using the Qt framework.
- Default is `false`.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
deleted file mode 100644
index fe35b7345f4210..00000000000000
--- a/clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
+++ /dev/null
@@ -1,50 +0,0 @@
-.. title:: clang-tidy - qt-integer-sign-comparison
-
-qt-integer-sign-comparison
-=============================
-
-The qt-integer-sign-comparison check is an alias, please see
-:doc:`modernize-use-integer-sign-comparison <../modernize/use-integer-sign-comparison>`
-for more information.
-
-Examples of fixes created by the check:
-
-.. code-block:: c++
-
- uint func(int a, uint b) {
- return a == b;
- }
-
-in C++17 becomes
-
-.. code-block:: c++
-
- <QtCore/q20utility.h>
-
- uint func(int a, uint b) {
- return (q20::cmp_equal(result, bla))
- }
-
-in C++20 becomes
-
-.. code-block:: c++
-
- #include <utility>
-
- uint func(int a, uint b) {
- return (std::cmp_equal(result, bla))
- }
-
-
-Options
--------
-
-.. option:: IncludeStyle
-
- A string specifying which include-style is used, `llvm` or `google`.
- Default is `llvm`.
-
-.. option:: IsQtApplication
-
- When `true`, it is assumed that the code being analyzed is using the Qt framework.
- Default is `false`.
diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst
index e0fc8ab73a5931..a4233d5d8e2694 100644
--- a/clang-tools-extra/docs/clang-tidy/index.rst
+++ b/clang-tools-extra/docs/clang-tidy/index.rst
@@ -84,7 +84,6 @@ Name prefix Description
``performance-`` Checks that target performance-related issues.
``portability-`` Checks that target portability-related issues that don't
relate to any particular coding style.
-``qt-`` Checks related to the Qt framework.
``readability-`` Checks that target readability-related issues that don't
relate to any particular coding style.
``zircon-`` Checks related to Zircon kernel coding conventions.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
deleted file mode 100644
index 07e977d5c1b2ee..00000000000000
--- a/clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp
+++ /dev/null
@@ -1,67 +0,0 @@
-// RUN: %check_clang_tidy -std=c++17 %s qt-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 [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::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[2] = {0, 1};
-
- int sVar = -42;
- short sArray[2] = {-1, -2};
-
- if (uVar < (unsigned int)sVar)
- return 0;
-// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_less(uVar , sVar))
-
- (uVar != sVar) ? uVar = sVar
- : sVar = uVar;
-// CHECK-MESSAGES: :[[@LINE-2]]:6: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-// CHECK-FIXES: (q20::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 [qt-integer-sign-comparison]
-// CHECK-FIXES: while (q20::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 [qt-integer-sign-comparison]
-// CHECK-FIXES: if (q20::cmp_greater(uArray[1] , sArray[1]))
-
- MAX_MACRO(uVar, sArray[0]);
-// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: comparison between 'signed' and 'unsigned' integers [qt-integer-sign-comparison]
-
- FuncParameters(uVar);
- TemplateFuncParameter(sVar);
- TemplateFuncParameters(uVar, sVar);
-
- return 0;
-}
>From 0ce31677d3ccd9ec77d8d78e33954b404ebb8c4a Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 21 Nov 2024 12:59:08 +0100
Subject: [PATCH 10/12] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup!
fixup! [clang-tidy] Create a check for signed and unsigned integers
comparison
---
.../clang-tidy/checks/modernize/use-integer-sign-comparison.rst | 1 -
1 file changed, 1 deletion(-)
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 54aef3ca948d21..5134af4d7dba7d 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
@@ -6,7 +6,6 @@ modernize-use-integer-sign-comparison
Replace comparisons between signed and unsigned integers with their safe
C++20 ``std::cmp_*`` alternative, if available.
-
Examples of fixes created by the check:
.. code-block:: c++
>From 70eac63e01101545a51573019fa0beeb9963da31 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Thu, 21 Nov 2024 20:43:37 +0100
Subject: [PATCH 11/12] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup!
fixup! fixup! [clang-tidy] Create a check for signed and unsigned integers
comparison
---
.../checks/modernize/use-integer-sign-comparison.rst | 3 +++
1 file changed, 3 insertions(+)
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 5134af4d7dba7d..aced6d088ea302 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
@@ -6,6 +6,9 @@ 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 a user fixes it manually.
+
Examples of fixes created by the check:
.. code-block:: c++
>From 0590ce079aec4a397152384d2f2e2d52cff7c751 Mon Sep 17 00:00:00 2001
From: Tatiana Borisova <tatiana.borisova at qt.io>
Date: Mon, 25 Nov 2024 18:18:14 +0100
Subject: [PATCH 12/12] fixup! fixup! fixup! fixup! fixup! fixup! fixup! fixup!
fixup! fixup! fixup! [clang-tidy] Create a check for signed and unsigned
integers comparison
---
.../UseIntegerSignComparisonCheck.cpp | 68 ++++++-------------
.../modernize/UseIntegerSignComparisonCheck.h | 3 +-
.../modernize/use-integer-sign-comparison.rst | 2 +-
.../modernize/use-integer-sign-comparison.cpp | 42 ++++++++++--
4 files changed, 57 insertions(+), 58 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
index 0ecce36eeff65e..a3e74b68cdb21c 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
@@ -108,71 +108,41 @@ void UseIntegerSignComparisonCheck::check(
return;
const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode();
- const Expr *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
- const Expr *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
+
+ 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());
-
- if (const auto *LHSCast = BinaryOp->getLHS() == SignedCastExpression
- ? SignedCastExpression
- : UnSignedCastExpression) {
- if (LHSCast->isPartOfExplicitCast()) {
- SubExprLHS = LHSCast->getSubExpr();
- R1 = SourceRange(LHS->getBeginLoc(), SubExprLHS->getBeginLoc());
- }
+ 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 = BinaryOp->getLHS() == SignedCastExpression
- ? UnSignedCastExpression
- : SignedCastExpression) {
- if (RHSCast->isPartOfExplicitCast()) {
- SubExprRHS = RHSCast->getSubExpr();
- R2.setEnd(SubExprRHS->getBeginLoc());
- }
+ 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");
-
if (!getLangOpts().CPlusPlus20)
return;
-
- const std::string CmpNamespace =
- llvm::Twine("std::" + parseOpCode(OpCode)).str();
+ const std::string CmpNamespace = ("std::" + parseOpCode(OpCode)).str();
const std::string CmpHeader = "<utility>";
-
// Prefer modernize-use-integer-sign-comparison when C++20 is available!
- if (SubExprLHS) {
- StringRef ExplicitLhsString =
- Lexer::getSourceText(CharSourceRange::getTokenRange(
- SubExprLHS->IgnoreCasts()->getSourceRange()),
- *Result.SourceManager, getLangOpts());
- Diag << FixItHint::CreateReplacement(
- R1, llvm::Twine(CmpNamespace + "(" + ExplicitLhsString).str());
- } else {
- Diag << FixItHint::CreateInsertion(R1.getBegin(),
- llvm::Twine(CmpNamespace + "(").str());
- }
-
- StringRef ExplicitLhsRhsString =
- SubExprRHS ? Lexer::getSourceText(
- CharSourceRange::getTokenRange(
- SubExprRHS->IgnoreCasts()->getSourceRange()),
- *Result.SourceManager, getLangOpts())
- : "";
Diag << FixItHint::CreateReplacement(
- R2, llvm::Twine(", " + ExplicitLhsRhsString).str());
- Diag << FixItHint::CreateInsertion(
- Lexer::getLocForEndOfToken(
- Result.SourceManager->getSpellingLoc(RHS->getEndLoc()), 0,
- *Result.SourceManager, Result.Context->getLangOpts()),
- ")");
+ 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(
diff --git a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
index 7bc79603d0857b..7df67470d1324d 100644
--- a/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
+++ b/clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
@@ -15,7 +15,8 @@
namespace clang::tidy::modernize {
-/// Class detects comparisons between signed and unsigned integers
+/// 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
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 aced6d088ea302..ce80447d738ab5 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
@@ -24,7 +24,7 @@ becomes
#include <utility>
uint func(int a, uint b) {
- return (std::cmp_equal(result, bla))
+ return (std::cmp_equal(a, b));
}
Options
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 4626a7f3a8a3bc..d1975b0777afd0 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
@@ -1,3 +1,4 @@
+// CHECK-FIXES: #include <utility>
// RUN: %check_clang_tidy -std=c++20 %s modernize-use-integer-sign-comparison %t
// The code that triggers the check
@@ -8,7 +9,7 @@ unsigned int FuncParameters(int bla) {
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))
+// CHECK-FIXES: if (std::cmp_equal(result , bla))
return 1;
}
@@ -31,34 +32,61 @@ int TemplateFuncParameters(T1 val1, T2 val2) {
int AllComparisons() {
unsigned int uVar = 42;
- unsigned short uArray[2] = {0, 1};
+ unsigned short uArray[7] = {0, 1, 2, 3, 9, 7, 9};
int sVar = -42;
- short sArray[2] = {-1, -2};
+ short sArray[7] = {-1, -2, -8, -94, -5, -4, -6};
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))
+// 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
+// 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]))
+// 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]))
+// 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);
More information about the cfe-commits
mailing list