From 507d766d76d873aa6e446889f93384745a0e1c0b Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Thu, 6 Jul 2023 10:56:08 +0200 Subject: [PATCH] [include-cleaner] Add an IgnoreHeaders flag to the command-line tool. Reviewed By: kadircet Differential Revision: https://reviews.llvm.org/D153340 --- clang-tools-extra/clangd/IncludeCleaner.cpp | 14 +---- .../include/clang-include-cleaner/Analysis.h | 14 +++-- .../include/clang-include-cleaner/Types.h | 5 ++ clang-tools-extra/include-cleaner/lib/Analysis.cpp | 18 ++++-- clang-tools-extra/include-cleaner/lib/Types.cpp | 12 ++++ clang-tools-extra/include-cleaner/test/tool.cpp | 13 ++++ .../include-cleaner/tool/IncludeCleaner.cpp | 69 ++++++++++++++++++++-- 7 files changed, 118 insertions(+), 27 deletions(-) diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index dd8b5af..2a8499d 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -125,18 +125,6 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, return true; } -llvm::StringRef getResolvedPath(const include_cleaner::Header &SymProvider) { - switch (SymProvider.kind()) { - case include_cleaner::Header::Physical: - return SymProvider.physical()->tryGetRealPathName(); - case include_cleaner::Header::Standard: - return SymProvider.standard().name().trim("<>\""); - case include_cleaner::Header::Verbatim: - return SymProvider.verbatim().trim("<>\""); - } - llvm_unreachable("Unknown header kind"); -} - std::vector generateMissingIncludeDiagnostics( ParsedAST &AST, llvm::ArrayRef MissingIncludes, llvm::StringRef Code, HeaderFilter IgnoreHeaders) { @@ -156,7 +144,7 @@ std::vector generateMissingIncludeDiagnostics( FileStyle->IncludeStyle); for (const auto &SymbolWithMissingInclude : MissingIncludes) { llvm::StringRef ResolvedPath = - getResolvedPath(SymbolWithMissingInclude.Providers.front()); + SymbolWithMissingInclude.Providers.front().resolvedPath(); if (isIgnored(ResolvedPath, IgnoreHeaders)) { dlog("IncludeCleaner: not diagnosing missing include {0}, filtered by " "config", diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h index 13a1cfa..547e9dd 100644 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h @@ -65,10 +65,16 @@ struct AnalysisResults { /// Determine which headers should be inserted or removed from the main file. /// This exposes conclusions but not reasons: use lower-level walkUsed for that. -AnalysisResults analyze(llvm::ArrayRef ASTRoots, - llvm::ArrayRef MacroRefs, - const Includes &I, const PragmaIncludes *PI, - const SourceManager &SM, const HeaderSearch &HS); +/// +/// The HeaderFilter is a predicate that receives absolute path or spelling +/// without quotes/brackets, when a phyiscal file doesn't exist. +/// No analysis will be performed for headers that satisfy the predicate. +AnalysisResults +analyze(llvm::ArrayRef ASTRoots, + llvm::ArrayRef MacroRefs, const Includes &I, + const PragmaIncludes *PI, const SourceManager &SM, + const HeaderSearch &HS, + llvm::function_ref HeaderFilter = nullptr); /// Removes unused includes and inserts missing ones in the main file. /// Returns the modified main-file code. diff --git a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h index 48b018b..b16d494 100644 --- a/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h +++ b/clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h @@ -29,6 +29,7 @@ #include "llvm/ADT/DenseMapInfoVariant.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include #include #include @@ -133,6 +134,10 @@ struct Header { } StringRef verbatim() const { return std::get(Storage); } + /// Absolute path for the header when it's a physical file. Otherwise just + /// the spelling without surrounding quotes/brackets. + llvm::StringRef resolvedPath() const; + private: // Order must match Kind enum! std::variant Storage; diff --git a/clang-tools-extra/include-cleaner/lib/Analysis.cpp b/clang-tools-extra/include-cleaner/lib/Analysis.cpp index 3952d10..616eeae 100644 --- a/clang-tools-extra/include-cleaner/lib/Analysis.cpp +++ b/clang-tools-extra/include-cleaner/lib/Analysis.cpp @@ -57,13 +57,17 @@ void walkUsed(llvm::ArrayRef ASTRoots, } } -AnalysisResults analyze(llvm::ArrayRef ASTRoots, - llvm::ArrayRef MacroRefs, - const Includes &Inc, const PragmaIncludes *PI, - const SourceManager &SM, const HeaderSearch &HS) { +AnalysisResults +analyze(llvm::ArrayRef ASTRoots, + llvm::ArrayRef MacroRefs, const Includes &Inc, + const PragmaIncludes *PI, const SourceManager &SM, + const HeaderSearch &HS, + llvm::function_ref HeaderFilter) { const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); llvm::DenseSet Used; llvm::StringSet<> Missing; + if (!HeaderFilter) + HeaderFilter = [](llvm::StringRef) { return false; }; walkUsed(ASTRoots, MacroRefs, PI, SM, [&](const SymbolReference &Ref, llvm::ArrayRef
Providers) { bool Satisfied = false; @@ -76,13 +80,15 @@ AnalysisResults analyze(llvm::ArrayRef ASTRoots, } } if (!Satisfied && !Providers.empty() && - Ref.RT == RefType::Explicit) + Ref.RT == RefType::Explicit && + !HeaderFilter(Providers.front().resolvedPath())) Missing.insert(spellHeader({Providers.front(), HS, MainFile})); }); AnalysisResults Results; for (const Include &I : Inc.all()) { - if (Used.contains(&I) || !I.Resolved) + if (Used.contains(&I) || !I.Resolved || + HeaderFilter(I.Resolved->tryGetRealPathName())) continue; if (PI) { if (PI->shouldKeep(I.Line)) diff --git a/clang-tools-extra/include-cleaner/lib/Types.cpp b/clang-tools-extra/include-cleaner/lib/Types.cpp index a5ba4a0..271780b 100644 --- a/clang-tools-extra/include-cleaner/lib/Types.cpp +++ b/clang-tools-extra/include-cleaner/lib/Types.cpp @@ -38,6 +38,18 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S) { llvm_unreachable("Unhandled Symbol kind"); } +llvm::StringRef Header::resolvedPath() const { + switch (kind()) { + case include_cleaner::Header::Physical: + return physical()->tryGetRealPathName(); + case include_cleaner::Header::Standard: + return standard().name().trim("<>\""); + case include_cleaner::Header::Verbatim: + return verbatim().trim("<>\""); + } + llvm_unreachable("Unknown header kind"); +} + llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Header &H) { switch (H.kind()) { case Header::Physical: diff --git a/clang-tools-extra/include-cleaner/test/tool.cpp b/clang-tools-extra/include-cleaner/test/tool.cpp index 937099a..bd90679 100644 --- a/clang-tools-extra/include-cleaner/test/tool.cpp +++ b/clang-tools-extra/include-cleaner/test/tool.cpp @@ -14,6 +14,14 @@ int x = foo(); // REMOVE: - "foobar.h" // REMOVE-NOT: + "foo.h" +// RUN: clang-include-cleaner -print=changes %s --ignore-headers="foobar\.h,foo\.h" -- -I%S/Inputs/ | FileCheck --match-full-lines --allow-empty --check-prefix=IGNORE %s +// IGNORE-NOT: - "foobar.h" +// IGNORE-NOT: + "foo.h" + +// RUN: clang-include-cleaner -print=changes %s --ignore-headers="foobar.*\.h" -- -I%S/Inputs/ | FileCheck --match-full-lines --allow-empty --check-prefix=IGNORE2 %s +// IGNORE2-NOT: - "foobar.h" +// IGNORE2: + "foo.h" + // RUN: clang-include-cleaner -print %s -- -I%S/Inputs/ | FileCheck --match-full-lines --check-prefix=PRINT %s // PRINT: #include "foo.h" // PRINT-NOT: {{^}}#include "foobar.h"{{$}} @@ -23,3 +31,8 @@ int x = foo(); // RUN: FileCheck --match-full-lines --check-prefix=EDIT %s < %t.cpp // EDIT: #include "foo.h" // EDIT-NOT: {{^}}#include "foobar.h"{{$}} + +// RUN: cp %s %t.cpp +// RUN: clang-include-cleaner -edit --ignore-headers="foobar\.h,foo\.h" %t.cpp -- -I%S/Inputs/ +// RUN: FileCheck --match-full-lines --check-prefix=EDIT2 %s < %t.cpp +// EDIT2-NOT: {{^}}#include "foo.h"{{$}} diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp index 62febb4..e7ae0c6 100644 --- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp +++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp @@ -14,10 +14,19 @@ #include "clang/Lex/Preprocessor.h" #include "clang/Tooling/CommonOptionsParser.h" #include "clang/Tooling/Tooling.h" +#include "llvm/ADT/STLFunctionalExtras.h" +#include "llvm/ADT/SmallVector.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/CommandLine.h" +#include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/Regex.h" #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" +#include +#include +#include +#include +#include namespace clang { namespace include_cleaner { @@ -47,6 +56,14 @@ cl::opt HTMLReportPath{ cl::cat(IncludeCleaner), }; +cl::opt IgnoreHeaders{ + "ignore-headers", + cl::desc("A comma-separated list of regexes to match against suffix of a " + "header, and disable analysis if matched."), + cl::init(""), + cl::cat(IncludeCleaner), +}; + enum class PrintStyle { Changes, Final }; cl::opt Print{ "print", @@ -91,9 +108,15 @@ format::FormatStyle getStyle(llvm::StringRef Filename) { } class Action : public clang::ASTFrontendAction { +public: + Action(llvm::function_ref HeaderFilter) + : HeaderFilter(HeaderFilter){}; + +private: RecordedAST AST; RecordedPP PP; PragmaIncludes PI; + llvm::function_ref HeaderFilter; bool BeginInvocation(CompilerInstance &CI) override { // We only perform include-cleaner analysis. So we disable diagnostics that @@ -135,8 +158,8 @@ class Action : public clang::ASTFrontendAction { assert(!Path.empty() && "Main file path not known?"); llvm::StringRef Code = SM.getBufferData(SM.getMainFileID()); - auto Results = - analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS); + auto Results = analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, + HS, HeaderFilter); if (!Insert) Results.Missing.clear(); if (!Remove) @@ -185,6 +208,43 @@ class Action : public clang::ASTFrontendAction { getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, OS); } }; +class ActionFactory : public tooling::FrontendActionFactory { +public: + ActionFactory(llvm::function_ref HeaderFilter) + : HeaderFilter(HeaderFilter) {} + + std::unique_ptr create() override { + return std::make_unique(HeaderFilter); + } + +private: + llvm::function_ref HeaderFilter; +}; + +std::function headerFilter() { + auto FilterRegs = std::make_shared>(); + + llvm::SmallVector Headers; + llvm::StringRef(IgnoreHeaders).split(Headers, ',', -1, /*KeepEmpty=*/false); + for (auto HeaderPattern : Headers) { + std::string AnchoredPattern = "(" + HeaderPattern.str() + ")$"; + llvm::Regex CompiledRegex(AnchoredPattern); + std::string RegexError; + if (!CompiledRegex.isValid(RegexError)) { + llvm::errs() << llvm::formatv("Invalid regular expression '{0}': {1}\n", + HeaderPattern, RegexError); + return nullptr; + } + FilterRegs->push_back(std::move(CompiledRegex)); + } + return [FilterRegs](llvm::StringRef Path) { + for (const auto &F : *FilterRegs) { + if (F.match(Path)) + return true; + } + return false; + }; +} } // namespace } // namespace include_cleaner @@ -210,9 +270,10 @@ int main(int argc, const char **argv) { } } } - auto Factory = clang::tooling::newFrontendActionFactory(); + auto HeaderFilter = headerFilter(); + ActionFactory Factory(HeaderFilter); return clang::tooling::ClangTool(OptionsParser->getCompilations(), OptionsParser->getSourcePathList()) - .run(Factory.get()) || + .run(&Factory) || Errors != 0; } -- 2.7.4