replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[analyzer] [NFC] PathDiagnostic: Create PathDiagnosticPopUpPiece 

Summary:
This new piece is similar to our macro expansion printing in HTML reports:
On mouse-hover event it pops up on variables. Similar to note pieces it
supports `plist` diagnostics as well.

It is optional, on by default: `add-pop-up-notes=true`.

Extra: In HTML reports `background-color: LemonChiffon` was too light,
changed to `PaleGoldenRod`.

Reviewers: NoQ, alexfh

Reviewed By: NoQ

Subscribers: cfe-commits, gerazo, gsd, george.karpenkov, alexfh, xazax.hun,
             baloghadamsoftware, szepet, a.sidorin, mikhail.ramalho,
             Szelethus, donat.nagy, dkrupp

Tags: #clang

Differential Revision: https://reviews.llvm.org/D60670

llvm-svn: 362014
This commit is contained in:
Csaba Dabis 2019-05-29 19:21:59 +00:00
parent 45e8cc6639
commit 1d7ca67769
9 changed files with 243 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
View File

@ -204,6 +204,10 @@ ANALYZER_OPTION(bool, ShouldPrunePaths, "prune-paths",
"be pruned out of the final output.",
true)
ANALYZER_OPTION(bool, ShouldAddPopUpNotes, "add-pop-up-notes",
"Whether pop-up notes should be added to the final output.",
true)
ANALYZER_OPTION(
bool, ShouldConditionalizeStaticInitializers,
"cfg-conditional-static-initializers",

22
clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
View File

@ -367,7 +367,7 @@ public:
class PathDiagnosticPiece: public llvm::FoldingSetNode {
public:
enum Kind { ControlFlow, Event, Macro, Call, Note };
enum Kind { ControlFlow, Event, Macro, Call, Note, PopUp };
enum DisplayHint { Above, Below };
private:
@ -482,7 +482,7 @@ public:
static bool classof(const PathDiagnosticPiece *P) {
return P->getKind() == Event || P->getKind() == Macro ||
P->getKind() == Note;
P->getKind() == Note || P->getKind() == PopUp;
}
};
@ -746,7 +746,7 @@ public:
class PathDiagnosticNotePiece: public PathDiagnosticSpotPiece {
public:
PathDiagnosticNotePiece(const PathDiagnosticLocation &Pos, StringRef S,
bool AddPosRange = true)
bool AddPosRange = true)
: PathDiagnosticSpotPiece(Pos, S, Note, AddPosRange) {}
~PathDiagnosticNotePiece() override;
@ -759,6 +759,22 @@ public:
void Profile(llvm::FoldingSetNodeID &ID) const override;
};
class PathDiagnosticPopUpPiece: public PathDiagnosticSpotPiece {
public:
PathDiagnosticPopUpPiece(const PathDiagnosticLocation &Pos, StringRef S,
bool AddPosRange = true)
: PathDiagnosticSpotPiece(Pos, S, PopUp, AddPosRange) {}
~PathDiagnosticPopUpPiece() override;
static bool classof(const PathDiagnosticPiece *P) {
return P->getKind() == PopUp;
}
void dump() const override;
void Profile(llvm::FoldingSetNodeID &ID) const override;
};
/// File IDs mapped to sets of line numbers.
using FilesToLineNumsMap = std::map<FileID, std::set<unsigned>>;

49
clang/lib/Rewrite/HTMLRewrite.cpp
View File

@ -306,14 +306,16 @@ h1 { font-size:14pt }
.keyword { color: blue }
.string_literal { color: red }
.directive { color: darkmagenta }
/* Macro expansions. */
.expansion { display: none; }
.macro:hover .expansion {
/* Macros and variables could have pop-up notes hidden by default.
- Macro pop-up: expansion of the macro
- Variable pop-up: value (table) of the variable */
.macro_popup, .variable_popup { display: none; }
/* Pop-up appears on mouse-hover event. */
.macro:hover .macro_popup, .variable:hover .variable_popup {
display: block;
border: 2px solid #FF0000;
padding: 2px;
background-color:#FFF0F0;
font-weight: normal;
-webkit-border-radius:5px;
-webkit-box-shadow:1px 1px 7px #000;
border-radius:5px;
@ -324,6 +326,27 @@ h1 { font-size:14pt }
z-index: 1
}
.macro_popup {
border: 2px solid red;
background-color:#FFF0F0;
font-weight: normal;
}
.variable_popup {
border: 2px solid blue;
background-color:#F0F0FF;
font-weight: bold;
font-family: Helvetica, sans-serif;
font-size: 9pt;
}
/* Pop-up notes needs a relative position as a base where they pops up. */
.macro, .variable {
background-color: PaleGoldenRod;
position: relative;
}
.macro { color: DarkMagenta; }
#tooltiphint {
position: fixed;
width: 50em;
@ -336,12 +359,6 @@ h1 { font-size:14pt }
background-color: #c0c0c0;
z-index: 2;
}
.macro {
color: darkmagenta;
background-color:LemonChiffon;
/* Macros are position: relative to provide base for expansions. */
position: relative;
}
.num { width:2.5em; padding-right:2ex; background-color:#eeeeee }
.num { text-align:right; font-size:8pt }
@ -369,6 +386,7 @@ h1 { font-size:14pt }
.PathIndex { border-radius:8px }
.PathIndexEvent { background-color:#bfba87 }
.PathIndexControl { background-color:#8c8c8c }
.PathIndexPopUp { background-color: #879abc; }
.PathNav a { text-decoration:none; font-size: larger }
.CodeInsertionHint { font-weight: bold; background-color: #10dd10 }
.CodeRemovalHint { background-color:#de1010 }
@ -636,10 +654,9 @@ void html::HighlightMacros(Rewriter &R, FileID FID, const Preprocessor& PP) {
TmpPP.Lex(Tok);
}
// Insert the expansion as the end tag, so that multi-line macros all get
// highlighted.
Expansion = "<span class='expansion'>" + Expansion + "</span></span>";
// Insert the 'macro_popup' as the end tag, so that multi-line macros all
// get highlighted.
Expansion = "<span class='macro_popup'>" + Expansion + "</span></span>";
HighlightRange(R, LLoc.getBegin(), LLoc.getEnd(), "<span class='macro'>",
Expansion.c_str(), LLoc.isTokenRange());

21
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
View File

@ -154,8 +154,6 @@ static void removeRedundantMsgs(PathPieces &path) {
case PathDiagnosticPiece::Macro:
removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(*piece).subPieces);
break;
case PathDiagnosticPiece::ControlFlow:
break;
case PathDiagnosticPiece::Event: {
if (i == N-1)
break;
@ -175,7 +173,9 @@ static void removeRedundantMsgs(PathPieces &path) {
}
break;
}
case PathDiagnosticPiece::ControlFlow:
case PathDiagnosticPiece::Note:
case PathDiagnosticPiece::PopUp:
break;
}
path.push_back(std::move(piece));
@ -230,9 +230,8 @@ static bool removeUnneededCalls(PathPieces &pieces, BugReport *R,
break;
}
case PathDiagnosticPiece::ControlFlow:
break;
case PathDiagnosticPiece::Note:
case PathDiagnosticPiece::PopUp:
break;
}
@ -242,6 +241,16 @@ static bool removeUnneededCalls(PathPieces &pieces, BugReport *R,
return containsSomethingInteresting;
}
/// Same logic as above to remove extra pieces.
static void removePopUpNotes(PathPieces &Path) {
for (unsigned int i = 0; i < Path.size(); ++i) {
auto Piece = std::move(Path.front());
Path.pop_front();
if (!isa<PathDiagnosticPopUpPiece>(*Piece))
Path.push_back(std::move(Piece));
}
}
/// Returns true if the given decl has been implicitly given a body, either by
/// the analyzer or by the compiler proper.
static bool hasImplicitBody(const Decl *D) {
@ -1981,6 +1990,10 @@ static std::unique_ptr<PathDiagnostic> generatePathDiagnosticForConsumer(
(void)stillHasNotes;
}
// Remove pop-up notes if needed.
if (!Opts.ShouldAddPopUpNotes)
removePopUpNotes(PD->getMutablePieces());
// Redirect all call pieces to have valid locations.
adjustCallLocations(PD->getMutablePieces());
removePiecesWithInvalidLocations(PD->getMutablePieces());

124
clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
View File

@ -90,8 +90,9 @@ public:
const PathDiagnosticMacroPiece& P,
unsigned num);
void HandlePiece(Rewriter& R, FileID BugFileID,
const PathDiagnosticPiece& P, unsigned num, unsigned max);
void HandlePiece(Rewriter &R, FileID BugFileID, const PathDiagnosticPiece &P,
const std::vector<SourceRange> &PopUpRanges, unsigned num,
unsigned max);
void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range,
const char *HighlightStart = "<span class=\"mrange\">",
@ -605,6 +606,53 @@ window.addEventListener("keydown", function (event) {
)<<<";
}
static void
HandlePopUpPieceStartTag(Rewriter &R,
const std::vector<SourceRange> &PopUpRanges) {
for (const auto &Range : PopUpRanges) {
html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "",
"<table class='variable_popup'><tbody>",
/*IsTokenRange=*/true);
}
}
static void HandlePopUpPieceEndTag(Rewriter &R,
const PathDiagnosticPopUpPiece &Piece,
std::vector<SourceRange> &PopUpRanges,
unsigned int LastReportedPieceIndex,
unsigned int PopUpPieceIndex) {
SmallString<256> Buf;
llvm::raw_svector_ostream Out(Buf);
SourceRange Range(Piece.getLocation().asRange());
// Write out the path indices with a right arrow and the message as a row.
Out << "<tr><td valign='top'><div class='PathIndex PathIndexPopUp'>"
<< LastReportedPieceIndex;
// Also annotate the state transition with extra indices.
Out << '.' << PopUpPieceIndex;
Out << "</div></td><td>" << Piece.getString() << "</td></tr>";
// If no report made at this range mark the variable and add the end tags.
if (std::find(PopUpRanges.begin(), PopUpRanges.end(), Range) ==
PopUpRanges.end()) {
// Store that we create a report at this range.
PopUpRanges.push_back(Range);
Out << "</tbody></table></span>";
html::HighlightRange(R, Range.getBegin(), Range.getEnd(),
"<span class='variable'>", Buf.c_str(),
/*IsTokenRange=*/true);
// Otherwise inject just the new row at the end of the range.
} else {
html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", Buf.c_str(),
/*IsTokenRange=*/true);
}
}
void HTMLDiagnostics::RewriteFile(Rewriter &R,
const PathPieces& path, FileID FID) {
// Process the path.
@ -615,39 +663,80 @@ void HTMLDiagnostics::RewriteFile(Rewriter &R,
[](const std::shared_ptr<PathDiagnosticPiece> &p) {
return isa<PathDiagnosticNotePiece>(*p);
});
unsigned PopUpPieceCount =
std::count_if(path.begin(), path.end(),
[](const std::shared_ptr<PathDiagnosticPiece> &p) {
return isa<PathDiagnosticPopUpPiece>(*p);
});
unsigned TotalRegularPieces = TotalPieces - TotalNotePieces;
unsigned TotalRegularPieces = TotalPieces - TotalNotePieces - PopUpPieceCount;
unsigned NumRegularPieces = TotalRegularPieces;
unsigned NumNotePieces = TotalNotePieces;
// Stores the count of the regular piece indices.
std::map<int, int> IndexMap;
// Stores the different ranges where we have reported something.
std::vector<SourceRange> PopUpRanges;
for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
if (isa<PathDiagnosticNotePiece>(I->get())) {
const auto &Piece = *I->get();
if (isa<PathDiagnosticPopUpPiece>(Piece)) {
++IndexMap[NumRegularPieces];
} else if (isa<PathDiagnosticNotePiece>(Piece)) {
// This adds diagnostic bubbles, but not navigation.
// Navigation through note pieces would be added later,
// as a separate pass through the piece list.
HandlePiece(R, FID, **I, NumNotePieces, TotalNotePieces);
HandlePiece(R, FID, Piece, PopUpRanges, NumNotePieces, TotalNotePieces);
--NumNotePieces;
} else {
HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces);
HandlePiece(R, FID, Piece, PopUpRanges, NumRegularPieces,
TotalRegularPieces);
--NumRegularPieces;
}
}
// Add line numbers, header, footer, etc.
// Secondary indexing if we are having multiple pop-ups between two notes.
// (e.g. [(13) 'a' is 'true']; [(13.1) 'b' is 'false']; [(13.2) 'c' is...)
NumRegularPieces = TotalRegularPieces;
for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
const auto &Piece = *I->get();
if (const auto *PopUpP = dyn_cast<PathDiagnosticPopUpPiece>(&Piece)) {
int PopUpPieceIndex = IndexMap[NumRegularPieces];
// Pop-up pieces needs the index of the last reported piece and its count
// how many times we report to handle multiple reports on the same range.
// This marks the variable, adds the </table> end tag and the message
// (list element) as a row. The <table> start tag will be added after the
// rows has been written out. Note: It stores every different range.
HandlePopUpPieceEndTag(R, *PopUpP, PopUpRanges, NumRegularPieces,
PopUpPieceIndex);
if (PopUpPieceIndex > 0)
--IndexMap[NumRegularPieces];
} else if (!isa<PathDiagnosticNotePiece>(Piece)) {
--NumRegularPieces;
}
}
// Add the <table> start tag of pop-up pieces based on the stored ranges.
HandlePopUpPieceStartTag(R, PopUpRanges);
// Add line numbers, header, footer, etc.
html::EscapeText(R, FID);
html::AddLineNumbers(R, FID);
// If we have a preprocessor, relex the file and syntax highlight.
// We might not have a preprocessor if we come from a deserialized AST file,
// for example.
html::SyntaxHighlight(R, FID, PP);
html::HighlightMacros(R, FID, PP);
}
void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
const PathDiagnosticPiece& P,
void HTMLDiagnostics::HandlePiece(Rewriter &R, FileID BugFileID,
const PathDiagnosticPiece &P,
const std::vector<SourceRange> &PopUpRanges,
unsigned num, unsigned max) {
// For now, just draw a box above the line in question, and emit the
// warning.
@ -689,9 +778,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
bool IsNote = false;
bool SuppressIndex = (max == 1);
switch (P.getKind()) {
case PathDiagnosticPiece::Call:
llvm_unreachable("Calls and extra notes should already be handled");
case PathDiagnosticPiece::Event: Kind = "Event"; break;
case PathDiagnosticPiece::Event: Kind = "Event"; break;
case PathDiagnosticPiece::ControlFlow: Kind = "Control"; break;
// Setting Kind to "Control" is intentional.
case PathDiagnosticPiece::Macro: Kind = "Control"; break;
@ -700,6 +787,9 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
IsNote = true;
SuppressIndex = true;
break;
case PathDiagnosticPiece::Call:
case PathDiagnosticPiece::PopUp:
llvm_unreachable("Calls and extra notes should already be handled");
}
std::string sbuf;
@ -859,8 +949,14 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
// Now highlight the ranges.
ArrayRef<SourceRange> Ranges = P.getRanges();
for (const auto &Range : Ranges)
for (const auto &Range : Ranges) {
// If we have already highlighted the range as a pop-up there is no work.
if (std::find(PopUpRanges.begin(), PopUpRanges.end(), Range) !=
PopUpRanges.end())
continue;
HighlightRange(R, LPosInfo.first, Range);
}
}
static void EmitAlphaCounter(raw_ostream &os, unsigned n) {

21
clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
View File

@ -90,6 +90,8 @@ PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() = default;
PathDiagnosticNotePiece::~PathDiagnosticNotePiece() = default;
PathDiagnosticPopUpPiece::~PathDiagnosticPopUpPiece() = default;
void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
bool ShouldFlattenMacros) const {
for (auto &Piece : *this) {
@ -119,6 +121,7 @@ void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
case PathDiagnosticPiece::Event:
case PathDiagnosticPiece::ControlFlow:
case PathDiagnosticPiece::Note:
case PathDiagnosticPiece::PopUp:
Current.push_back(Piece);
break;
}
@ -369,15 +372,16 @@ static Optional<bool> comparePiece(const PathDiagnosticPiece &X,
case PathDiagnosticPiece::ControlFlow:
return compareControlFlow(cast<PathDiagnosticControlFlowPiece>(X),
cast<PathDiagnosticControlFlowPiece>(Y));
case PathDiagnosticPiece::Event:
case PathDiagnosticPiece::Note:
return None;
case PathDiagnosticPiece::Macro:
return compareMacro(cast<PathDiagnosticMacroPiece>(X),
cast<PathDiagnosticMacroPiece>(Y));
case PathDiagnosticPiece::Call:
return compareCall(cast<PathDiagnosticCallPiece>(X),
cast<PathDiagnosticCallPiece>(Y));
case PathDiagnosticPiece::Event:
case PathDiagnosticPiece::Note:
case PathDiagnosticPiece::PopUp:
return None;
}
llvm_unreachable("all cases handled");
}
@ -1287,6 +1291,10 @@ void PathDiagnosticNotePiece::Profile(llvm::FoldingSetNodeID &ID) const {
PathDiagnosticSpotPiece::Profile(ID);
}
void PathDiagnosticPopUpPiece::Profile(llvm::FoldingSetNodeID &ID) const {
PathDiagnosticSpotPiece::Profile(ID);
}
void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
ID.Add(getLocation());
ID.AddString(BugType);
@ -1412,6 +1420,13 @@ LLVM_DUMP_METHOD void PathDiagnosticNotePiece::dump() const {
getLocation().dump();
}
LLVM_DUMP_METHOD void PathDiagnosticPopUpPiece::dump() const {
llvm::errs() << "POP-UP\n--------------\n";
llvm::errs() << getString() << "\n";
llvm::errs() << " ---- at ----\n";
getLocation().dump();
}
LLVM_DUMP_METHOD void PathDiagnosticLocation::dump() const {
if (!isValid()) {
llvm::errs() << "<INVALID>\n";

34
clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
View File

@ -120,6 +120,9 @@ private:
case PathDiagnosticPiece::Note:
ReportNote(o, cast<PathDiagnosticNotePiece>(P), indent);
break;
case PathDiagnosticPiece::PopUp:
ReportPopUp(o, cast<PathDiagnosticPopUpPiece>(P), indent);
break;
}
}
@ -138,6 +141,9 @@ private:
unsigned indent, unsigned depth);
void ReportNote(raw_ostream &o, const PathDiagnosticNotePiece& P,
unsigned indent);
void ReportPopUp(raw_ostream &o, const PathDiagnosticPopUpPiece &P,
unsigned indent);
};
} // end of anonymous namespace
@ -397,6 +403,34 @@ void PlistPrinter::ReportNote(raw_ostream &o, const PathDiagnosticNotePiece& P,
Indent(o, indent); o << "</dict>\n";
}
void PlistPrinter::ReportPopUp(raw_ostream &o,
const PathDiagnosticPopUpPiece &P,
unsigned indent) {
const SourceManager &SM = PP.getSourceManager();
Indent(o, indent) << "<dict>\n";
++indent;
Indent(o, indent) << "<key>kind</key><string>pop-up</string>\n";
// Output the location.
FullSourceLoc L = P.getLocation().asLocation();
Indent(o, indent) << "<key>location</key>\n";
EmitLocation(o, SM, L, FM, indent);
// Output the ranges (if any).
ArrayRef<SourceRange> Ranges = P.getRanges();
EmitRanges(o, Ranges, indent);
// Output the text.
EmitMessage(o, P.getString(), indent);
// Finish up.
--indent;
Indent(o, indent) << "</dict>\n";
}
//===----------------------------------------------------------------------===//
// Static function definitions.
//===----------------------------------------------------------------------===//

11
clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
View File

@ -191,15 +191,16 @@ static json::Object createLocation(json::Object &&PhysicalLocation,
static Importance calculateImportance(const PathDiagnosticPiece &Piece) {
switch (Piece.getKind()) {
case PathDiagnosticPiece::Kind::Call:
case PathDiagnosticPiece::Kind::Macro:
case PathDiagnosticPiece::Kind::Note:
case PathDiagnosticPiece::Call:
case PathDiagnosticPiece::Macro:
case PathDiagnosticPiece::Note:
case PathDiagnosticPiece::PopUp:
// FIXME: What should be reported here?
break;
case PathDiagnosticPiece::Kind::Event:
case PathDiagnosticPiece::Event:
return Piece.getTagStr() == "ConditionBRVisitor" ? Importance::Important
: Importance::Essential;
case PathDiagnosticPiece::Kind::ControlFlow:
case PathDiagnosticPiece::ControlFlow:
return Importance::Unimportant;
}
return Importance::Unimportant;

3
clang/test/Analysis/analyzer-config.c
View File

@ -2,6 +2,7 @@
// RUN: FileCheck --input-file=%t %s --match-full-lines
// CHECK: [config]
// CHECK-NEXT: add-pop-up-notes = true
// CHECK-NEXT: aggressive-binary-operation-simplification = false
// CHECK-NEXT: alpha.clone.CloneChecker:IgnoredFilesPattern = ""
// CHECK-NEXT: alpha.clone.CloneChecker:MinimumCloneComplexity = 50
@ -87,4 +88,4 @@
// CHECK-NEXT: unroll-loops = false
// CHECK-NEXT: widen-loops = false
// CHECK-NEXT: [stats]
// CHECK-NEXT: num-entries = 84
// CHECK-NEXT: num-entries = 85