hokein updated this revision to Diff 533896.
hokein marked 2 inline comments as done.
hokein added a comment.
Herald added a subscriber: arphaman.
address comments:
- move the filtering in analysis API
- add an actual -edit lit test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153340/new/
https://reviews.llvm.org/D153340
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/Types.cpp
clang-tools-extra/include-cleaner/test/tool.cpp
clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
Index: clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -14,8 +14,11 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CommonOptionsParser.h"
#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Signals.h"
#include "llvm/Support/raw_ostream.h"
@@ -47,6 +50,14 @@
cl::cat(IncludeCleaner),
};
+cl::opt<std::string> 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<PrintStyle> Print{
"print",
@@ -91,9 +102,15 @@
}
class Action : public clang::ASTFrontendAction {
+public:
+ Action(llvm::function_ref<bool(llvm::StringRef Path)> HeaderFilter)
+ : HeaderFilter(HeaderFilter){};
+
+private:
RecordedAST AST;
RecordedPP PP;
PragmaIncludes PI;
+ llvm::function_ref<bool(llvm::StringRef Path)> HeaderFilter;
void ExecuteAction() override {
auto &P = getCompilerInstance().getPreprocessor();
@@ -126,8 +143,8 @@
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)
@@ -137,10 +154,12 @@
if (Print.getNumOccurrences()) {
switch (Print) {
case PrintStyle::Changes:
- for (const Include *I : Results.Unused)
+ for (const Include *I : Results.Unused) {
llvm::outs() << "- " << I->quote() << " @Line:" << I->Line << "\n";
- for (const auto &I : Results.Missing)
+ }
+ for (const auto &I : Results.Missing) {
llvm::outs() << "+ " << I << "\n";
+ }
break;
case PrintStyle::Final:
llvm::outs() << Final;
@@ -176,6 +195,18 @@
getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, OS);
}
};
+class ActionFactory : public tooling::FrontendActionFactory {
+public:
+ ActionFactory(llvm::function_ref<bool(llvm::StringRef Path)> HeaderFilter)
+ : HeaderFilter(HeaderFilter) {}
+
+ std::unique_ptr<clang::FrontendAction> create() override {
+ return std::make_unique<Action>(HeaderFilter);
+ }
+
+private:
+ llvm::function_ref<bool(llvm::StringRef Path)> HeaderFilter;
+};
} // namespace
} // namespace include_cleaner
@@ -191,6 +222,26 @@
llvm::errs() << toString(OptionsParser.takeError());
return 1;
}
+ std::vector<llvm::Regex> FilterRegs;
+ llvm::SmallVector<llvm::StringRef> 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 1;
+ }
+ FilterRegs.push_back(std::move(CompiledRegex));
+ }
+ auto HeaderFilter = [&FilterRegs](llvm::StringRef Path) {
+ for (const auto &F : FilterRegs)
+ if (F.match(Path))
+ return true;
+ return false;
+ };
if (OptionsParser->getSourcePathList().size() != 1) {
std::vector<cl::Option *> IncompatibleFlags = {&HTMLReportPath, &Print};
@@ -201,9 +252,9 @@
}
}
}
- auto Factory = clang::tooling::newFrontendActionFactory<Action>();
+ ActionFactory Factory(HeaderFilter);
return clang::tooling::ClangTool(OptionsParser->getCompilations(),
OptionsParser->getSourcePathList())
- .run(Factory.get()) ||
+ .run(&Factory) ||
Errors != 0;
}
Index: clang-tools-extra/include-cleaner/test/tool.cpp
===================================================================
--- clang-tools-extra/include-cleaner/test/tool.cpp
+++ clang-tools-extra/include-cleaner/test/tool.cpp
@@ -14,6 +14,10 @@
// 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 %s -- -I%S/Inputs/ | FileCheck --match-full-lines --check-prefix=PRINT %s
// PRINT: #include "foo.h"
// PRINT-NOT: {{^}}#include "foobar.h"{{$}}
@@ -23,3 +27,8 @@
// 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"{{$}}
Index: clang-tools-extra/include-cleaner/lib/Types.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Types.cpp
+++ clang-tools-extra/include-cleaner/lib/Types.cpp
@@ -38,6 +38,18 @@
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:
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -57,10 +57,12 @@
}
}
-AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
- llvm::ArrayRef<SymbolReference> MacroRefs,
- const Includes &Inc, const PragmaIncludes *PI,
- const SourceManager &SM, const HeaderSearch &HS) {
+AnalysisResults
+analyze(llvm::ArrayRef<Decl *> ASTRoots,
+ llvm::ArrayRef<SymbolReference> MacroRefs, const Includes &Inc,
+ const PragmaIncludes *PI, const SourceManager &SM,
+ const HeaderSearch &HS,
+ llvm::function_ref<bool(llvm::StringRef Path)> HeaderFilter) {
const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
llvm::DenseSet<const Include *> Used;
llvm::StringSet<> Missing;
@@ -76,13 +78,15 @@
}
}
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))
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
+++ 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 <memory>
#include <utility>
#include <variant>
@@ -133,6 +134,8 @@
}
StringRef verbatim() const { return std::get<Verbatim>(Storage); }
+ llvm::StringRef resolvedPath() const;
+
private:
// Order must match Kind enum!
std::variant<const FileEntry *, tooling::stdlib::Header, StringRef> Storage;
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -65,10 +65,14 @@
/// 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<Decl *> ASTRoots,
- llvm::ArrayRef<SymbolReference> MacroRefs,
- const Includes &I, const PragmaIncludes *PI,
- const SourceManager &SM, const HeaderSearch &HS);
+AnalysisResults analyze(
+ llvm::ArrayRef<Decl *> ASTRoots, llvm::ArrayRef<SymbolReference> MacroRefs,
+ const Includes &I, const PragmaIncludes *PI, const SourceManager &SM,
+ const HeaderSearch &HS,
+ llvm::function_ref<bool(llvm::StringRef)> HeaderFilter =
+ [](llvm::StringRef HeaderPath) {
+ return false; // don't filter any headers by default
+ });
/// Removes unused includes and inserts missing ones in the main file.
/// Returns the modified main-file code.
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -125,18 +125,6 @@
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<Diag> generateMissingIncludeDiagnostics(
ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes,
llvm::StringRef Code, HeaderFilter IgnoreHeaders) {
@@ -156,7 +144,7 @@
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",
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
