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 llvm#52959

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 llvm#52959

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 llvm#52959

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 llvm#52959

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:
llvm#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 llvm#52959

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:
llvm#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 llvm#52959

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:
llvm#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 llvm#52959

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:
llvm#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 llvm#52959

…8150)

[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:
llvm#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 llvm#52959

Co-authored-by: Carlos Gálvez <[email protected]>