[clang-tools-extra] 5e6e40f - [clang-tidy] Improve performance of google-runtime-int (#86596)

[via cfe-commits]

Author: Piotr Zegar
Date: 2024-03-26T20:19:23+01:00
New Revision: 5e6e40fee31d5db2f44d604b0362e1e819f41ba5

URL: https://github.com/llvm/llvm-project/commit/5e6e40fee31d5db2f44d604b0362e1e819f41ba5
DIFF: https://github.com/llvm/llvm-project/commit/5e6e40fee31d5db2f44d604b0362e1e819f41ba5.diff

LOG: [clang-tidy] Improve performance of google-runtime-int (#86596)

Main problem with performance of this check is caused by hasAncestor
matcher, and to be more precise by an llvm::DenseSet and std::deque in
matchesAnyAncestorOf.

To reduce impact of this matcher, multiple conditions that were checked
in check method were copied into AST matcher that is now checked before
hasAncestor.

Using custom getCheckTraversalKind to exclude template instances that
shouldn't be checked anyway is an additional improvement, but gain from
that one is low.

Tested on ffl_tests.cc, visible reduction from ~442 seconds to ~15
seconds (~96% reduction).

Closes #86553

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
    clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
    clang-tools-extra/docs/ReleaseNotes.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
index ef511e9108f2ee..359d8efd100bad 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.cpp
@@ -40,6 +40,34 @@ namespace {
 AST_MATCHER(FunctionDecl, isUserDefineLiteral) {
   return Node.getLiteralIdentifier() != nullptr;
 }
+
+AST_MATCHER(TypeLoc, isValidAndNotInMacro) {
+  const SourceLocation Loc = Node.getBeginLoc();
+  return Loc.isValid() && !Loc.isMacroID();
+}
+
+AST_MATCHER(TypeLoc, isBuiltinType) {
+  TypeLoc TL = Node;
+  if (auto QualLoc = Node.getAs<QualifiedTypeLoc>())
+    TL = QualLoc.getUnqualifiedLoc();
+
+  const auto BuiltinLoc = TL.getAs<BuiltinTypeLoc>();
+  if (!BuiltinLoc)
+    return false;
+
+  switch (BuiltinLoc.getTypePtr()->getKind()) {
+  case BuiltinType::Short:
+  case BuiltinType::Long:
+  case BuiltinType::LongLong:
+  case BuiltinType::UShort:
+  case BuiltinType::ULong:
+  case BuiltinType::ULongLong:
+    return true;
+  default:
+    return false;
+  }
+}
+
 } // namespace
 
 namespace tidy::google::runtime {
@@ -63,11 +91,11 @@ void IntegerTypesCheck::registerMatchers(MatchFinder *Finder) {
   // "Where possible, avoid passing arguments of types specified by
   // bitwidth typedefs to printf-based APIs."
   Finder->addMatcher(
-      typeLoc(loc(isInteger()),
-              unless(anyOf(hasAncestor(callExpr(
-                               callee(functionDecl(hasAttr(attr::Format))))),
-                           hasParent(parmVarDecl(hasAncestor(
-                               functionDecl(isUserDefineLiteral())))))))
+      typeLoc(loc(isInteger()), isValidAndNotInMacro(), isBuiltinType(),
+              unless(hasAncestor(
+                  callExpr(callee(functionDecl(hasAttr(attr::Format)))))),
+              unless(hasParent(parmVarDecl(
+                  hasAncestor(functionDecl(isUserDefineLiteral()))))))
           .bind("tl"),
       this);
   IdentTable = std::make_unique<IdentifierTable>(getLangOpts());
@@ -77,9 +105,6 @@ void IntegerTypesCheck::check(const MatchFinder::MatchResult &Result) {
   auto TL = *Result.Nodes.getNodeAs<TypeLoc>("tl");
   SourceLocation Loc = TL.getBeginLoc();
 
-  if (Loc.isInvalid() || Loc.isMacroID())
-    return;
-
   // Look through qualification.
   if (auto QualLoc = TL.getAs<QualifiedTypeLoc>())
     TL = QualLoc.getUnqualifiedLoc();

diff  --git a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
index 3be7d24021b9ff..c62bda67ae2d98 100644
--- a/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
+++ b/clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
@@ -35,6 +35,9 @@ class IntegerTypesCheck : public ClangTidyCheck {
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
 
 private:
   const StringRef UnsignedTypePrefix;

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f6ae712510ca89..f119e14ae478a9 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -201,6 +201,9 @@ Changes in existing checks
   <clang-tidy/checks/google/global-names-in-headers>` check by replacing the local
   option `HeaderFileExtensions` by the global option of the same name.
 
+- Improved :doc:`google-runtime-int <clang-tidy/checks/google/runtime-int>`
+  check performance through optimizations.
+
 - Improved :doc:`llvm-header-guard
   <clang-tidy/checks/llvm/header-guard>` check by replacing the local
   option `HeaderFileExtensions` by the global option of the same name.