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

Piotr Zegar via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 25 15:50:09 PDT 2024


https://github.com/PiotrZSL created https://github.com/llvm/llvm-project/pull/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

>From 1a9dae14ca859688724ad5405c132528efe3a6cd Mon Sep 17 00:00:00 2001
From: Piotr Zegar <me at piotrzegar.pl>
Date: Mon, 25 Mar 2024 22:25:54 +0000
Subject: [PATCH] [clang-tidy] Improve performance of google-runtime-int

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 improvment, but gain
from that one is low.

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

Closes #86553
---
 .../clang-tidy/google/IntegerTypesCheck.cpp   | 41 +++++++++++++++----
 .../clang-tidy/google/IntegerTypesCheck.h     |  3 ++
 clang-tools-extra/docs/ReleaseNotes.rst       |  3 ++
 3 files changed, 39 insertions(+), 8 deletions(-)

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 a604e9276668ae..4d1cd57c2fd478 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -197,6 +197,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.



More information about the cfe-commits mailing list