sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: mgrang.
Herald added a project: All.
sammccall requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
This adds command-line flags to the tool:
+ -print: prints changed source code
+ -print=changes: prints headers added/removed
+ -edit: rewrites code in place
+ -insert=0/-remove=0: disables additions/deletions for the above
These are supported by a couple of new functions dumped into Analysis:
analyze() sits on top of walkUsed and makes used/unused decisions for
Includes. fixIncludes() applies those results to source code.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D139013
Files:
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/test/Inputs/foobar.h
clang-tools-extra/include-cleaner/test/tool.cpp
clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -15,9 +15,11 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Frontend/FrontendActions.h"
#include "clang/Testing/TestAST.h"
+#include "clang/Tooling/Inclusions/IncludeStyle.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Testing/Support/Annotations.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -25,6 +27,7 @@
namespace clang::include_cleaner {
namespace {
+using testing::ElementsAre;
using testing::Pair;
using testing::UnorderedElementsAre;
@@ -134,6 +137,90 @@
UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile))));
}
+TEST(Analyze, Basic) {
+ TestInputs Inputs;
+ Inputs.Code = R"cpp(
+#include "a.h"
+#include "b.h"
+
+int x = a + c;
+)cpp";
+ Inputs.ExtraFiles["a.h"] = R"cpp(
+ #pragma once
+ int a;
+ )cpp";
+ Inputs.ExtraFiles["b.h"] = R"cpp(
+ #pragma once
+ #include "c.h"
+ int b;
+ )cpp";
+ Inputs.ExtraFiles["c.h"] = R"cpp(
+ #pragma once
+ int c;
+ )cpp";
+
+ RecordedPP PP;
+ Inputs.MakeAction = [&PP] {
+ struct Hook : public SyntaxOnlyAction {
+ public:
+ Hook(RecordedPP &PP) : PP(PP) {}
+ bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+ CI.getPreprocessor().addPPCallbacks(PP.record(CI.getPreprocessor()));
+ return true;
+ }
+
+ RecordedPP &PP;
+ };
+ return std::make_unique<Hook>(PP);
+ };
+
+ TestAST AST(Inputs);
+ auto Decls = AST.context().getTranslationUnitDecl()->decls();
+ auto Results =
+ analyze(std::vector<Decl *>{Decls.begin(), Decls.end()},
+ PP.MacroReferences, PP.Includes, /*PragmaIncludes=*/nullptr,
+ AST.sourceManager(), AST.preprocessor().getHeaderSearchInfo());
+
+ const Include *B = PP.Includes.atLine(3);
+ ASSERT_EQ(B->Spelled, "b.h");
+ EXPECT_THAT(Results.Missing, ElementsAre("\"c.h\""));
+ EXPECT_THAT(Results.Unused, ElementsAre(B));
+}
+
+TEST(FixIncludes, Basic) {
+ auto Code = llvm::MemoryBuffer::getMemBufferCopy(R"cpp(
+#include "a.h"
+#include "b.h"
+#include <c.h>
+)cpp",
+ "test.cc");
+
+ RecordedPP::RecordedIncludes Includes;
+ Include I;
+ I.Spelled = "a.h";
+ I.Line = 2;
+ Includes.add(I);
+ I.Spelled = "b.h";
+ I.Line = 3;
+ Includes.add(I);
+ I.Spelled = "c.h";
+ I.Line = 4;
+ Includes.add(I);
+
+ AnalysisResults Results;
+ Results.Missing.push_back("\"aa.h\"");
+ Results.Missing.push_back("\"ab.h\"");
+ Results.Missing.push_back("<e.h>");
+ Results.Unused.push_back(Includes.atLine(3));
+ Results.Unused.push_back(Includes.atLine(4));
+
+ EXPECT_EQ(fixIncludes(Results, *Code, tooling::IncludeStyle()), R"cpp(
+#include "a.h"
+#include "aa.h"
+#include "ab.h"
+#include <e.h>
+)cpp");
+}
} // namespace
} // namespace clang::include_cleaner
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
@@ -7,11 +7,13 @@
//===----------------------------------------------------------------------===//
#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Record.h"
#include "clang/Frontend/CompilerInstance.h"
#include "clang/Frontend/FrontendAction.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Inclusions/IncludeStyle.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/CommandLine.h"
@@ -46,7 +48,38 @@
cl::cat(IncludeCleaner),
};
-class HTMLReportAction : public clang::ASTFrontendAction {
+enum class PrintStyle { Changes, Final };
+cl::opt<PrintStyle> Print{
+ "print",
+ cl::values(
+ clEnumValN(PrintStyle::Changes, "changes", "Print symbolic changes"),
+ clEnumValN(PrintStyle::Final, "", "Print final code")),
+ cl::ValueOptional,
+ cl::init(PrintStyle::Final),
+ cl::desc("Print the list of headers to insert and remove"),
+ cl::cat(IncludeCleaner),
+};
+
+cl::opt<bool> Edit{
+ "edit",
+ cl::desc("Apply edits to analyzed source files"),
+ cl::cat(IncludeCleaner),
+};
+
+cl::opt<bool> Insert{
+ "insert",
+ cl::desc("Allow header insertions"),
+ cl::init(true),
+};
+cl::opt<bool> Remove{
+ "remove",
+ cl::desc("Allow header removals"),
+ cl::init(true),
+};
+
+std::atomic<unsigned> Errors = ATOMIC_VAR_INIT(0);
+
+class Action : public clang::ASTFrontendAction {
RecordedAST AST;
RecordedPP PP;
PragmaIncludes PI;
@@ -64,12 +97,62 @@
}
void EndSourceFile() override {
+ if (!HTMLReportPath.empty())
+ writeHTML();
+
+ const auto &SM = getCompilerInstance().getSourceManager();
+ auto &HS = getCompilerInstance().getPreprocessor().getHeaderSearchInfo();
+ auto Results =
+ analyze(AST.Roots, PP.MacroReferences, PP.Includes, &PI, SM, HS);
+ if (!Insert)
+ Results.Missing.clear();
+ if (!Remove)
+ Results.Unused.clear();
+
+ auto Main = SM.getBufferOrFake(SM.getMainFileID());
+ tooling::IncludeStyle IncludeStyle; // FIXME: determine properly somehow
+ std::string Final = fixIncludes(Results, Main, IncludeStyle);
+
+ if (Print.getNumOccurrences()) {
+ switch (Print) {
+ case PrintStyle::Changes:
+ for (const Include *I : Results.Unused)
+ // FIXME: this omits quotes, as we don't know the style.
+ llvm::outs() << "- " << I->Spelled << "\n";
+ for (const auto &I : Results.Missing)
+ llvm::outs() << "+ " << I << "\n";
+ break;
+ case PrintStyle::Final:
+ llvm::outs() << Final;
+ break;
+ }
+ }
+
+ if (Edit) {
+ llvm::StringRef Path =
+ SM.getFileEntryForID(SM.getMainFileID())->tryGetRealPathName();
+ assert(!Path.empty() && "Main file path not known?");
+ if (auto Err = llvm::writeToOutput(
+ Path, [&](llvm::raw_ostream &OS) -> llvm::Error {
+ OS << Final;
+ return llvm::Error::success();
+ })) {
+ llvm::errs() << "Failed to apply edits to "
+ << Main.getBufferIdentifier() << ": "
+ << toString(std::move(Err)) << "\n";
+ ++Errors;
+ }
+ }
+ }
+
+ void writeHTML() {
std::error_code EC;
llvm::raw_fd_ostream OS(HTMLReportPath, EC);
if (EC) {
llvm::errs() << "Unable to write HTML report to " << HTMLReportPath
<< ": " << EC.message() << "\n";
- exit(1);
+ ++Errors;
+ return;
}
writeHTMLReport(
AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
@@ -93,20 +176,17 @@
return 1;
}
- std::unique_ptr<clang::tooling::FrontendActionFactory> Factory;
- if (HTMLReportPath.getNumOccurrences()) {
- if (OptionsParser->getSourcePathList().size() != 1) {
- llvm::errs() << "-" << HTMLReportPath.ArgStr
- << " requires a single input file";
+ if (OptionsParser->getSourcePathList().size() != 1) {
+ std::vector<cl::Option *> IncompatibleFlags = {&HTMLReportPath, &Print};
+ for (const auto *Flag : IncompatibleFlags) {
+ if (Flag->getNumOccurrences())
+ llvm::errs() << "-" << Flag->ArgStr << " requires a single input file";
return 1;
}
- Factory = clang::tooling::newFrontendActionFactory<HTMLReportAction>();
- } else {
- llvm::errs() << "Unimplemented\n";
- return 1;
}
-
+ auto Factory = clang::tooling::newFrontendActionFactory<Action>();
return clang::tooling::ClangTool(OptionsParser->getCompilations(),
OptionsParser->getSourcePathList())
- .run(Factory.get());
+ .run(Factory.get()) ||
+ Errors != 0;
}
Index: clang-tools-extra/include-cleaner/test/tool.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/include-cleaner/test/tool.cpp
@@ -0,0 +1,25 @@
+#include "foobar.h"
+
+int x = foo();
+
+// RUN: clang-include-cleaner -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=CHANGE %s
+// CHANGE: - foobar.h
+// CHANGE-NEXT: + "foo.h"
+
+// RUN: clang-include-cleaner -remove=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=INSERT %s
+// INSERT-NOT: - foobar.h
+// INSERT: + "foo.h"
+
+// RUN: clang-include-cleaner -insert=0 -print=changes %s -- -I%S/Inputs/ | FileCheck --check-prefix=REMOVE %s
+// REMOVE: - foobar.h
+// REMOVE-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"{{$}}
+
+// RUN: cp %s %t.cpp
+// RUN: clang-include-cleaner -edit %t.cpp -- -I%S/Inputs/
+// RUN: FileCheck --match-full-lines --check-prefix=EDIT %s < %t.cpp
+// EDIT: #include "foo.h"
+// EDIT-NOT: {{^}}#include "foobar.h"{{$}}
Index: clang-tools-extra/include-cleaner/test/Inputs/foobar.h
===================================================================
--- /dev/null
+++ clang-tools-extra/include-cleaner/test/Inputs/foobar.h
@@ -0,0 +1,3 @@
+#pragma once
+#include "bar.h"
+#include "foo.h"
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
@@ -8,9 +8,13 @@
#include "clang-include-cleaner/Analysis.h"
#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Record.h" // FIXME: move/rename RecordedIncludes
#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Inclusions/HeaderIncludes.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
@@ -42,4 +46,92 @@
}
}
+static std::string spellHeader(const Header &H, HeaderSearch &HS,
+ const FileEntry *Main) {
+ switch (H.kind()) {
+ case Header::Physical: {
+ bool IsSystem = false;
+ std::string Path = HS.suggestPathToFileForDiagnostics(
+ H.physical(), Main->tryGetRealPathName(), &IsSystem);
+ return IsSystem ? "<" + Path + ">" : "\"" + Path + "\"";
+ }
+ case Header::Standard:
+ return H.standard().name().str();
+ case Header::Verbatim:
+ return H.verbatim().str();
+ }
+ llvm_unreachable("Unknown Header kind");
+}
+
+AnalysisResults analyze(llvm::ArrayRef<Decl *> ASTRoots,
+ llvm::ArrayRef<SymbolReference> MacroRefs,
+ const RecordedPP::RecordedIncludes &Includes,
+ const PragmaIncludes *PI, const SourceManager &SM,
+ HeaderSearch &HS) {
+ const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
+ llvm::DenseSet<const Include *> Used;
+ llvm::StringSet<> Missing;
+ walkUsed(ASTRoots, MacroRefs, PI, SM,
+ [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
+ bool Satisfied = false;
+ for (const Header &H : Providers) {
+ if (H.kind() == Header::Physical && H.physical() == MainFile)
+ Satisfied = true;
+ for (const Include *I : Includes.match(H)) {
+ Used.insert(I);
+ Satisfied = true;
+ }
+ }
+ if (!Satisfied && !Providers.empty() &&
+ Ref.RT == RefType::Explicit)
+ Missing.insert(spellHeader(Providers.front(), HS, MainFile));
+ });
+
+ AnalysisResults Results;
+ for (const Include &I : Includes.all())
+ if (!Used.contains(&I))
+ Results.Unused.push_back(&I);
+ for (llvm::StringRef S : Missing.keys())
+ Results.Missing.push_back(S.str());
+ llvm::sort(Results.Missing);
+ return Results;
+}
+
+std::string fixIncludes(const AnalysisResults &Results,
+ const llvm::MemoryBufferRef MainFile,
+ const tooling::IncludeStyle &IncludeStyle) {
+ tooling::HeaderIncludes HI(MainFile.getBufferIdentifier(),
+ MainFile.getBuffer(), IncludeStyle);
+ // tooling::Replacements has an irritating "order-independent" invariant
+ // that forces us to pre-merge any insertions at the same point.
+ tooling::Replacements Result;
+ llvm::Optional<tooling::Replacement> PendingInsert;
+ for (llvm::StringRef Insert : Results.Missing) {
+ if (auto Edit = HI.insert(Insert.trim("<>\""), Insert.starts_with("<"))) {
+ if (PendingInsert && Edit->getOffset() == PendingInsert->getOffset()) {
+ PendingInsert = tooling::Replacement(
+ PendingInsert->getFilePath(), PendingInsert->getOffset(),
+ PendingInsert->getLength(),
+ (PendingInsert->getReplacementText() + Edit->getReplacementText())
+ .str());
+ } else {
+ if (PendingInsert)
+ cantFail(Result.add(std::move(*PendingInsert)));
+ PendingInsert = std::move(Edit);
+ }
+ }
+ }
+ if (PendingInsert)
+ cantFail(Result.add(std::move(*PendingInsert)));
+ for (const Include *Unused : Results.Unused) {
+ for (bool Angled : {false, true}) // FIXME: only remove matching quotes
+ for (const tooling::Replacement &R : HI.remove(Unused->Spelled, Angled))
+ // HeaderIncludes::remove is by name rather than line number, so we
+ // could remove the same header multiple times. Ignore that.
+ consumeError(Result.add(R));
+ }
+
+ return cantFail(tooling::applyAllReplacements(MainFile.getBuffer(), Result));
+}
+
} // namespace clang::include_cleaner
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
@@ -15,12 +15,18 @@
#include "clang-include-cleaner/Types.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/Support/MemoryBufferRef.h"
#include <variant>
namespace clang {
class SourceLocation;
class Decl;
class FileEntry;
+class HeaderSearch;
+namespace tooling {
+class Replacements;
+struct IncludeStyle;
+} // namespace tooling
namespace include_cleaner {
/// A UsedSymbolCB is a callback invoked for each symbol reference seen.
@@ -47,6 +53,25 @@
llvm::ArrayRef<SymbolReference> MacroRefs,
const PragmaIncludes *PI, const SourceManager &, UsedSymbolCB CB);
+struct AnalysisResults {
+ std::vector<const Include *> Unused;
+ std::vector<std::string> Missing; // Spellings, like "<vector>"
+};
+
+/// 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 RecordedPP::RecordedIncludes &Includes,
+ const PragmaIncludes *PI, const SourceManager &SM,
+ HeaderSearch &HS);
+
+/// Removes unused includes and inserts missing ones in the main file.
+/// Returns the modified main-file code.
+std::string fixIncludes(const AnalysisResults &Results,
+ const llvm::MemoryBufferRef MainFile,
+ const tooling::IncludeStyle &IncludeStyle);
+
} // namespace include_cleaner
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
