[clang-tidy] Create a check for signed and unsigned integers comparison #113144
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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.
|
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
EugeneZelenko
requested review from
PiotrZSL,
SimplyDanny,
5chmidti and
HerrCai0907
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Show resolved
Hide resolved
HerrCai0907
reviewed
Oct 24, 2024
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
…ned integers comparison
5chmidti
requested changes
Oct 27, 2024
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Show resolved
Hide resolved
Comment on lines
29
to
30
| Options.store(Opts, "IsQtApplication", IsQtApplication); | ||
| Options.store(Opts, "StringsMatchHeader", StringsMatchHeader); |
There was a problem hiding this comment.
If instead, you provide the header as an option and a namespace, then the check would be even more generic and the Qt module would not be needed (not saying a Qt module wouldn't make sense, just that this check may not require adding it). Though a quick look at boost, gsl and abseil did not show them having such functions.
(You could mention the settings for QT in the documentation as an example)
It would be best to check what others think, though. Thoughts?
There was a problem hiding this comment.
I thought it would be comfortable to have QT module, because in that case it's possible to turn on several QT-related checks together at the same moment (without providing parameters for each check from command line).
Yes, there is only 1 check that has QT-related extention for now, but I have several check-ideas....
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
Show resolved
Hide resolved
There was a problem hiding this comment.
Please add tests where templates and macros are involved
There was a problem hiding this comment.
Do you mean more templates and macros?
There already were:
// The code that triggers the check
#define MAX_MACRO(a, b) (a < b) ? b : a
template
void TemplateFuncParameter(T val) {
unsigned long uL = 0;
if (val >= uL)
return;
}
But anyway I updated existing ones (and also now templates are skipped by check)
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
Comment on lines
+122
to
+123
| const Expr *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts(); | ||
| const Expr *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts(); |
There was a problem hiding this comment.
Because you determine LHS and RHS like this, the replacement will include the explicit cast if there is any, while it could have removed that cast instead. With the below comment on the fixit, you'd simply need to know which of sIntCastExpression and uIntCastExpression (also voiding my comment on it being unused) is the LHS and which is the RHS, allowing you, e.g., to replace instead of insert the std::cmp_*( instead of inserting it at the front
There was a problem hiding this comment.
Not sure If I did everything 100% according the comment, but please see the change :)
clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp
Outdated
Show resolved
Hide resolved
…d unsigned integers comparison
clang-tools-extra/docs/clang-tidy/checks/qt/integer-sign-comparison.rst
Outdated
Show resolved
Hide resolved
| is the Qt-based code. | ||
|
|
||
| .. option:: StringsMatchHeader | ||
| A string specifying a include header file to be added by fix-it. Default |
There was a problem hiding this comment.
Please separate with newline.
There was a problem hiding this comment.
I updated my solution a little bit and deleted this property, but forgot to delete from doc. thanks!
…gned and unsigned integers comparison
… for signed and unsigned integers comparison
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 include.
add a qt module for qt-related checks.
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.