[all-commits] [llvm/llvm-project] e4a896: [clang-tidy] Avoid processing declarations in syst...

[Carlos Galvez via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: e4a8969e56572371201863594b3a549de2e23f32
      https://github.com/llvm/llvm-project/commit/e4a8969e56572371201863594b3a549de2e23f32
  Author: Carlos Galvez <carlosgalvezp at gmail.com>
  Date:   2025-03-14 (Fri, 14 Mar 2025)

  Changed paths:
    M clang-tools-extra/clang-query/Query.cpp
    M clang-tools-extra/clang-tidy/ClangTidy.cpp
    M clang-tools-extra/clang-tidy/cert/DontModifyStdNamespaceCheck.cpp
    M clang-tools-extra/docs/ReleaseNotes.rst
    M clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-anon-record-fields.cpp
    M clang-tools-extra/test/clang-tidy/infrastructure/file-filter.cpp
    M clang-tools-extra/test/clang-tidy/infrastructure/system-headers.cpp
    M clang/docs/ReleaseNotes.rst
    M clang/include/clang/ASTMatchers/ASTMatchFinder.h
    M clang/lib/ASTMatchers/ASTMatchFinder.cpp
    M clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp

  Log Message:
  -----------
  [clang-tidy] Avoid processing declarations in system headers (#128150)

[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 at zenseact.com>



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications