-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy] Create a check for signed and unsigned integers comparison #113144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-clang-tidy Author: None (qt-tatiana) Changes
Patch is 24.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/113144.diff 14 Files Affected:
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/c...
[truncated]
|
@llvm/pr-subscribers-clang-tools-extra Author: None (qt-tatiana) Changes
Patch is 24.72 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/113144.diff 14 Files Affected:
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/c...
[truncated]
|
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
@qt-tatiana: Please do not forget to resolve fixed comments. |
Done, thanks! :) |
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be very nice to get in
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
Options | ||
------- | ||
|
||
.. option:: IncludeStyle |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(not for this patch) Sounds like it's about time to move this to the top-level configuration instead of duplicated for every check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created #113577.
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
Hi all! Is there anything else expected from my side? |
Perhaps this has been discussed before, but I think the part about "add qt module" should be added on a separate patch, since adding a brand-new tidy module is a rather big thing. So this patch could be only about adding the |
Done, I've deleted qt related code from current code-version. |
Ping :) |
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Show resolved
Hide resolved
Ping :) |
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The check will currently not flag the use of literals like so:
void foo(unsigned short *uArray){
uArray[0] <= 10;
}
https://godbolt.org/z/a6W9xGcMY
because they do not contain an implicit cast. Only one of the two operands needs to have an implicit cast to the other operands type for the comparison to be a potential issue.
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some small things to address
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your patience.
LGTM with some small nits.
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp
Outdated
Show resolved
Hide resolved
Nit tip: the additional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Ok, thanks! Started to use it in my next commits |
Hi all, |
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
see https://llvm.org/docs/CodeReview.html#lgtm-how-a-patch-is-accepted I can help you to merge it. please feel free to ping me. |
I post a RFC about the cooperation between clang-tidy and common used libraries. Maybe it can be a more generic solution to this kind of modern checks. |
- modernize-use-integer-sign-comparison - replace comparisons between signed and unsigned integers with their safe C++20 ``std::cmp_*`` alternative, if available.
Thank you for help! I squashed all fixups into the main change (used this instruction https://llvm.org/docs/GitHub.html#landing-your-change) Whould it be possible to land the chamge? :-) |
actually github will do squash. please do not squash it manually in further pr. |
@qt-tatiana Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
Ooops, I didn't know. Thank you! |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/153/builds/17264 Here is the relevant piece of the build log for the reference
|
std::cmp_*
alternative, if available.