[clang-tidy] Avoid processing declarations in system headers

Carlos Gálvez · Carlos Gálvez · commit 18a676ba6629 · 2025-02-21T12:18:22.000Z
Currently, clang-tidy processes the entire TranslationUnit, including declarations in system headers. However, the work done in system headers is discarded at the very end when presenting results, unless the SystemHeaders option is active. This is a lot of wasted work, and makes clang-tidy very slow. In comparison, clangd only processes declarations in the main file, and it's claimed to be 10x faster than clang-tidy: https://github.com/lljbash/clangd-tidy To solve this problem, we can apply a similar solution done in clangd into clang-tidy. We do this by changing the traversal scope from the default TranslationUnitDecl, to only contain the top-level declarations that are _not_ part of system headers. We do this by prepending a new ASTConsumer to the list of consumers: this new consumer sets the traversal scope in the ASTContext, which is later used by the MatchASTConsumer. Note: this behavior is not active if the user requests warnings from system headers via the SystemHeaders option. Note2: out of all the unit tests, only one of them fails: readability/identifier-naming-anon-record-fields.cpp This is because the limited traversal scope no longer includes the "IndirectFieldDecl" that appears in the AST when having a global scope anonymous union. I have not found a way to make this one work. However, it does seem like a very niche use case, and the benefits of a 10x faster clang-tidy largely outweigh the false negative now introduced by this patch. This use case is therefore removed from the unit test to make it pass. Note3: I have purposely decided to make this new feature enabled by default, instead of adding a new "opt-in/opt-out" flag. Having a new flag would mean duplicating all our tests to ensure they work in both modes, which would be infeasible. Having it enabled by default allow people to get the benefits immediately. Given that all unit tests pass, the risk for regressions is low. Even if that's the case, the only issue would be false negatives (fewer things are detected), which are much more tolerable than false positives. Credits: original implementation by @njames93, here: https://reviews.llvm.org/D150126 This implementation is simpler in the sense that it does not consider HeaderFilterRegex to filter even further. A follow-up patch could include the functionality if wanted. Fixes #52959
diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp
@@ -339,6 +339,35 @@ class ClangTidyASTConsumer : public MultiplexConsumer {
   void anchor() override {};
 };
 
+/// ASTConsumer that filters top-level declarations that are in system headers,
+/// and sets the AST traversal scope to only cover the declarations in user
+/// headers. This makes all clang-tidy checks avoid spending time processing
+/// declarations in system headers. The results are discarded anyway when
+/// presenting the results.
+class IgnoreSystemHeadersConsumer : public ASTConsumer {
+public:
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+    for (Decl *D : DG) {
+      if (!isInSystemHeader(D))
+        Decls.push_back(D);
+    }
+    return true;
+  }
+
+  void HandleTranslationUnit(ASTContext &Ctx) override {
+    Ctx.setTraversalScope(Decls);
+  }
+
+private:
+  std::vector<Decl *> Decls;
+
+  bool isInSystemHeader(Decl *D) {
+    SourceManager &SM = D->getASTContext().getSourceManager();
+    SourceLocation Loc = SM.getExpansionLoc(D->getBeginLoc());
+    return SM.isInSystemHeader(Loc);
+  }
+};
+
 } // namespace
 
 ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
@@ -449,6 +478,10 @@ ClangTidyASTConsumerFactory::createASTConsumer(
   }
 
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+
+  if (!Context.getOptions().SystemHeaders.value_or(false))
+    Consumers.push_back(std::make_unique<IgnoreSystemHeadersConsumer>());
+
   if (!Checks.empty())
     Consumers.push_back(Finder->newASTConsumer());
 
diff --git a/clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp
@@ -35,19 +35,41 @@ AST_POLYMORPHIC_MATCHER_P(
                              Builder) != Args.end();
 }
 
