Skip to content
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

[clang-tidy] Create a check for signed and unsigned integers comparison #113144

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

qt-tatiana
Copy link

  • 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.

- 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.
Copy link

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 @ followed by their GitHub username.

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang-tidy

Author: None (qt-tatiana)

Changes
  • 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.

  • 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.


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:

  • (modified) clang-tools-extra/clang-tidy/CMakeLists.txt (+2)
  • (modified) clang-tools-extra/clang-tidy/ClangTidyForceLinker.h (+5)
  • (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
  • (added) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp (+169)
  • (added) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h (+46)
  • (added) clang-tools-extra/clang-tidy/qt/CMakeLists.txt (+26)
  • (added) clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp (+46)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+17)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+3)
  • (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst (+43)
  • (added) clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst (+64)
  • (modified) clang-tools-extra/docs/clang-tidy/index.rst (+1)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp (+61)
  • (added) clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp (+61)
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]

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 21, 2024

@llvm/pr-subscribers-clang-tools-extra

Author: None (qt-tatiana)

Changes
  • 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.

  • 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.


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:

  • (modified) clang-tools-extra/clang-tidy/CMakeLists.txt (+2)
  • (modified) clang-tools-extra/clang-tidy/ClangTidyForceLinker.h (+5)
  • (modified) clang-tools-extra/clang-tidy/modernize/CMakeLists.txt (+1)
  • (added) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp (+169)
  • (added) clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h (+46)
  • (added) clang-tools-extra/clang-tidy/qt/CMakeLists.txt (+26)
  • (added) clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp (+46)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+17)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+3)
  • (added) clang-tools-extra/docs/clang-tidy/checks/modernize/use-integer-sign-comparison.rst (+43)
  • (added) clang-tools-extra/docs/clang-tidy/checks/qt/IntegerCrossSignComparison.rst (+64)
  • (modified) clang-tools-extra/docs/clang-tidy/index.rst (+1)
  • (added) clang-tools-extra/test/clang-tidy/checkers/modernize/use-integer-sign-comparison.cpp (+61)
  • (added) clang-tools-extra/test/clang-tidy/checkers/qt/qt-integer-sign-comparison.cpp (+61)
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]

@EugeneZelenko
Copy link
Contributor

@qt-tatiana: Please do not forget to resolve fixed comments.

@qt-tatiana
Copy link
Author

@qt-tatiana: Please do not forget to resolve fixed comments.

Done, thanks! :)

Copy link
Contributor

@5chmidti 5chmidti left a 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

Comment on lines 29 to 30
Options.store(Opts, "IsQtApplication", IsQtApplication);
Options.store(Opts, "StringsMatchHeader", StringsMatchHeader);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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....

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add tests where templates and macros are involved

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Comment on lines +122 to +123
const Expr *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
const Expr *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure If I did everything 100% according the comment, but please see the change :)

is the Qt-based code.

.. option:: StringsMatchHeader
A string specifying a include header file to be added by fix-it. Default
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please separate with newline.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated my solution a little bit and deleted this property, but forgot to delete from doc. thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants