[clang-tools-extra] 6a95e67 - [include-cleaner] HTMLReport shows headers that would be inserted
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 25 02:57:42 PST 2022
Author: Sam McCall
Date: 2022-11-25T11:57:35+01:00
New Revision: 6a95e67323dd64e6023c0b5ea89407cac2dbc01b
URL: https://github.com/llvm/llvm-project/commit/6a95e67323dd64e6023c0b5ea89407cac2dbc01b
DIFF: https://github.com/llvm/llvm-project/commit/6a95e67323dd64e6023c0b5ea89407cac2dbc01b.diff
LOG: [include-cleaner] HTMLReport shows headers that would be inserted
Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/9730f933a2cf2e003365520b6636f731/raw/7911d8251ceab7c244e0510285105027cd0a9403/PathMapping.cpp.html
Header insertion doesn't actually work that well (not this patch's fault):
- we don't have ranking of locations/headers yet, so inserted header is pretty
random
- on my system, we get a lot of absolute "/usr/bin/../include/..." paths.
This is a HeaderSearch bug introduced in D60873 that I'll send a fix for
Differential Revision: https://reviews.llvm.org/D138676
Added:
Modified:
clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
index 1b85c679f6a7d..7bca824137e42 100644
--- a/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
+++ b/clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
@@ -29,6 +29,7 @@
namespace clang {
class ASTContext;
class Decl;
+class HeaderSearch;
class NamedDecl;
namespace include_cleaner {
@@ -87,7 +88,8 @@ llvm::SmallVector<Header> findHeaders(const SymbolLocation &Loc,
void writeHTMLReport(FileID File, const RecordedPP::RecordedIncludes &Includes,
llvm::ArrayRef<Decl *> Roots,
llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx,
- PragmaIncludes *PI, llvm::raw_ostream &OS);
+ HeaderSearch &HS, PragmaIncludes *PI,
+ llvm::raw_ostream &OS);
} // namespace include_cleaner
} // namespace clang
diff --git a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
index e28a951592c43..b57bbbdc94910 100644
--- a/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ b/clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Lexer.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/Support/ScopedPrinter.h"
@@ -29,7 +30,7 @@ namespace {
constexpr llvm::StringLiteral CSS = R"css(
body { margin: 0; }
pre { line-height: 1.5em; counter-reset: line; margin: 0; }
- pre .line { counter-increment: line; }
+ pre .line:not(.added) { counter-increment: line; }
pre .line::before {
content: counter(line);
display: inline-block;
@@ -37,6 +38,7 @@ constexpr llvm::StringLiteral CSS = R"css(
text-align: right;
width: 3em; padding-right: 0.5em; margin-right: 0.5em;
}
+ pre .line.added::before { content: '+' }
.ref, .inc { text-decoration: underline; color: #008; }
.sel { position: relative; cursor: pointer; }
.ref.implicit { background-color: #ff8; }
@@ -52,6 +54,7 @@ constexpr llvm::StringLiteral CSS = R"css(
#hover .target.implicit, .provides .implicit { background-color: #bbb; }
#hover .target.ambiguous, .provides .ambiguous { background-color: #caf; }
.missing, .unused { background-color: #faa !important; }
+ .inserted { background-color: #bea !important; }
.semiused { background-color: #888 !important; }
#hover th { color: #008; text-align: right; padding-right: 0.5em; }
#hover .target:not(:first-child) {
@@ -59,6 +62,8 @@ constexpr llvm::StringLiteral CSS = R"css(
padding-top: 1em;
border-top: 1px solid #444;
}
+ .ref.missing #hover .insert { font-weight: bold; }
+ .ref:not(.missing) #hover .insert { font-style: italic; }
)css";
constexpr llvm::StringLiteral JS = R"js(
@@ -128,6 +133,7 @@ class Reporter {
llvm::raw_ostream &OS;
const ASTContext &Ctx;
const SourceManager &SM;
+ HeaderSearch &HS;
const RecordedPP::RecordedIncludes &Includes;
const PragmaIncludes *PI;
FileID MainFile;
@@ -142,6 +148,7 @@ class Reporter {
SmallVector<Header> Headers;
SmallVector<const Include *> Includes;
bool Satisfied = false; // Is the include present?
+ std::string Insert; // If we had no includes, what would we insert?
};
std::vector<Target> Targets;
// Points within the main file that reference a Target.
@@ -157,6 +164,7 @@ class Reporter {
};
std::vector<Ref> Refs;
llvm::DenseMap<const Include *, std::vector<unsigned>> IncludeRefs;
+ llvm::StringMap<std::vector</*RefIndex*/unsigned>> Insertion;
llvm::StringRef includeType(const Include *I) {
auto &List = IncludeRefs[I];
@@ -169,8 +177,24 @@ class Reporter {
return "semiused";
}
+ std::string spellHeader(const Header &H) {
+ switch (H.kind()) {
+ case Header::Physical: {
+ bool IsSystem = false;
+ std::string Path = HS.suggestPathToFileForDiagnostics(
+ H.physical(), MainFE->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");
+ }
+
Target makeTarget(const SymbolReference &SR) {
- Target T{SR.Target, SR.RT, {}, {}, {}};
+ Target T{SR.Target, SR.RT, {}, {}, {}, {}, {}};
// Duplicates logic from walkUsed(), which doesn't expose SymbolLocations.
// FIXME: use locateDecl and friends once implemented.
@@ -200,16 +224,21 @@ class Reporter {
llvm::sort(T.Includes);
T.Includes.erase(std::unique(T.Includes.begin(), T.Includes.end()),
T.Includes.end());
+
+ if (!T.Headers.empty())
+ // FIXME: library should tell us which header to use.
+ T.Insert = spellHeader(T.Headers.front());
return T;
}
public:
- Reporter(llvm::raw_ostream &OS, ASTContext &Ctx,
+ Reporter(llvm::raw_ostream &OS, ASTContext &Ctx, HeaderSearch &HS,
const RecordedPP::RecordedIncludes &Includes,
const PragmaIncludes *PI, FileID MainFile)
- : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), Includes(Includes),
- PI(PI), MainFile(MainFile), MainFE(SM.getFileEntryForID(MainFile)) {}
+ : OS(OS), Ctx(Ctx), SM(Ctx.getSourceManager()), HS(HS),
+ Includes(Includes), PI(PI), MainFile(MainFile),
+ MainFE(SM.getFileEntryForID(MainFile)) {}
void addRef(const SymbolReference &SR) {
auto [File, Offset] = SM.getDecomposedLoc(SM.getFileLoc(SR.RefLocation));
@@ -225,6 +254,9 @@ class Reporter {
Targets.push_back(makeTarget(SR));
for (const auto *I : Targets.back().Includes)
IncludeRefs[I].push_back(Targets.size() - 1);
+ if (Targets.back().Type == RefType::Explicit && !Targets.back().Satisfied &&
+ !Targets.back().Insert.empty())
+ Insertion[Targets.back().Insert].push_back(Targets.size() - 1);
}
void write() {
@@ -233,6 +265,13 @@ class Reporter {
OS << "<head>\n";
OS << "<style>" << CSS << "</style>\n";
OS << "<script>" << JS << "</script>\n";
+ for (const auto &Ins : Insertion) {
+ OS << "<template id='i";
+ escapeString(Ins.first());
+ OS << "'>";
+ writeInsertion(Ins.first(), Ins.second);
+ OS << "</template>\n";
+ }
for (auto &Inc : Includes.all()) {
OS << "<template id='i" << Inc.Line << "'>";
writeInclude(Inc);
@@ -295,17 +334,13 @@ class Reporter {
printFilename(SM.getSpellingLoc(Loc).printToString(SM));
OS << ">";
}
-
- void writeInclude(const Include &Inc) {
- OS << "<table class='include'>";
- if (Inc.Resolved) {
- OS << "<tr><th>Resolved</td><td>";
- escapeString(Inc.Resolved->getName());
- OS << "</td></tr>\n";
- }
+
+ // Write "Provides: " rows of an include or include-insertion table.
+ // These describe the symbols the header provides, referenced by RefIndices.
+ void writeProvides(llvm::ArrayRef<unsigned> RefIndices) {
// We show one ref for each symbol: first by (RefType != Explicit, Sequence)
llvm::DenseMap<Symbol, /*RefIndex*/ unsigned> FirstRef;
- for (unsigned RefIndex : IncludeRefs[&Inc]) {
+ for (unsigned RefIndex : RefIndices) {
const Target &T = Targets[Refs[RefIndex].TargetIndex];
auto I = FirstRef.try_emplace(T.Sym, RefIndex);
if (!I.second && T.Type == RefType::Explicit &&
@@ -327,11 +362,27 @@ class Reporter {
escapeString(llvm::to_string(S));
if (!Details.empty())
OS << "</span>";
-
+
unsigned Line = SM.getLineNumber(MainFile, Refs[RefIndex].Offset);
OS << ", <a href='#line" << Line << "'>line " << Line << "</a>";
OS << "</td></tr>";
}
+ }
+
+ void writeInclude(const Include &Inc) {
+ OS << "<table class='include'>";
+ if (Inc.Resolved) {
+ OS << "<tr><th>Resolved</td><td>";
+ escapeString(Inc.Resolved->getName());
+ OS << "</td></tr>\n";
+ writeProvides(IncludeRefs[&Inc]);
+ }
+ OS << "</table>";
+ }
+
+ void writeInsertion(llvm::StringRef Text, llvm::ArrayRef<unsigned> Refs) {
+ OS << "<table class='insertion'>";
+ writeProvides(Refs);
OS << "</table>";
}
@@ -382,6 +433,12 @@ class Reporter {
OS << ", <a href='#line" << I->Line << "'>line " << I->Line << "</a>";
OS << "</td></tr>";
}
+
+ if (!T.Insert.empty()) {
+ OS << "<tr><th>Insert</th><td class='insert'>";
+ escapeString(T.Insert);
+ OS << "</td></tr>";
+ }
OS << "</table>";
}
@@ -392,6 +449,18 @@ class Reporter {
OS << "<pre onclick='select(event)' class='code'>";
+ std::vector<llvm::StringRef> Insertions{Insertion.keys().begin(),
+ Insertion.keys().end()};
+ llvm::sort(Insertions);
+ for (llvm::StringRef Insertion : Insertions) {
+ OS << "<code class='line added'>"
+ << "<span class='inc sel inserted' data-hover='i";
+ escapeString(Insertion);
+ OS << "'>#include ";
+ escapeString(Insertion);
+ OS << "</span></code>\n";
+ }
+
const Include *Inc = nullptr;
unsigned LineNum = 0;
// Lines are <code>, include lines have an inner <span>.
@@ -421,8 +490,8 @@ class Reporter {
while (!Rest.empty() && Rest.front().Offset == I &&
Rest.front().Implicit) {
const Ref &R = Rest.front();
- OS << "<span class='ref sel implicit"
- << (Targets[R.TargetIndex].Satisfied ? "" : " missing")
+ OS << "<span class='ref sel implicit "
+ << (Targets[R.TargetIndex].Satisfied ? "satisfied" : "missing")
<< "' data-hover='t" << R.TargetIndex << "'>◊</span>";
Rest = Rest.drop_front();
};
@@ -462,8 +531,9 @@ class Reporter {
void writeHTMLReport(FileID File, const RecordedPP::RecordedIncludes &Includes,
llvm::ArrayRef<Decl *> Roots,
llvm::ArrayRef<SymbolReference> MacroRefs, ASTContext &Ctx,
- PragmaIncludes *PI, llvm::raw_ostream &OS) {
- Reporter R(OS, Ctx, Includes, PI, File);
+ HeaderSearch &HS, PragmaIncludes *PI,
+ llvm::raw_ostream &OS) {
+ Reporter R(OS, Ctx, HS, Includes, PI, File);
for (Decl *Root : Roots)
walkAST(*Root, [&](SourceLocation Loc, const NamedDecl &D, RefType T) {
R.addRef(SymbolReference{Loc, D, T});
diff --git a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
index 1231576bbfd32..80450c86e3f30 100644
--- a/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
+++ b/clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
@@ -71,8 +71,10 @@ class HTMLReportAction : public clang::ASTFrontendAction {
<< ": " << EC.message() << "\n";
exit(1);
}
- writeHTMLReport(AST.Ctx->getSourceManager().getMainFileID(), PP.Includes,
- AST.Roots, PP.MacroReferences, *AST.Ctx, &PI, OS);
+ writeHTMLReport(
+ AST.Ctx->getSourceManager().getMainFileID(), PP.Includes, AST.Roots,
+ PP.MacroReferences, *AST.Ctx,
+ getCompilerInstance().getPreprocessor().getHeaderSearchInfo(), &PI, OS);
}
};
More information about the cfe-commits
mailing list