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

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
   )

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 =

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

clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp

@@ -37,6 +37,7 @@
 #include "UseEmplaceCheck.h"
 #include "UseEqualsDefaultCheck.h"
 #include "UseEqualsDeleteCheck.h"
+#include "UseIntegerSignComparisonCheck.h"
 #include "UseNodiscardCheck.h"
 #include "UseNoexceptCheck.h"
 #include "UseNullptrCheck.h"
@@ -76,6 +77,8 @@ class ModernizeModule : public ClangTidyModule {
     CheckFactories.registerCheck<PassByValueCheck>("modernize-pass-by-value");
     CheckFactories.registerCheck<UseDesignatedInitializersCheck>(
         "modernize-use-designated-initializers");
+    CheckFactories.registerCheck<UseIntegerSignComparisonCheck>(
+        "modernize-use-integer-sign-comparison");
     CheckFactories.registerCheck<UseRangesCheck>("modernize-use-ranges");
     CheckFactories.registerCheck<UseStartsEndsWithCheck>(
         "modernize-use-starts-ends-with");

clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.cpp

@@ -0,0 +1,165 @@
+//===--- 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)),
+      StringsMatchHeader(Options.get("StringsMatchHeader", "<utility>")) {}
+
+void UseIntegerSignComparisonCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IsQtApplication", IsQtApplication);
+  Options.store(Opts, "StringsMatchHeader", StringsMatchHeader);
+}
+
+void UseIntegerSignComparisonCheck::registerMatchers(MatchFinder *Finder) {
+  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 {};
+  }
+}
+
+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)) {
+    const llvm::APSInt SValue = EVResult.Val.getInt();
+    if (SValue.isNonNegative())
+      return;
+  }
+
+  const auto *BinaryOp =
+      Result.Nodes.getNodeAs<BinaryOperator>("intComparison");
+  if (BinaryOp == nullptr)
+    return;
+
+  const BinaryOperator::Opcode OpCode = BinaryOp->getOpcode();
+  const Expr *LHS = BinaryOp->getLHS()->IgnoreParenImpCasts();
+  const Expr *RHS = BinaryOp->getRHS()->IgnoreParenImpCasts();
+  if (LHS == nullptr || RHS == nullptr)
+    return;
+
+  const StringRef LhsString(Lexer::getSourceText(
+      CharSourceRange::getTokenRange(LHS->getSourceRange()),
+      *Result.SourceManager, getLangOpts()));
+
+  const StringRef RhsString(Lexer::getSourceText(
+      CharSourceRange::getTokenRange(RHS->getSourceRange()),
+      *Result.SourceManager, getLangOpts()));
+
+  DiagnosticBuilder Diag =
+      diag(BinaryOp->getBeginLoc(),
+           "comparison between 'signed' and 'unsigned' integers");
+
+  if (!(getLangOpts().CPlusPlus17 && IsQtApplication) &&
+      !getLangOpts().CPlusPlus20)
+    return;
+
+  std::string CmpNamespace;
+  if (getLangOpts().CPlusPlus20) {
+    CmpNamespace = std::string("std::") + parseOpCode(OpCode);
+  } else if (getLangOpts().CPlusPlus17 && IsQtApplication) {
+    CmpNamespace = std::string("q20::") + parseOpCode(OpCode);
+  }
+
+  // Use qt-use-integer-sign-comparison when C++17 is available and only for Qt
+  // 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()),
+      StringsMatchHeader);
+}
+
+} // namespace clang::tidy::modernize

clang-tools-extra/clang-tidy/modernize/UseIntegerSignComparisonCheck.h

@@ -0,0 +1,47 @@
+//===--- 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;
+  const StringRef StringsMatchHeader;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USEINTEGERSIGNCOMPARISONCHECK_H

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
+)

clang-tools-extra/clang-tidy/qt/QtTidyModule.cpp

@@ -0,0 +1,48 @@
+//===--- 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.IncludeStyle"] = "llvm";
+    Opts["qt-integer-sign-comparison.IsQtApplication"] = "true";
+    Opts["qt-integer-sign-comparison.StringsMatchHeader"] =
+        "<QtCore/q20utility.h>";
+
+    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

clang-tools-extra/docs/ReleaseNotes.rst

@@ -123,10 +123,19 @@ New checks
   Gives warnings for tagged unions, where the number of tags is
   different from the number of data members inside the union.
 
+- New :doc:`modernize-use-integer-sign-comparison
+  <clang-tidy/checks/modernize/use-integer-sign-comparison>` check.
+
+  Performs comparisons between signed and unsigned integer types
+  mathematically correct. If C++20 is supported a fix-it replaces
+  integers comparisons to ``std::cmp_equal``, ``std::cmp_not_equal``,
+  ``std::cmp_less``, ``std::cmp_greater``, ``std::cmp_less_equal`` and
+  ``std::cmp_greater_equal`` functions.
+
 - New :doc:`portability-template-virtual-member-function
   <clang-tidy/checks/portability/template-virtual-member-function>` check.
 
-  Finds cases when an uninstantiated virtual member function in a template class 
+  Finds cases when an uninstantiated virtual member function in a template class
   causes cross-compiler incompatibility.
 
 New check aliases
@@ -136,6 +145,16 @@ 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``, ``q20::cmp_not_equal``, ``q20::cmp_less``,
+  ``q20::cmp_greater``, ``q20::cmp_less_equal`` and ``q20::cmp_greater_equal``
+  functions.
+  The check assumes the analysed code is Qt-based code.
+
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^