https://github.com/VladimirMakaev updated https://github.com/llvm/llvm-project/pull/194370
>From 392b94539bfba5f199f0c4af0f49cae69196f978 Mon Sep 17 00:00:00 2001 From: Vladimir Makaev <[email protected]> Date: Tue, 5 May 2026 11:19:37 -0700 Subject: [PATCH] Add experimental header-filter scope flag Add an experimental `-experimental-header-filter-scope` option to clang-tidy that skips AST matching for declarations outside the active `-header-filter` and `-exclude-header-filter` scope, instead of only filtering diagnostics after matching. Motivation clang-tidy currently matches every non-system-header declaration and then discards diagnostics that fall outside `-header-filter` in its diagnostic consumer. For narrow header-filter runs on large translation units this wastes time matching declarations that cannot produce user-visible diagnostics. This patch makes that pruning opt-in via a new boolean flag. When enabled, the AST matcher skips matching `Decl` nodes whose source locations do not pass the header-filter and exclude-header-filter regexes. It may also skip subtrees for filtered-out declarations whose lexical children cannot contain an in-scope declaration. Filtered-out `DeclContext` nodes with in-scope lexical children are still traversed, so in-scope declarations included inside wrapper contexts can still be visited. Parent maps remain intact because this uses matcher traversal filtering rather than `setTraversalScope`, so `hasParent` and `hasAncestor` matchers continue to work correctly. Design * Add a generic `std::function<bool(SourceLocation)> ShouldSkipLocation` callback to `MatchFinder::MatchFinderOptions` in clang core. No clang-tidy concepts leak into the ASTMatchers library. * Add a `HeaderFilterLocationFilter` utility in `HeaderFilterHelpers.h` that evaluates clang-tidy's header-filter regexes with a per-`FileID` cache. Both the diagnostic consumer and the matching-layer skip use this helper for shared regex and file handling, while each caller still passes the source location that is appropriate for its own filtering decision. * Refactor `ClangTidyDiagnosticConsumer` to use `HeaderFilterLocationFilter` instead of owning separate `llvm::Regex` objects directly. * Keep a lazy memoized `Decl`-subtree cache in matcher traversal. For an out-of-scope `DeclContext`, traversal checks whether any lexical child declaration is itself in scope or recursively contains an in-scope declaration; if not, the whole subtree can be pruned. This recovers most regular out-of-scope-header pruning while preserving wrapper cases such as an excluded namespace header that includes an in-scope header inside the namespace body. The callback is only consulted for `Decl` nodes with valid source locations, so the translation-unit root is never skipped. This is a best-effort declaration pruning mechanism: matchers using `hasDescendant` or `forEachDescendant` from an in-scope root may still visit declarations in skipped regions through the internal child-match visitor, which is the same limitation that `IgnoreSystemHeaders` has. Performance Local benchmarking used full compile-database folder slices with module-specific header filters and three repetitions per slice. Median results on this LLVM checkout with `run-clang-tidy.py -j8` and `-*,llvm-namespace-comment,llvm-qualified-auto,llvm-else-after-return,misc-use-internal-linkage`: * `clang/lib/Sema`, 86 TUs: real time 51.12s -> 44.40s, check time 58.2820s -> 39.6766s, identical warning headlines. * `clang/lib/AST`, 114 TUs: real time 38.20s -> 35.51s, check time 44.6134s -> 33.4866s, identical warning headlines. * `llvm/lib/Transforms/Scalar`, 82 TUs: real time 22.18s -> 20.09s, check time 25.1804s -> 17.0532s, identical warning headlines. * `llvm/lib/Target/X86`, 89 TUs: real time 31.63s -> 29.83s, check time 38.3378s -> 28.2155s, identical warning headlines. * `clang-tools-extra/clang-tidy/readability`, 62 TUs: real time 33.67s -> 29.99s, check time 40.4961s -> 26.4269s, identical warning headlines. A full-tree compile-database run was deliberately not used as evidence because it timed out locally; the module-slice benchmark matches the intended workflow of focusing clang-tidy on a related folder in a large monorepo. Completeness The patch includes full `.clang-tidy` YAML config support (`ExperimentalHeaderFilterScope`), `--dump-config`, option merging/defaults, `run-clang-tidy.py` support, release notes, clang-tidy documentation, CLI/config/exclude-header-filter/line-filter/run-clang-tidy regression tests, a positive control for main-file diagnostics, wrapper-`DeclContext` coverage, macro expansion coverage, and ASTMatchers unit tests for the generic `ShouldSkipLocation` hook. Known limitations Checks that build cross-file databases, such as `misc-confusable-identifiers` and `misc-unused-using-decls`, may produce different results, including false negatives or false positives, when this flag is enabled because declarations outside the filter scope are not matched. This is documented and the flag is explicitly marked experimental. Prior art: D150126, PR #128150 (reverted), PR #151035 (foundation). --- clang-tools-extra/clang-tidy/ClangTidy.cpp | 11 ++ .../ClangTidyDiagnosticConsumer.cpp | 33 ++-- .../clang-tidy/ClangTidyDiagnosticConsumer.h | 15 +- .../clang-tidy/ClangTidyOptions.cpp | 5 + .../clang-tidy/ClangTidyOptions.h | 7 + .../clang-tidy/HeaderFilterHelpers.h | 64 ++++++ .../clang-tidy/tool/ClangTidyMain.cpp | 27 +++ .../clang-tidy/tool/run-clang-tidy.py | 18 ++ clang-tools-extra/docs/ReleaseNotes.rst | 9 + clang-tools-extra/docs/clang-tidy/index.rst | 29 ++- .../Inputs/config-files/.clang-tidy | 1 + .../Inputs/config-files/1/.clang-tidy | 1 + .../Inputs/config-files/3/.clang-tidy | 1 + .../confusable_header.h | 1 + .../macro_def_header.h | 6 + .../macro_expansion_header.h | 3 + .../wrapper_header.h | 3 + .../wrapper_kept_header.h | 1 + .../infrastructure/config-files.cpp | 7 +- .../clang-tidy/infrastructure/diagnostic.cpp | 2 + .../infrastructure/dump-config-filtering.cpp | 2 + ...xperimental-header-filter-scope-macros.cpp | 18 ++ ...perimental-header-filter-scope-wrapper.cpp | 10 + .../experimental-header-filter-scope.cpp | 22 +++ .../infrastructure/run-clang-tidy.cpp | 15 ++ clang/docs/ReleaseNotes.rst | 2 + .../clang/ASTMatchers/ASTMatchFinder.h | 18 ++ clang/lib/ASTMatchers/ASTMatchFinder.cpp | 67 ++++++- .../ASTMatchers/ASTMatchersInternalTest.cpp | 182 ++++++++++++++++++ 29 files changed, 542 insertions(+), 38 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/HeaderFilterHelpers.h create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/confusable_header.h create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_def_header.h create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_expansion_header.h create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_header.h create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_kept_header.h create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-macros.cpp create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-wrapper.cpp create mode 100644 clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope.cpp diff --git a/clang-tools-extra/clang-tidy/ClangTidy.cpp b/clang-tools-extra/clang-tidy/ClangTidy.cpp index 05c8fd02fe86a..e2a9d41600736 100644 --- a/clang-tools-extra/clang-tidy/ClangTidy.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidy.cpp @@ -20,6 +20,7 @@ #include "ClangTidyModule.h" #include "ClangTidyProfiling.h" #include "ExpandModularHeadersPPCallbacks.h" +#include "HeaderFilterHelpers.h" #include "clang-tidy-config.h" #include "clang/AST/ASTConsumer.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -448,6 +449,16 @@ ClangTidyASTConsumerFactory::createASTConsumer(CompilerInstance &Compiler, if (!Context.getOptions().SystemHeaders.value_or(false)) FinderOptions.IgnoreSystemHeaders = true; + if (Context.getOptions().ExperimentalHeaderFilterScope.value_or(false)) { + auto LocationFilter = std::make_shared<HeaderFilterLocationFilter>( + Context.getOptions().HeaderFilterRegex.value_or(""), + Context.getOptions().ExcludeHeaderFilterRegex.value_or("")); + FinderOptions.ShouldSkipLocation = [LocationFilter, + SM](SourceLocation Location) { + return !LocationFilter->shouldInclude(Location, *SM); + }; + } + auto Finder = std::make_unique<ast_matchers::MatchFinder>(std::move(FinderOptions)); diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp index f7232645a329c..d35cd954e1fd7 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp @@ -36,7 +36,6 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringMap.h" #include "llvm/Support/FormatVariadic.h" -#include "llvm/Support/Regex.h" #include <optional> #include <tuple> #include <utility> @@ -372,6 +371,7 @@ void ClangTidyDiagnosticConsumer::BeginSourceFile(const LangOptions &LangOpts, assert(!InSourceFile); InSourceFile = true; + HeaderFilterLocation.reset(); } void ClangTidyDiagnosticConsumer::EndSourceFile() { @@ -568,9 +568,10 @@ void ClangTidyDiagnosticConsumer::forwardDiagnostic(const Diagnostic &Info) { void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location, const SourceManager &Sources) { - // Invalid location may mean a diagnostic in a command line, don't skip these. if (!Location.isValid()) { - LastErrorRelatesToUserCode = true; + LastErrorRelatesToUserCode = + LastErrorRelatesToUserCode || + getHeaderFilterLocationFilter().shouldInclude(Location, Sources); LastErrorPassesLineFilter = true; return; } @@ -579,6 +580,10 @@ void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location, (Sources.isInSystemHeader(Location) || Sources.isInSystemMacro(Location))) return; + LastErrorRelatesToUserCode = + LastErrorRelatesToUserCode || + getHeaderFilterLocationFilter().shouldInclude(Location, Sources); + // FIXME: We start with a conservative approach here, but the actual type of // location needed depends on the check (in particular, where this check wants // to apply fixes). @@ -588,34 +593,24 @@ void ClangTidyDiagnosticConsumer::checkFilters(SourceLocation Location, // -DMACRO definitions on the command line have locations in a virtual buffer // that doesn't have a FileEntry. Don't skip these as well. if (!File) { - LastErrorRelatesToUserCode = true; LastErrorPassesLineFilter = true; return; } const StringRef FileName(File->getName()); - LastErrorRelatesToUserCode = LastErrorRelatesToUserCode || - Sources.isInMainFile(Location) || - (getHeaderFilter()->match(FileName) && - !getExcludeHeaderFilter()->match(FileName)); const unsigned LineNumber = Sources.getExpansionLineNumber(Location); LastErrorPassesLineFilter = LastErrorPassesLineFilter || passesLineFilter(FileName, LineNumber); } -llvm::Regex *ClangTidyDiagnosticConsumer::getHeaderFilter() { - if (!HeaderFilter) - HeaderFilter = std::make_unique<llvm::Regex>( - Context.getOptions().HeaderFilterRegex.value_or("")); - return HeaderFilter.get(); -} - -llvm::Regex *ClangTidyDiagnosticConsumer::getExcludeHeaderFilter() { - if (!ExcludeHeaderFilter) - ExcludeHeaderFilter = std::make_unique<llvm::Regex>( +HeaderFilterLocationFilter & +ClangTidyDiagnosticConsumer::getHeaderFilterLocationFilter() { + if (!HeaderFilterLocation) + HeaderFilterLocation = std::make_unique<HeaderFilterLocationFilter>( + Context.getOptions().HeaderFilterRegex.value_or(""), Context.getOptions().ExcludeHeaderFilterRegex.value_or("")); - return ExcludeHeaderFilter.get(); + return *HeaderFilterLocation; } void ClangTidyDiagnosticConsumer::removeIncompatibleErrors() { diff --git a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h index 8de5778dfefb0..241e70f915677 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h +++ b/clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h @@ -12,12 +12,12 @@ #include "ClangTidyOptions.h" #include "ClangTidyProfiling.h" #include "FileExtensionsSet.h" +#include "HeaderFilterHelpers.h" #include "NoLintDirectiveHandler.h" #include "clang/Basic/Diagnostic.h" #include "clang/Tooling/Core/Diagnostic.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringSet.h" -#include "llvm/Support/Regex.h" #include <optional> #include <utility> @@ -310,13 +310,9 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer { void removeIncompatibleErrors(); void removeDuplicatedDiagnosticsOfAliasCheckers(); - /// Returns the \c HeaderFilter constructed for the options set in the - /// context. - llvm::Regex *getHeaderFilter(); - - /// Returns the \c ExcludeHeaderFilter constructed for the options set in the - /// context. - llvm::Regex *getExcludeHeaderFilter(); + /// Returns the cached header-filter evaluator for the current translation + /// unit. + HeaderFilterLocationFilter &getHeaderFilterLocationFilter(); /// Updates \c LastErrorRelatesToUserCode and LastErrorPassesLineFilter /// according to the diagnostic \p Location. @@ -331,8 +327,7 @@ class ClangTidyDiagnosticConsumer : public DiagnosticConsumer { bool GetFixesFromNotes; bool EnableNolintBlocks; std::vector<ClangTidyError> Errors; - std::unique_ptr<llvm::Regex> HeaderFilter; - std::unique_ptr<llvm::Regex> ExcludeHeaderFilter; + std::unique_ptr<HeaderFilterLocationFilter> HeaderFilterLocation; bool LastErrorRelatesToUserCode = false; bool LastErrorPassesLineFilter = false; bool LastErrorWasIgnored = false; diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp index 0a0f392346f6d..fb4ae7f472547 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.cpp @@ -242,6 +242,8 @@ template <> struct MappingTraits<ClangTidyOptions> { IO.mapOptional("InheritParentConfig", Options.InheritParentConfig); IO.mapOptional("UseColor", Options.UseColor); IO.mapOptional("SystemHeaders", Options.SystemHeaders); + IO.mapOptional("ExperimentalHeaderFilterScope", + Options.ExperimentalHeaderFilterScope); IO.mapOptional("CustomChecks", Options.CustomChecks); } }; @@ -259,6 +261,7 @@ ClangTidyOptions ClangTidyOptions::getDefaults() { Options.HeaderFilterRegex = ".*"; Options.ExcludeHeaderFilterRegex = ""; Options.SystemHeaders = false; + Options.ExperimentalHeaderFilterScope = false; Options.FormatStyle = "none"; Options.User = std::nullopt; Options.RemovedArgs = std::nullopt; @@ -300,6 +303,8 @@ ClangTidyOptions &ClangTidyOptions::mergeWith(const ClangTidyOptions &Other, overrideValue(HeaderFilterRegex, Other.HeaderFilterRegex); overrideValue(ExcludeHeaderFilterRegex, Other.ExcludeHeaderFilterRegex); overrideValue(SystemHeaders, Other.SystemHeaders); + overrideValue(ExperimentalHeaderFilterScope, + Other.ExperimentalHeaderFilterScope); overrideValue(FormatStyle, Other.FormatStyle); overrideValue(User, Other.User); overrideValue(UseColor, Other.UseColor); diff --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h b/clang-tools-extra/clang-tidy/ClangTidyOptions.h index 73fdbabd5bdba..aa255ea6de52c 100644 --- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h +++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h @@ -92,6 +92,13 @@ struct ClangTidyOptions { /// Output warnings from system headers matching \c HeaderFilterRegex. std::optional<bool> SystemHeaders; + /// When set, clang-tidy experimentally skips AST matching for declarations + /// in headers that do not match \c HeaderFilterRegex or that match + /// \c ExcludeHeaderFilterRegex. This is a semantic opt-in: checks that rely + /// on declarations outside the filtered headers can produce different + /// results, including false negatives or false positives. + std::optional<bool> ExperimentalHeaderFilterScope; + /// Format code around applied fixes with clang-format using this /// style. /// diff --git a/clang-tools-extra/clang-tidy/HeaderFilterHelpers.h b/clang-tools-extra/clang-tidy/HeaderFilterHelpers.h new file mode 100644 index 0000000000000..23fad8b9533ad --- /dev/null +++ b/clang-tools-extra/clang-tidy/HeaderFilterHelpers.h @@ -0,0 +1,64 @@ +//===--- HeaderFilterHelpers.h - clang-tidy header filtering ----*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HEADERFILTERHELPERS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HEADERFILTERHELPERS_H + +#include "clang/Basic/SourceManager.h" +#include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" + +namespace clang::tidy { + +/// Evaluates clang-tidy's header filters for source locations and caches the +/// per-file results for a translation unit. +class HeaderFilterLocationFilter { +public: + HeaderFilterLocationFilter(llvm::StringRef HeaderFilterRegex, + llvm::StringRef ExcludeHeaderFilterRegex) + : HeaderFilter(HeaderFilterRegex), + ExcludeHeaderFilter(ExcludeHeaderFilterRegex) {} + + /// Returns true when the location should be treated as in scope for + /// clang-tidy's header filters. + /// + /// Main-file locations are always in scope. Invalid locations and locations + /// without a FileEntry (such as command-line buffers) are also treated as in + /// scope to match clang-tidy's diagnostic filtering behavior. + bool shouldInclude(SourceLocation Location, const SourceManager &Sources) { + if (!Location.isValid()) + return true; + + if (Sources.isInMainFile(Location)) + return true; + + const FileID FID = Sources.getDecomposedExpansionLoc(Location).first; + if (const auto It = Cache.find(FID); It != Cache.end()) + return It->second; + + bool Result = true; + if (const OptionalFileEntryRef File = Sources.getFileEntryRefForID(FID)) { + const llvm::StringRef FileName = File->getName(); + Result = + HeaderFilter.match(FileName) && !ExcludeHeaderFilter.match(FileName); + } + + Cache[FID] = Result; + return Result; + } + +private: + llvm::Regex HeaderFilter; + llvm::Regex ExcludeHeaderFilter; + llvm::DenseMap<FileID, bool> Cache; +}; + +} // namespace clang::tidy + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HEADERFILTERHELPERS_H diff --git a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp index f61e2f40ed03b..6c808185f22f9 100644 --- a/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp +++ b/clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp @@ -63,6 +63,9 @@ Configuration files: CustomChecks - Array of user defined checks based on Clang-Query syntax. ExcludeHeaderFilterRegex - Same as '--exclude-header-filter'. + ExperimentalHeaderFilterScope + - Same as + '--experimental-header-filter-scope'. ExtraArgs - Same as '--extra-arg'. ExtraArgsBefore - Same as '--extra-arg-before'. FormatStyle - Same as '--format-style'. @@ -95,6 +98,7 @@ Configuration files: HeaderFileExtensions: ['', 'h','hh','hpp','hxx'] ImplementationFileExtensions: ['c','cc','cpp','cxx'] HeaderFilterRegex: '.*' + ExperimentalHeaderFilterScope: false FormatStyle: none InheritParentConfig: true User: user @@ -158,6 +162,25 @@ option in .clang-tidy file, if any. cl::init(""), cl::cat(ClangTidyCategory)); +static cl::opt<bool> + ExperimentalHeaderFilterScope("experimental-header-filter-scope", desc(R"( +When enabled, clang-tidy experimentally skips +AST matching for declarations in headers that +do not match -header-filter or that match +-exclude-header-filter. +This can improve performance for narrow +header-filter runs, but checks that rely on +declarations outside the filtered headers can +produce different results, including false +negatives or false positives. +This is an opt-in semantic mode, not only a +diagnostic filtering mode. +This option overrides the +'ExperimentalHeaderFilterScope' option in +.clang-tidy file, if any. +)"), + cl::init(false), cl::cat(ClangTidyCategory)); + static cl::opt<bool> SystemHeaders("system-headers", desc(R"( Display the errors from system headers. This option overrides the 'SystemHeaders' option @@ -412,6 +435,7 @@ createOptionsProvider(llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) { DefaultOptions.HeaderFilterRegex = HeaderFilter; DefaultOptions.ExcludeHeaderFilterRegex = ExcludeHeaderFilter; DefaultOptions.SystemHeaders = SystemHeaders; + DefaultOptions.ExperimentalHeaderFilterScope = ExperimentalHeaderFilterScope; DefaultOptions.FormatStyle = FormatStyle; DefaultOptions.User = llvm::sys::Process::GetEnv("USER"); // USERNAME is used on Windows. @@ -429,6 +453,9 @@ createOptionsProvider(llvm::IntrusiveRefCntPtr<vfs::FileSystem> FS) { OverrideOptions.ExcludeHeaderFilterRegex = ExcludeHeaderFilter; if (SystemHeaders.getNumOccurrences() > 0) OverrideOptions.SystemHeaders = SystemHeaders; + if (ExperimentalHeaderFilterScope.getNumOccurrences() > 0) + OverrideOptions.ExperimentalHeaderFilterScope = + ExperimentalHeaderFilterScope; if (FormatStyle.getNumOccurrences() > 0) OverrideOptions.FormatStyle = FormatStyle; if (UseColor.getNumOccurrences() > 0) diff --git a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py index f827ef492f01c..0873386d13196 100755 --- a/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py +++ b/clang-tools-extra/clang-tidy/tool/run-clang-tidy.py @@ -94,6 +94,7 @@ def get_tidy_invocation( tmpdir: Optional[str], build_path: str, header_filter: Optional[str], + experimental_header_filter_scope: Optional[bool], allow_enabling_alpha_checkers: bool, extra_arg: List[str], extra_arg_before: List[str], @@ -117,6 +118,11 @@ def get_tidy_invocation( start.append(f"--exclude-header-filter={exclude_header_filter}") if header_filter is not None: start.append(f"-header-filter={header_filter}") + if experimental_header_filter_scope is not None: + if experimental_header_filter_scope: + start.append("--experimental-header-filter-scope") + else: + start.append("--experimental-header-filter-scope=false") if line_filter is not None: start.append(f"-line-filter={line_filter}") if use_color is not None: @@ -378,6 +384,7 @@ async def run_tidy( tmpdir, build_path, args.header_filter, + args.experimental_header_filter_scope, args.allow_enabling_alpha_checkers, args.extra_arg, args.extra_arg_before, @@ -478,6 +485,16 @@ async def main() -> None: "the main file of each translation unit are always " "displayed.", ) + parser.add_argument( + "-experimental-header-filter-scope", + type=strtobool, + nargs="?", + const=True, + default=None, + help="Enable experimental semantic AST scoping to headers " + "that match -header-filter and do not match " + "-exclude-header-filter.", + ) parser.add_argument( "-source-filter", default=None, @@ -647,6 +664,7 @@ async def main() -> None: None, build_path, args.header_filter, + args.experimental_header_filter_scope, args.allow_enabling_alpha_checkers, args.extra_arg, args.extra_arg_before, diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 2419917778182..69083d6aec9c1 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -111,6 +111,15 @@ Improvements to clang-tidy - Improved :program:`clang-tidy` ``-store-check-profile`` by generating valid JSON when the source file path contains characters that require JSON escaping. +- Added an experimental :program:`clang-tidy` + ``-experimental-header-filter-scope`` option that skips AST matching for + declarations in headers that do not match ``-header-filter`` (or that match + ``-exclude-header-filter``). This can improve performance for narrow + header-filter runs, but it is an opt-in semantic mode: checks that need AST + visibility outside the filtered headers, such as checks that build cross-file + state, can produce different results including false negatives or false + positives. + New checks ^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/index.rst b/clang-tools-extra/docs/clang-tidy/index.rst index db7f2deade9ca..b63e79d949191 100644 --- a/clang-tools-extra/docs/clang-tidy/index.rst +++ b/clang-tools-extra/docs/clang-tidy/index.rst @@ -185,6 +185,22 @@ An overview of all the command-line options: Can be used together with -line-filter. This option overrides the 'ExcludeHeaderFilterRegex' option in .clang-tidy file, if any. + --experimental-header-filter-scope + - When enabled, clang-tidy experimentally + skips AST matching for declarations in + headers that do not match -header-filter + or that match -exclude-header-filter. + This can improve performance for narrow + header-filter runs, but checks that rely + on declarations outside the filtered + headers can produce different results, + including false negatives or false + positives. + This is an opt-in semantic mode, not + only a diagnostic filtering mode. + This option overrides the + 'ExperimentalHeaderFilterScope' option in + .clang-tidy file, if any. --experimental-custom-checks - Enable experimental clang-query based custom checks. see https://clang.llvm.org/extra/clang-tidy/QueryBasedCustomChecks.html. @@ -320,6 +336,12 @@ An overview of all the command-line options: CustomChecks - Array of user defined checks based on Clang-Query syntax. ExcludeHeaderFilterRegex - Same as '--exclude-header-filter'. + ExperimentalHeaderFilterScope + - Same as + '--experimental-header-filter-scope'. This + can change results for checks that build + cross-file state or otherwise rely on + declarations outside the filtered headers. ExtraArgs - Same as '--extra-arg'. ExtraArgsBefore - Same as '--extra-arg-before'. FormatStyle - Same as '--format-style'. @@ -352,6 +374,7 @@ An overview of all the command-line options: HeaderFileExtensions: ['', 'h','hh','hpp','hxx'] ImplementationFileExtensions: ['c','cc','cpp','cxx'] HeaderFilterRegex: '.*' + ExperimentalHeaderFilterScope: false FormatStyle: none InheritParentConfig: true User: user @@ -414,9 +437,9 @@ can be generated by build systems like CMake (using ``-DCMAKE_EXPORT_COMPILE_COMMANDS=ON``) or by tools like `Bear`_. The script supports most of the same options as :program:`clang-tidy` itself, -including ``-checks=``, ``-fix``, ``-header-filter=``, and configuration -options. Run ``run-clang-tidy.py --help`` for a complete list of available -options. +including ``-checks=``, ``-fix``, ``-header-filter=``, +``-experimental-header-filter-scope``, and configuration options. Run +``run-clang-tidy.py --help`` for a complete list of available options. Example invocations: diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/.clang-tidy index 83605c85dd92c..64da72adc342e 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/.clang-tidy +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/.clang-tidy @@ -1,3 +1,4 @@ Checks: 'from-parent' HeaderFilterRegex: 'parent' ExcludeHeaderFilterRegex: 'exc-parent' +ExperimentalHeaderFilterScope: true diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/1/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/1/.clang-tidy index c37f16bc2d7d2..1991cd343e076 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/1/.clang-tidy +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/1/.clang-tidy @@ -1,3 +1,4 @@ Checks: 'from-child1' HeaderFilterRegex: 'child1' ExcludeHeaderFilterRegex: 'exc-child1' +ExperimentalHeaderFilterScope: false diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy index 9365108255bd8..39831e357b09e 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/config-files/3/.clang-tidy @@ -2,3 +2,4 @@ InheritParentConfig: true Checks: 'from-child3' HeaderFilterRegex: 'child3' ExcludeHeaderFilterRegex: 'exc-child3' +ExperimentalHeaderFilterScope: false diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/confusable_header.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/confusable_header.h new file mode 100644 index 0000000000000..659f2a3879880 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/confusable_header.h @@ -0,0 +1 @@ +int l0 = 0; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_def_header.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_def_header.h new file mode 100644 index 0000000000000..c1b843641bcdc --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_def_header.h @@ -0,0 +1,6 @@ +#ifndef MACRO_DEF_HEADER_H +#define MACRO_DEF_HEADER_H + +#define DEFINE_CONFUSABLE(Name) int Name = 0; + +#endif diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_expansion_header.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_expansion_header.h new file mode 100644 index 0000000000000..32e61671fd46f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/macro_expansion_header.h @@ -0,0 +1,3 @@ +#include "macro_def_header.h" + +DEFINE_CONFUSABLE(l0) diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_header.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_header.h new file mode 100644 index 0000000000000..74bc3497ca2d3 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_header.h @@ -0,0 +1,3 @@ +namespace ns { +#include "wrapper_kept_header.h" +} diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_kept_header.h b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_kept_header.h new file mode 100644 index 0000000000000..659f2a3879880 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/Inputs/experimental-header-filter-scope/wrapper_kept_header.h @@ -0,0 +1 @@ +int l0 = 0; diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp index 44d43ebbf8d20..f17d6cf138371 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/config-files.cpp @@ -2,22 +2,27 @@ // CHECK-BASE: Checks: {{.*}}from-parent // CHECK-BASE: HeaderFilterRegex: parent // CHECK-BASE: ExcludeHeaderFilterRegex: exc-parent +// CHECK-BASE: ExperimentalHeaderFilterScope: true // RUN: clang-tidy -dump-config %S/Inputs/config-files/1/- -- | FileCheck %s -check-prefix=CHECK-CHILD1 // CHECK-CHILD1: Checks: {{.*}}from-child1 // CHECK-CHILD1: HeaderFilterRegex: child1 // CHECK-CHILD1: ExcludeHeaderFilterRegex: exc-child1 +// CHECK-CHILD1: ExperimentalHeaderFilterScope: false // RUN: clang-tidy -dump-config %S/Inputs/config-files/2/- -- | FileCheck %s -check-prefix=CHECK-CHILD2 // CHECK-CHILD2: Checks: {{.*}}from-parent // CHECK-CHILD2: HeaderFilterRegex: parent // CHECK-CHILD2: ExcludeHeaderFilterRegex: exc-parent +// CHECK-CHILD2: ExperimentalHeaderFilterScope: true // RUN: clang-tidy -dump-config %S/Inputs/config-files/3/- -- | FileCheck %s -check-prefix=CHECK-CHILD3 // CHECK-CHILD3: Checks: {{.*}}from-parent,from-child3 // CHECK-CHILD3: HeaderFilterRegex: child3 // CHECK-CHILD3: ExcludeHeaderFilterRegex: exc-child3 -// RUN: clang-tidy -dump-config -checks='from-command-line' -header-filter='from command line' -exclude-header-filter='from_command_line' %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-COMMAND-LINE +// CHECK-CHILD3: ExperimentalHeaderFilterScope: false +// RUN: clang-tidy -dump-config -checks='from-command-line' -header-filter='from command line' -exclude-header-filter='from_command_line' -experimental-header-filter-scope=false %S/Inputs/config-files/- -- | FileCheck %s -check-prefix=CHECK-COMMAND-LINE // CHECK-COMMAND-LINE: Checks: {{.*}}from-parent,from-command-line // CHECK-COMMAND-LINE: HeaderFilterRegex: from command line // CHECK-COMMAND-LINE: ExcludeHeaderFilterRegex: from_command_line +// CHECK-COMMAND-LINE: ExperimentalHeaderFilterScope: false // For this test we have to use names of the real checks because otherwise values are ignored. // Running with the old key: <Key>, value: <value> CheckOptions diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp index a9d140bc9318e..b31f57fed9eab 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/diagnostic.cpp @@ -1,4 +1,5 @@ // RUN: not clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s +// RUN: not clang-tidy -checks='-*,modernize-use-override' -header-filter='' -experimental-header-filter-scope %s.nonexistent.cpp -- | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s // RUN: not clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s // RUN: not clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s @@ -6,6 +7,7 @@ // // Now repeat the tests and ensure no other errors appear on stderr: // RUN: not clang-tidy -checks='-*,modernize-use-override' %s.nonexistent.cpp -- 2>&1 | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s +// RUN: not clang-tidy -checks='-*,modernize-use-override' -header-filter='' -experimental-header-filter-scope %s.nonexistent.cpp -- 2>&1 | FileCheck -check-prefix=CHECK1 -implicit-check-not='{{warning:|error:}}' %s // RUN: not clang-tidy -checks='-*,clang-diagnostic-*,google-explicit-constructor' %s -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK2 -implicit-check-not='{{warning:|error:}}' %s // RUN: not clang-tidy -checks='-*,google-explicit-constructor,clang-diagnostic-literal-conversion' %s -- -fan-unknown-option 2>&1 | FileCheck -check-prefix=CHECK3 -implicit-check-not='{{warning:|error:}}' %s // RUN: clang-tidy -checks='-*,modernize-use-override,clang-diagnostic-macro-redefined' %s -- -DMACRO_FROM_COMMAND_LINE 2>&1 | FileCheck -check-prefix=CHECK4 -implicit-check-not='{{warning:|error:}}' %s diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/dump-config-filtering.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/dump-config-filtering.cpp index f22ec7edb1775..5f88968a3d163 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/dump-config-filtering.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/dump-config-filtering.cpp @@ -5,8 +5,10 @@ // CHECK-NEXT: misc-unused-parameters.IgnoreVirtual: 'false' // CHECK-NEXT: misc-unused-parameters.StrictMode: 'false' // CHECK-NEXT: SystemHeaders: false +// CHECK-NEXT: ExperimentalHeaderFilterScope: false // CHECK-DISABLED: CheckOptions: {} // CHECK-DISABLED-NEXT: SystemHeaders: false +// CHECK-DISABLED-NEXT: ExperimentalHeaderFilterScope: false int main() { return 0; } diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-macros.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-macros.cpp new file mode 100644 index 0000000000000..29ee6ce2c9e77 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-macros.cpp @@ -0,0 +1,18 @@ +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' -experimental-header-filter-scope %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-SCOPED --allow-empty +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' -experimental-header-filter-scope %s -- -DEXPAND_IN_MAIN -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-MAIN-EXPANSION +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='macro_def_header\.h' -experimental-header-filter-scope %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-DEFINITION-ONLY --allow-empty + +#include "macro_def_header.h" + +#ifndef EXPAND_IN_MAIN +#include "macro_expansion_header.h" +#else +DEFINE_CONFUSABLE(l0) +#endif + +int lO = 1; +// CHECK-DEFAULT: :[[@LINE-1]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +// CHECK-SCOPED-NOT: warning: +// CHECK-MAIN-EXPANSION: :[[@LINE-3]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +// CHECK-DEFINITION-ONLY-NOT: warning: diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-wrapper.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-wrapper.cpp new file mode 100644 index 0000000000000..ce6f4936d6e18 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope-wrapper.cpp @@ -0,0 +1,10 @@ +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='wrapper_kept_header\.h' %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='wrapper_kept_header\.h' -experimental-header-filter-scope %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-SCOPED + +#include "wrapper_header.h" + +namespace ns { +int lO = 1; +// CHECK-DEFAULT: :[[@LINE-1]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +// CHECK-SCOPED: :[[@LINE-2]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +} // namespace ns diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope.cpp new file mode 100644 index 0000000000000..ef95fb4e83187 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/infrastructure/experimental-header-filter-scope.cpp @@ -0,0 +1,22 @@ +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' -experimental-header-filter-scope %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-CLI --allow-empty +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='.*' -exclude-header-filter='confusable_header\.h' %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-EXCLUDE +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='.*' -exclude-header-filter='confusable_header\.h' -config='{ExperimentalHeaderFilterScope: true}' %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-CONFIG --allow-empty +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' -line-filter='[{"name":"experimental-header-filter-scope.cpp","lines":[[1,20]]}]' %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-LINE-FILTER +// RUN: clang-tidy -checks='-*,misc-confusable-identifiers' -header-filter='does-not-match' -line-filter='[{"name":"experimental-header-filter-scope.cpp","lines":[[1,20]]}]' -experimental-header-filter-scope %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-LINE-FILTER-SCOPED --allow-empty +// +// Positive control: main-file diagnostics must still fire with scope enabled. +// RUN: clang-tidy -checks='-*,google-explicit-constructor' -header-filter='does-not-match' -experimental-header-filter-scope %s -- -I %S/Inputs/experimental-header-filter-scope 2>&1 | FileCheck %s --check-prefix=CHECK-MAIN-FILE + +#include "confusable_header.h" + +int lO = 1; +// CHECK-DEFAULT: :[[@LINE-1]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +// CHECK-CLI-NOT: warning: +// CHECK-EXCLUDE: :[[@LINE-3]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +// CHECK-CONFIG-NOT: warning: +// CHECK-LINE-FILTER: :[[@LINE-5]]:5: warning: 'lO' is confusable with 'l0' [misc-confusable-identifiers] +// CHECK-LINE-FILTER-SCOPED-NOT: warning: + +class A { A(int); }; +// CHECK-MAIN-FILE: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit{{.*}} [google-explicit-constructor] diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp b/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp index 6337686c58518..31b71a331a7ff 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp +++ b/clang-tools-extra/test/clang-tidy/infrastructure/run-clang-tidy.cpp @@ -14,6 +14,21 @@ // RUN: not %run_clang_tidy -j 1 "test.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-J1 // CHECK-J1: Running clang-tidy in 1 threads for +// RUN: rm -rf %t-scope +// RUN: mkdir -p %t-scope/include +// RUN: echo "[{\"directory\":\".\",\"command\":\"clang++ -c %/t-scope/test.cpp -I%/t-scope/include\",\"file\":\"%/t-scope/test.cpp\"}]" | sed -e 's/\\/\\\\/g' > %t-scope/compile_commands.json +// RUN: echo "Checks: '-*,misc-confusable-identifiers'" > %t-scope/.clang-tidy +// RUN: echo "WarningsAsErrors: '*'" >> %t-scope/.clang-tidy +// RUN: echo "int l0 = 0;" > %t-scope/include/confusable_header.h +// RUN: echo '#include "confusable_header.h"' > %t-scope/test.cpp +// RUN: echo 'int lO = 1;' >> %t-scope/test.cpp +// RUN: cd "%t-scope" +// RUN: not %run_clang_tidy -j 1 -header-filter=does-not-match "test.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-SCOPE-OFF +// RUN: %run_clang_tidy -j 1 -header-filter=does-not-match -experimental-header-filter-scope=true "test.cpp" 2>&1 | FileCheck %s --check-prefix=CHECK-SCOPE-ON +// CHECK-SCOPE-OFF: 'lO' is confusable with 'l0' +// CHECK-SCOPE-ON: Running clang-tidy in 1 threads for 1 files out of 1 in compilation database +// CHECK-SCOPE-ON-NOT: 'lO' is confusable with 'l0' + int main() { int* x = new int(); diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c408196c3816b..83f0da3114581 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -707,6 +707,8 @@ AST Matchers ------------ - Add ``functionTypeLoc`` matcher for matching ``FunctionTypeLoc``. - Add missing support for ``TraversalKind`` in some ``addMatcher()`` overloads. +- Add ``MatchFinderOptions::ShouldSkipLocation`` as a best-effort declaration + pruning callback for the outer AST matcher traversal. clang-format ------------ diff --git a/clang/include/clang/ASTMatchers/ASTMatchFinder.h b/clang/include/clang/ASTMatchers/ASTMatchFinder.h index b0ccbf22a4269..bb946c332bbf8 100644 --- a/clang/include/clang/ASTMatchers/ASTMatchFinder.h +++ b/clang/include/clang/ASTMatchers/ASTMatchFinder.h @@ -44,6 +44,7 @@ #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/StringMap.h" #include "llvm/Support/Timer.h" +#include <functional> #include <optional> namespace clang { @@ -145,6 +146,23 @@ class MatchFinder { /// Avoids matching declarations in system headers. bool IgnoreSystemHeaders{false}; + /// Skips matching for declarations based on location. + /// + /// When set and the callback returns true for a \c Decl node's location, + /// that declaration is not matched during the outer AST traversal. The + /// traversal may also skip that declaration's subtree when its lexical + /// children do not contain any declaration that the callback would keep. + /// The callback is only consulted for \c Decl nodes (not \c Stmt, \c Type, + /// etc.) during this outer traversal and only when the source location is + /// valid, so the translation-unit root is never skipped. + /// + /// \note This is a best-effort declaration pruning mechanism, not a general + /// source-location filter. Matchers that recursively visit children of an + /// in-scope node (e.g. + /// \c hasDescendant / \c forEachDescendant) may still reach declarations + /// in skipped regions through the internal child-match visitor. + std::function<bool(SourceLocation)> ShouldSkipLocation; + bool SkipDeclsInModules{false}; }; diff --git a/clang/lib/ASTMatchers/ASTMatchFinder.cpp b/clang/lib/ASTMatchers/ASTMatchFinder.cpp index 004a02c279099..c231795cd80cb 100644 --- a/clang/lib/ASTMatchers/ASTMatchFinder.cpp +++ b/clang/lib/ASTMatchers/ASTMatchFinder.cpp @@ -1366,12 +1366,57 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, return SM.isInSystemHeader(Loc); } - template <typename T> bool shouldSkipNode(T &Node) { - if (Options.IgnoreSystemHeaders && isInSystemHeader(getNodeLocation(Node))) - return true; + bool shouldSkipLocation(const Decl &Node) { + if (!Options.ShouldSkipLocation) + return false; + + SourceLocation Loc = getNodeLocation(Node); + return Loc.isValid() && Options.ShouldSkipLocation(Loc); + } + + bool shouldSkipNode(Decl &Node) { + return shouldSkipSystemHeaders(Node) || shouldSkipLocation(Node); + } + + bool declContextMayContainNonSkippedDecl(const DeclContext &DC) { + for (const Decl *Child : DC.decls()) + if (Child && declOrChildrenMayBeNonSkipped(*Child)) + return true; return false; } + bool declOrChildrenMayBeNonSkipped(const Decl &Node) { + auto It = DeclOrChildrenMayBeNonSkippedCache.find(&Node); + if (It != DeclOrChildrenMayBeNonSkippedCache.end()) + return It->second; + + bool MayBeNonSkipped = false; + if (!shouldSkipSystemHeaders(Node)) { + if (!shouldSkipLocation(Node)) { + MayBeNonSkipped = true; + } else if (const auto *DC = dyn_cast<DeclContext>(&Node)) { + MayBeNonSkipped = declContextMayContainNonSkippedDecl(*DC); + } + } + + DeclOrChildrenMayBeNonSkippedCache[&Node] = MayBeNonSkipped; + return MayBeNonSkipped; + } + + bool shouldSkipTraversal(Decl &Node) { + if (shouldSkipSystemHeaders(Node)) + return true; + if (!shouldSkipLocation(Node)) + return false; + + const auto *DC = dyn_cast<DeclContext>(&Node); + return !DC || !declContextMayContainNonSkippedDecl(*DC); + } + + template <typename T> bool shouldSkipNode(T &Node) { + return shouldSkipSystemHeaders(Node); + } + template <typename T> bool shouldSkipNode(T *Node) { return (Node == nullptr) || shouldSkipNode(*Node); } @@ -1380,6 +1425,12 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, bool shouldSkipNode(NestedNameSpecifier &) { return false; } + template <typename T> bool shouldSkipSystemHeaders(T &Node) { + if (Options.IgnoreSystemHeaders && isInSystemHeader(getNodeLocation(Node))) + return true; + return false; + } + /// Bucket to record map. /// /// Used to get the appropriate bucket for each matcher. @@ -1406,6 +1457,8 @@ class MatchASTVisitor : public RecursiveASTVisitor<MatchASTVisitor>, llvm::SmallPtrSet<const ObjCCompatibleAliasDecl *, 2>> CompatibleAliases; + llvm::DenseMap<const Decl *, bool> DeclOrChildrenMayBeNonSkippedCache; + // Maps (matcher, node) -> the match result for memoization. typedef std::map<MatchKey, MemoizedMatchResult> MemoizationMap; MemoizationMap ResultCache; @@ -1509,7 +1562,10 @@ bool MatchASTVisitor::objcClassIsDerivedFrom( } bool MatchASTVisitor::TraverseDecl(Decl *DeclNode) { - if (shouldSkipNode(DeclNode)) + if (!DeclNode) + return true; + + if (shouldSkipTraversal(*DeclNode)) return true; if (Options.SkipDeclsInModules && DeclNode->isInAnotherModuleUnit()) @@ -1536,7 +1592,8 @@ bool MatchASTVisitor::TraverseDecl(Decl *DeclNode) { ASTNodeNotSpelledInSourceScope RAII1(this, ScopedTraversal); ASTChildrenNotSpelledInSourceScope RAII2(this, ScopedChildren); - match(*DeclNode); + if (!shouldSkipNode(*DeclNode)) + match(*DeclNode); return RecursiveASTVisitor<MatchASTVisitor>::TraverseDecl(DeclNode); } diff --git a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp index 3fa71804710ac..82859fd0680a8 100644 --- a/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp +++ b/clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp @@ -345,6 +345,188 @@ TEST(IsInlineMatcher, IsInline) { // Windows. #ifndef _WIN32 +static bool isExpansionInFileEndingWith(const SourceManager &SM, + SourceLocation Location, + StringRef Suffix) { + const FileID FID = SM.getDecomposedExpansionLoc(Location).first; + const OptionalFileEntryRef File = SM.getFileEntryRefForID(FID); + return File && File->getName().ends_with(Suffix); +} + +TEST(MatchFinder, HonorsShouldSkipLocation) { + FileContentMappings M; + M.emplace_back("/other.h", "class HeaderDecl {};"); + auto AST = tooling::buildASTFromCodeWithArgs( + "#include \"other.h\"\n" + "class MainDecl {};\n", + {"-std=gnu++11", "-target", "i386-unknown-unknown", "-I/"}, "input.cc", + "clang-tool", std::make_shared<PCHContainerOperations>(), + tooling::getClangStripDependencyFileAdjuster(), M); + ASSERT_TRUE(AST); + + auto matchRecordDecls = [&](MatchFinder::MatchFinderOptions Options) { + struct RecordCallback : MatchFinder::MatchCallback { + explicit RecordCallback(std::vector<std::string> &Names) : Names(Names) {} + + void run(const MatchFinder::MatchResult &Result) override { + const auto *Record = Result.Nodes.getNodeAs<CXXRecordDecl>("record"); + ASSERT_NE(nullptr, Record); + Names.push_back(std::string(Record->getName())); + } + + std::vector<std::string> &Names; + }; + + std::vector<std::string> Names; + RecordCallback Callback(Names); + MatchFinder Finder(std::move(Options)); + Finder.addMatcher( + cxxRecordDecl(isDefinition(), unless(isImplicit())).bind("record"), + &Callback); + Finder.matchAST(AST->getASTContext()); + llvm::sort(Names); + return Names; + }; + + const auto AllMatches = matchRecordDecls({}); + ASSERT_EQ(2u, AllMatches.size()); + EXPECT_EQ("HeaderDecl", AllMatches[0]); + EXPECT_EQ("MainDecl", AllMatches[1]); + + MatchFinder::MatchFinderOptions ScopedOptions; + SourceManager &SM = AST->getSourceManager(); + ScopedOptions.ShouldSkipLocation = [&SM](SourceLocation Location) { + return !SM.isInMainFile(Location); + }; + const auto ScopedMatches = matchRecordDecls(std::move(ScopedOptions)); + ASSERT_EQ(1u, ScopedMatches.size()); + EXPECT_EQ("MainDecl", ScopedMatches[0]); +} + +TEST(MatchFinder, ShouldSkipLocationKeepsAncestorMatchersReachable) { + FileContentMappings M; + M.emplace_back("/kept.h", "class Kept {};"); + M.emplace_back("/wrapper.h", "namespace skipped {\n" + "#include \"kept.h\"\n" + "}"); + auto AST = tooling::buildASTFromCodeWithArgs( + "#include \"wrapper.h\"\n", + {"-std=gnu++11", "-target", "i386-unknown-unknown", "-I/"}, "input.cc", + "clang-tool", std::make_shared<PCHContainerOperations>(), + tooling::getClangStripDependencyFileAdjuster(), M); + ASSERT_TRUE(AST); + + MatchFinder::MatchFinderOptions Options; + SourceManager &SM = AST->getSourceManager(); + Options.ShouldSkipLocation = [&SM](SourceLocation Location) { + return isExpansionInFileEndingWith(SM, Location, "/wrapper.h"); + }; + + struct MatchCallback : MatchFinder::MatchCallback { + void run(const MatchFinder::MatchResult &Result) override { + ASSERT_NE(nullptr, Result.Nodes.getNodeAs<CXXRecordDecl>("record")); + Matched = true; + } + + bool Matched = false; + } Callback; + + MatchFinder Finder(std::move(Options)); + Finder.addMatcher( + cxxRecordDecl(isDefinition(), unless(isImplicit()), hasName("Kept"), + hasParent(namespaceDecl(hasName("skipped"))), + hasAncestor(namespaceDecl(hasName("skipped")))) + .bind("record"), + &Callback); + Finder.matchAST(AST->getASTContext()); + EXPECT_TRUE(Callback.Matched); +} + +TEST(MatchFinder, ShouldSkipLocationPrunesSkippedDeclContexts) { + FileContentMappings M; + M.emplace_back("/skipped.h", "namespace skipped { class Hidden {}; }"); + auto AST = tooling::buildASTFromCodeWithArgs( + "#include \"skipped.h\"\n", + {"-std=gnu++11", "-target", "i386-unknown-unknown", "-I/"}, "input.cc", + "clang-tool", std::make_shared<PCHContainerOperations>(), + tooling::getClangStripDependencyFileAdjuster(), M); + ASSERT_TRUE(AST); + + auto matchSkippedDeclNames = [&](MatchFinder::MatchFinderOptions Options) { + struct DeclCallback : MatchFinder::MatchCallback { + explicit DeclCallback(std::vector<std::string> &Names) : Names(Names) {} + + void run(const MatchFinder::MatchResult &Result) override { + const auto *Node = Result.Nodes.getNodeAs<NamedDecl>("decl"); + ASSERT_NE(nullptr, Node); + Names.push_back(std::string(Node->getName())); + } + + std::vector<std::string> &Names; + }; + + std::vector<std::string> Names; + DeclCallback Callback(Names); + MatchFinder Finder(std::move(Options)); + Finder.addMatcher( + decl(anyOf(namespaceDecl(hasName("skipped")), + cxxRecordDecl(isDefinition(), unless(isImplicit()), + hasName("Hidden")))) + .bind("decl"), + &Callback); + Finder.matchAST(AST->getASTContext()); + llvm::sort(Names); + return Names; + }; + + const auto AllMatches = matchSkippedDeclNames({}); + ASSERT_EQ(2u, AllMatches.size()); + EXPECT_EQ("Hidden", AllMatches[0]); + EXPECT_EQ("skipped", AllMatches[1]); + + MatchFinder::MatchFinderOptions ScopedOptions; + SourceManager &SM = AST->getSourceManager(); + ScopedOptions.ShouldSkipLocation = [&SM](SourceLocation Location) { + return isExpansionInFileEndingWith(SM, Location, "/skipped.h"); + }; + EXPECT_TRUE(matchSkippedDeclNames(std::move(ScopedOptions)).empty()); +} + +TEST(MatchFinder, ShouldSkipLocationPrunesSkippedSubtrees) { + FileContentMappings M; + M.emplace_back("/skipped.h", "void hidden() { int local = 0; }"); + auto AST = tooling::buildASTFromCodeWithArgs( + "#include \"skipped.h\"\n", + {"-std=gnu++11", "-target", "i386-unknown-unknown", "-I/"}, "input.cc", + "clang-tool", std::make_shared<PCHContainerOperations>(), + tooling::getClangStripDependencyFileAdjuster(), M); + ASSERT_TRUE(AST); + + auto countSkippedSubtreeMatches = + [&](MatchFinder::MatchFinderOptions Options) { + struct CountCallback : MatchFinder::MatchCallback { + void run(const MatchFinder::MatchResult &Result) override { ++Count; } + + unsigned Count = 0; + } Callback; + + MatchFinder Finder(std::move(Options)); + Finder.addMatcher(functionDecl(hasName("hidden")), &Callback); + Finder.addMatcher(integerLiteral(equals(0)), &Callback); + Finder.matchAST(AST->getASTContext()); + return Callback.Count; + }; + + EXPECT_EQ(2u, countSkippedSubtreeMatches({})); + + MatchFinder::MatchFinderOptions ScopedOptions; + SourceManager &SM = AST->getSourceManager(); + ScopedOptions.ShouldSkipLocation = [&SM](SourceLocation Location) { + return isExpansionInFileEndingWith(SM, Location, "/skipped.h"); + }; + EXPECT_EQ(0u, countSkippedSubtreeMatches(std::move(ScopedOptions))); +} + TEST(Matcher, IsExpansionInMainFileMatcher) { EXPECT_TRUE(matches("class X {};", recordDecl(hasName("X"), isExpansionInMainFile()))); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