+bool isStdOrPosixImpl(const DeclContext *Ctx) {
+  if (!Ctx->isNamespace())
+    return false;
+
+  const auto *ND = cast<NamespaceDecl>(Ctx);
+  if (ND->isInline()) {
+    return isStdOrPosixImpl(ND->getParent());
+  }
+
+  if (!ND->getParent()->getRedeclContext()->isTranslationUnit())
+    return false;
+
+  const IdentifierInfo *II = ND->getIdentifier();
+  return II && (II->isStr("std") || II->isStr("posix"));
+}
+
+AST_MATCHER(Decl, isInStdOrPosixNS) {
+  for (const auto *Ctx = Node.getDeclContext(); Ctx; Ctx = Ctx->getParent()) {
+    if (isStdOrPosixImpl(Ctx))
+      return true;
+  }
+  return false;
+}
+
 } // namespace
 
 namespace clang::tidy::cert {
 
 void DontModifyStdNamespaceCheck::registerMatchers(MatchFinder *Finder) {
   auto HasStdParent =
       hasDeclContext(namespaceDecl(hasAnyName("std", "posix"),
-                                   unless(hasParent(namespaceDecl())))
+                                   unless(hasDeclContext(namespaceDecl())))
                          .bind("nmspc"));
-  auto UserDefinedType = qualType(
-      hasUnqualifiedDesugaredType(tagType(unless(hasDeclaration(tagDecl(
-          hasAncestor(namespaceDecl(hasAnyName("std", "posix"),
-                                    unless(hasParent(namespaceDecl()))))))))));
+  auto UserDefinedType = qualType(hasUnqualifiedDesugaredType(
+      tagType(unless(hasDeclaration(tagDecl(isInStdOrPosixNS()))))));
   auto HasNoProgramDefinedTemplateArgument = unless(
       hasAnyTemplateArgumentIncludingPack(refersToType(UserDefinedType)));
   auto InsideStdClassOrClassTemplateSpecialization = hasDeclContext(
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -88,6 +88,10 @@ Improvements to clang-query
 Improvements to clang-tidy
 --------------------------
 
+- It no longer processes declarations from system headers by default, greatly
+  improving performance (up to 10x speed-up). This behavior is disabled if the
+  `SystemHeaders` option is enabled.
+
 New checks
 ^^^^^^^^^^
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-anon-record-fields.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-anon-record-fields.cpp
@@ -33,24 +33,6 @@
 // RUN:     readability-identifier-naming.LocalConstantPointerPrefix: 'lc_', \
 // RUN:   }}'
 
-static union {
-  int global;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global variable 'global'
-// CHECK-FIXES: {{^}}  int g_global;{{$}}
-
-  const int global_const;
-// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for global constant 'global_const'
-// CHECK-FIXES: {{^}}  const int GLOBAL_CONST;{{$}}
-
-  int *global_ptr;
-// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for global pointer 'global_ptr'
-// CHECK-FIXES: {{^}}  int *GlobalPtr_Ptr;{{$}}
-
-  int *const global_const_ptr;
-// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for global constant pointer 'global_const_ptr'
-// CHECK-FIXES: {{^}}  int *const GLOBAL_CONST_PTR_Ptr;{{$}}
-};
-
 namespace ns {
 
 static union {
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
@@ -66,19 +66,12 @@ class A { A(int); };
 // CHECK4-NOT: warning:
 // CHECK4-QUIET-NOT: warning:
 
-// CHECK: Suppressed 3 warnings (3 in non-user code)
 // CHECK: Use -header-filter=.* to display errors from all non-system headers.
 // CHECK-QUIET-NOT: Suppressed
-// CHECK2: Suppressed 1 warnings (1 in non-user code)
-// CHECK2: Use -header-filter=.* {{.*}}
 // CHECK2-QUIET-NOT: Suppressed
-// CHECK3: Suppressed 2 warnings (2 in non-user code)
-// CHECK3: Use -header-filter=.* {{.*}}
 // CHECK3-QUIET-NOT: Suppressed
 // CHECK4-NOT: Suppressed {{.*}} warnings
-// CHECK4-NOT: Use -header-filter=.* {{.*}}
 // CHECK4-QUIET-NOT: Suppressed
-// CHECK6: Suppressed 2 warnings (2 in non-user code)
 // CHECK6: Use -header-filter=.* {{.*}}
 
 int x = 123;
diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
@@ -11,9 +11,9 @@
 // RUN: clang-tidy -help | FileCheck -check-prefix=CHECK-OPT-PRESENT %s
 
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=true %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -system-headers=false %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS --allow-empty %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: true' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-SYSTEM-HEADERS %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='.*' -config='SystemHeaders: false' %s -- -isystem %S/Inputs/system-headers 2>&1 | FileCheck -check-prefix=CHECK-NO-SYSTEM-HEADERS --allow-empty %s
 
 #include <system_header.h>
 // CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked explicit
