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

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
njames93 added reviewers: carlosgalvezp, PiotrZSL.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
njames93 updated this revision to Diff 520620.
njames93 added a comment.
njames93 updated this revision to Diff 520918.
njames93 retitled this revision from "[clang-tidy] Set traversal scope to prevent analysis in headers" to "[clang-tidy][WIP] Set traversal scope to prevent analysis in headers".
njames93 published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fix failing unittests


njames93 added a comment.

Fix infrastructure test failures



================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:511
+
+    Consumers.push_back(std::make_unique<TraversalScopeConsumer>(
+        !Context.getOptions().SystemHeaders.value_or(false),
----------------
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.


================
Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:511
+
+    Consumers.push_back(std::make_unique<TraversalScopeConsumer>(
+        !Context.getOptions().SystemHeaders.value_or(false),
----------------
carlosgalvezp wrote:
> 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.
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


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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D150126

Files:
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tools-extra/clang-tidy/abseil/UpgradeDurationConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/file-filter-symlinks.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/line-filter.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D150126.520918.patch
Type: text/x-patch
Size: 14709 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230510/2f39f25c/attachment-0001.bin>