[clang-tidy][WIP] Set traversal scope to prevent analysis in headers

[njames93]

Summary

See https://discourse.llvm.org/t/rfc-exclude-issues-from-system-headers-as-early-as-posible/68483

[njames93]

Fix failing unittests

[carlosgalvezp]

clang-tools-extra/clang-tidy/ClangTidy.cpp

511

Here comes a stupid question :) If I understand correctly we are adding one more ASTConsumer to the list of Consumers, i.e. we are not replacing the "normal" consumer with the "optimized" consumer. So I fail to understand how adding this new consumer fixes the problems of the other consumer? I'd be happy to read more about the implementation if there are resources you could point me to.

[njames93]

clang-tools-extra/clang-tidy/ClangTidy.cpp

511

The MultiplexConsumer runs each consumers callbacks in the order they are added. So this consumers HandleTranslationUnit callback will be executed before the FinderASTConsumer runs its HandleTranslationUnit callback(The function that actually does the AST matching. So by modifying the ASTContext traversal scope in this HandleTranslationUnit callback, the results are visible when the Finder comes to do its matching.

Also part of this implementation was taken from clangd which uses a somewhat similar approach to only run tidy checks that originate in the main file https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/ParsedAST.cpp#L73

[njames93]

Fix infrastructure test failures

[PiotrZSL]

I love an idea, but i'm afraid that it may impact checks that build some internal database, like misc-confusable-identifiers or misc-unused-using-decls.
Probably some of those checks would need to be rewrite to use some own visitor. And probably we should have access to that Cache (function that check if location should be ignored) from a check code, in such case in some cases it could be used instead of re-inventing a wheal.

clang-tools-extra/clang-tidy/ClangTidy.cpp

ClangTidyASTConsumerFactory::createASTConsumer(
   }
 
   for (auto &Check : Checks) {
     Check->registerMatchers(&*Finder);
     Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+
+  if (!Context.getOptions().SystemHeaders.value_or(false) ||
+      Context.getOptions().HeaderFilterRegex.value_or("") != ".*") {
+    class TraversalScopeConsumer : public ASTConsumer {
+      void Initialize(ASTContext &Context) override {
+        // Make sure the main file ID always gets included.
+        Cache.insert(
+            std::make_pair(Context.getSourceManager().getMainFileID(), true));
+      }
+
+      bool shouldKeepDeclsFromFile(SourceManager &SM, FileID FID) {
+        // Tricky cases, just assume we should include
+        if (FID.isInvalid() || FID == FileID::getSentinel())
+          return true;
+
+        auto [Item, Inserted] = Cache.try_emplace(FID, true);
+
+        // Already in cache, return cached value
+        if (!Inserted)
+          return Item->getSecond();
+
+        // Hash value isn't strictly what we need, but it currently just returns
+        // the opaque id which is what we need. Any other way to get the ID
+        // would require modifying the FID class or type-punning.
+        auto Value = static_cast<int>(FID.getHashValue());
+        SrcMgr::SLocEntry Entry;
+        if (Value > 0) {
+          // Regular file
+          Entry = SM.getLocalSLocEntry(static_cast<unsigned>(Value));
+        } else {
+          // Module
+          bool Invalid = false;
+          Entry = SM.getLoadedSLocEntry(static_cast<unsigned>(-Value - 2),
+                                        &Invalid);
+          if (Invalid)
+            return true;
+        }
+
+        if (FilterSystem &&
+            SrcMgr::isSystem(Entry.getFile().getFileCharacteristic()))
+          return Item->second = false;
+        if (HeaderFilter)
+          return Item->second = HeaderFilter->match(Entry.getFile().getName());
+        return true;
+      }
+
+      // Gather all top level decls
+      bool HandleTopLevelDecl(DeclGroupRef DG) override {
+        for (auto *D : DG) {
+          auto &SM = D->getASTContext().getSourceManager();
+          if (!shouldKeepDeclsFromFile(
+                  SM, SM.getDecomposedExpansionLoc(D->getLocation()).first))
+            continue;
+          Decls.push_back(D);
+        }
+        return true;
+      }
+
+      // Set the contexts traversal scope to only include top-level decls.
+      void HandleTranslationUnit(ASTContext &Ctx) override {
+        Ctx.setTraversalScope(Decls);
+      }
+
+      std::optional<llvm::Regex> HeaderFilter;
+      llvm::DenseMap<clang::FileID, bool> Cache;
+      bool FilterSystem;
+      std::vector<Decl *> Decls;
+
+    public:
+      TraversalScopeConsumer(bool FilterSystem,
+                             const std::optional<std::string> &HeaderFilter)
+          : FilterSystem(FilterSystem) {
+        if (HeaderFilter && *HeaderFilter != ".*")
+          this->HeaderFilter.emplace(*HeaderFilter);
+      }
+    };
+
+    Consumers.push_back(std::make_unique<TraversalScopeConsumer>(
+        !Context.getOptions().SystemHeaders.value_or(false),
+        Context.getOptions().HeaderFilterRegex));
+  }
   if (!Checks.empty())
     Consumers.push_back(Finder->newASTConsumer());
 
 #if CLANG_TIDY_ENABLE_STATIC_ANALYZER
   AnalyzerOptionsRef AnalyzerOptions = Compiler.getAnalyzerOpts();
   AnalyzerOptions->CheckersAndPackages = getAnalyzerCheckersAndPackages(
       Context, Context.canEnableAnalyzerAlphaCheckers());
   if (!AnalyzerOptions->CheckersAndPackages.empty()) {

clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp

 using namespace clang::ast_matchers;
 
 namespace clang::tidy::abseil {
 
 void NoInternalDependenciesCheck::registerMatchers(MatchFinder *Finder) {
   // TODO: refactor matcher to be configurable or just match on any internal
   // access from outside the enclosing namespace.
 
-  Finder->addMatcher(
-      nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl(
-                                 matchesName("internal"),
-                                 hasParent(namespaceDecl(hasName("absl")))))),
-                             unless(isInAbseilFile()))
+  Finder->addMatcher(nestedNameSpecifierLoc(
+                         loc(specifiesNamespace(namespaceDecl(
+                             hasDeclContext(namespaceDecl(hasName("absl")))))),
                          .bind("InternalDep"),
                      this);
 }
 
 void NoInternalDependenciesCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *InternalDependency =
       Result.Nodes.getNodeAs<NestedNameSpecifierLoc>("InternalDep");
 
   SourceLocation LocAtFault =
       Result.SourceManager->getSpellingLoc(InternalDependency->getBeginLoc());

clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp

void UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
   // `absl::Duration` and `b` is not of a built-in type.
   Finder->addMatcher(
       cxxOperatorCallExpr(
           argumentCountIs(2),
           hasArgument(
               0, expr(hasType(cxxRecordDecl(hasName("::absl::Duration"))))),
           hasArgument(1, expr().bind("arg")),
           callee(functionDecl(
-              hasParent(functionTemplateDecl()),
+              ast_matchers::isTemplateInstantiation(),
               unless(hasTemplateArgument(0, refersToType(builtinType()))),
               hasAnyName("operator*=", "operator/="))))
           .bind("OuterExpr"),
       this);
 
   // Match expressions like `a.operator*=(b)` and `a.operator/=(b)` where `a`
   // has type `absl::Duration` and `b` is not of a built-in type.
   Finder->addMatcher(
       cxxMemberCallExpr(
           callee(cxxMethodDecl(
               ofClass(cxxRecordDecl(hasName("::absl::Duration"))),
-              hasParent(functionTemplateDecl()),
+              ast_matchers::isTemplateInstantiation(),
               unless(hasTemplateArgument(0, refersToType(builtinType()))),
               hasAnyName("operator*=", "operator/="))),
           argumentCountIs(1), hasArgument(0, expr().bind("arg")))
           .bind("OuterExpr"),
       this);
 
   // Match expressions like `a * b`, `a / b`, `operator*(a, b)`, and
   // `operator/(a, b)` where `a` has type `absl::Duration` and `b` is not of a
   // built-in type.
   Finder->addMatcher(
       callExpr(callee(functionDecl(
-                   hasParent(functionTemplateDecl()),
+                   ast_matchers::isTemplateInstantiation(),
                    unless(hasTemplateArgument(0, refersToType(builtinType()))),
                    hasAnyName("::absl::operator*", "::absl::operator/"))),
                argumentCountIs(2),
                hasArgument(0, expr(hasType(
                                   cxxRecordDecl(hasName("::absl::Duration"))))),
                hasArgument(1, expr().bind("arg")))
           .bind("OuterExpr"),
       this);
 
   // Match expressions like `a * b` and `operator*(a, b)` where `a` is not of a
   // built-in type and `b` has type `absl::Duration`.
   Finder->addMatcher(
       callExpr(callee(functionDecl(
-                   hasParent(functionTemplateDecl()),
+                   ast_matchers::isTemplateInstantiation(),
                    unless(hasTemplateArgument(0, refersToType(builtinType()))),
                    hasName("::absl::operator*"))),
                argumentCountIs(2), hasArgument(0, expr().bind("arg")),
                hasArgument(1, expr(hasType(
                                   cxxRecordDecl(hasName("::absl::Duration"))))))
           .bind("OuterExpr"),
       this);
 
void UpgradeDurationConversionsCheck::registerMatchers(MatchFinder *Finder) {
   //   `absl::Nanoseconds(x)`
   //   `absl::Microseconds(x)`
   //   `absl::Milliseconds(x)`
   //   `absl::Seconds(x)`
   //   `absl::Minutes(x)`
   //   `absl::Hours(x)`
   // where `x` is not of a built-in type.
   Finder->addMatcher(
-      traverse(TK_AsIs, implicitCastExpr(
-                            anyOf(hasCastKind(CK_UserDefinedConversion),
-                                  has(implicitCastExpr(
-                                      hasCastKind(CK_UserDefinedConversion)))),
-                            hasParent(callExpr(
-                                callee(functionDecl(
-                                    DurationFactoryFunction(),
-                                    unless(hasParent(functionTemplateDecl())))),
-                                hasArgument(0, expr().bind("arg")))))
-                            .bind("OuterExpr")),
+      traverse(TK_AsIs,
+               implicitCastExpr(
+                           unless(ast_matchers::isTemplateInstantiation()))),
       this);
 }
 
 void UpgradeDurationConversionsCheck::check(
     const MatchFinder::MatchResult &Result) {
   const llvm::StringRef Message =
       "implicit conversion to 'int64_t' is deprecated in this context; use an "
       "explicit cast instead";

clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp

for (const auto &Arg : Args) {
     if (matchesFirstInRange(InnerMatcher, PackArgs.begin(), PackArgs.end(),
                             Finder, Builder) != PackArgs.end())
       return true;
   }
   return matchesFirstInRange(InnerMatcher, Args.begin(), Args.end(), Finder,
                              Builder) != Args.end();
 }
 
+static 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(
       anyOf(cxxRecordDecl(HasStdParent),
             classTemplateSpecializationDecl(
                 HasStdParent, HasNoProgramDefinedTemplateArgument)));
 
   // Try to follow exactly CERT rule DCL58-CPP (this text is taken from C++

clang-tools-extra/test/clang-tidy/infrastructure/file-filter-symlinks.cpp

 // REQUIRES: shell
 
 // RUN: rm -rf %t
 // RUN: mkdir -p %t/dir1/dir2
 // RUN: echo 'class A { A(int); };' > %t/dir1/header.h
 // RUN: ln -s %t/dir1/header.h %t/dir1/header_alias.h
 //
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='dir1/dir2/\.\./header_alias\.h' %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER_ALIAS %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='dir1/dir2/\.\./header_alias\.h' -quiet %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER_ALIAS %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header_alias\.h' %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER_ALIAS %s
 // RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header_alias\.h' -quiet %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER_ALIAS %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header\.h' %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER %s
-// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header\.h' -quiet %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER %s
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header\.h' %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER %s --allow-empty
+// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='header\.h' -quiet %s -- -I %t 2>&1 | FileCheck --check-prefix=CHECK_HEADER %s --allow-empty
 
 // Check that `-header-filter` operates on the same file paths as paths in
 // diagnostics printed by ClangTidy.
 #include "dir1/dir2/../header_alias.h"
 // CHECK_HEADER_ALIAS: dir1/dir2/../header_alias.h:1:11: warning: single-argument constructors
 // CHECK_HEADER-NOT: warning:

clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp

 // CHECK-QUIET-NOT: warning:
 // CHECK2-NOT: warning:
 // CHECK2-QUIET-NOT: warning:
 // CHECK3-NOT: warning:
 // CHECK3-QUIET-NOT: warning:
 // 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
 
 int x = 123;
 auto x_ptr = TO_FLOAT_PTR(&x);
 // CHECK5: :[[@LINE-1]]:14: warning: do not use C-style cast to convert between unrelated types
 // CHECK5-NO-SYSTEM-HEADERS-NOT: :[[@LINE-2]]:14: warning: do not use C-style cast to convert between unrelated types

clang-tools-extra/test/clang-tidy/infrastructure/line-filter.cpp

 // CHECK: :[[@LINE-1]]:11: warning: single-argument constructors {{.*}}
 class B { B(int); };
 // CHECK-NOT: :[[@LINE-1]]:{{.*}} warning
 class C { C(int); };
 // CHECK: :[[@LINE-1]]:11: warning: single-argument constructors {{.*}}
 
 // CHECK-NOT: warning:
 
-// CHECK: Suppressed 3 warnings (1 in non-user code, 2 due to line filter)
+// CHECK: Suppressed 2 warnings (2 due to line filter)

clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp

 // RUN: clang-tidy -dump-config -system-headers=true | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
 // RUN: clang-tidy -dump-config -system-headers=false | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
 // RUN: clang-tidy -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
 // RUN: clang-tidy -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
 
 // RUN: clang-tidy -system-headers=true -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
 // RUN: clang-tidy -system-headers=true -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-SYSTEM-HEADERS %s
 // RUN: clang-tidy -system-headers=false -config='SystemHeaders: true' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
 // RUN: clang-tidy -system-headers=false -config='SystemHeaders: false' -dump-config | FileCheck -check-prefix=CHECK-CONFIG-NO-SYSTEM-HEADERS %s
 
 // 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 %s --allow-empty
 // 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 %s --allow-empty
 
 #include <system_header.h>
 // CHECK-SYSTEM-HEADERS: system_header.h:1:13: warning: single-argument constructors must be marked explicit
 // CHECK-NO-SYSTEM-HEADERS-NOT: system_header.h:1:13: warning: single-argument constructors must be marked explicit
 
 // CHECK-CONFIG-NO-SYSTEM-HEADERS: SystemHeaders: false
 // CHECK-CONFIG-SYSTEM-HEADERS: SystemHeaders: true
 // CHECK-OPT-PRESENT: --system-headers