mirror of
https://github.com/llvm/llvm-project.git
synced 2025-04-26 10:16:07 +00:00
e4a8969e56
[clang-tidy] Avoid processing declarations in system headers 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 in the MatchASTConsumer class, so the logic can be reused by other tools. This behavior is currently off by default, and only clang-tidy enables skipping system headers. If wanted, this behavior can be activated by other tools in follow-up patches. I had to move MatchFinderOptions out of the MatchFinder class, because otherwise I could not set a default value for the "bool SkipSystemHeaders" member otherwise. The compiler error message was "default member initializer required before the end of its enclosing class". 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 "CXXRecordDecl" of the global anonymous union, see: https://github.com/llvm/llvm-project/issues/130618 I have not found a way to make this work. For now, document the technical debt introduced. 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 Co-authored-by: Carlos Gálvez <carlos.galvez@zenseact.com>
See llvm/docs/README.txt