Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def =================================================================== --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -204,6 +204,10 @@ "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", Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h @@ -365,7 +365,7 @@ 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: @@ -480,7 +480,7 @@ static bool classof(const PathDiagnosticPiece *P) { return P->getKind() == Event || P->getKind() == Macro || - P->getKind() == Note; + P->getKind() == Note || P->getKind() == PopUp; } }; @@ -744,7 +744,7 @@ class PathDiagnosticNotePiece: public PathDiagnosticSpotPiece { public: PathDiagnosticNotePiece(const PathDiagnosticLocation &Pos, StringRef S, - bool AddPosRange = true) + bool AddPosRange = true) : PathDiagnosticSpotPiece(Pos, S, Note, AddPosRange) {} ~PathDiagnosticNotePiece() override; @@ -757,6 +757,22 @@ 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>; Index: clang/lib/Rewrite/HTMLRewrite.cpp =================================================================== --- clang/lib/Rewrite/HTMLRewrite.cpp +++ clang/lib/Rewrite/HTMLRewrite.cpp @@ -306,14 +306,16 @@ .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 @@ 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 @@ 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 @@ .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 @@ TmpPP.Lex(Tok); } - - // Insert the expansion as the end tag, so that multi-line macros all get - // highlighted. - Expansion = "" + Expansion + ""; + // Insert the 'macro_popup' as the end tag, so that multi-line macros all + // get highlighted. + Expansion = "" + Expansion + ""; HighlightRange(R, LLoc.getBegin(), LLoc.getEnd(), "", Expansion.c_str(), LLoc.isTokenRange()); Index: clang/lib/StaticAnalyzer/Core/BugReporter.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporter.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporter.cpp @@ -154,8 +154,6 @@ case PathDiagnosticPiece::Macro: removeRedundantMsgs(cast(*piece).subPieces); break; - case PathDiagnosticPiece::ControlFlow: - break; case PathDiagnosticPiece::Event: { if (i == N-1) break; @@ -175,7 +173,9 @@ } break; } + case PathDiagnosticPiece::ControlFlow: case PathDiagnosticPiece::Note: + case PathDiagnosticPiece::PopUp: break; } path.push_back(std::move(piece)); @@ -230,9 +230,8 @@ break; } case PathDiagnosticPiece::ControlFlow: - break; - case PathDiagnosticPiece::Note: + case PathDiagnosticPiece::PopUp: break; } @@ -242,6 +241,16 @@ 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(*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 @@ (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()); Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp @@ -90,8 +90,9 @@ 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 &PopUpRanges, unsigned num, + unsigned max); void HighlightRange(Rewriter& R, FileID BugFileID, SourceRange Range, const char *HighlightStart = "", @@ -605,6 +606,53 @@ )<<<"; } +static void +HandlePopUpPieceStartTag(Rewriter &R, + const std::vector &PopUpRanges) { + for (const auto &Range : PopUpRanges) { + html::HighlightRange(R, Range.getBegin(), Range.getEnd(), "", + "", + /*IsTokenRange=*/true); + } +} + +static void HandlePopUpPieceEndTag(Rewriter &R, + const PathDiagnosticPopUpPiece &Piece, + std::vector &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 << ""; + + // 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 << "
" + << LastReportedPieceIndex; + + // Also annotate the state transition with extra indices. + Out << '.' << PopUpPieceIndex; + + Out << "
" << Piece.getString() << "
"; + html::HighlightRange(R, Range.getBegin(), Range.getEnd(), + "", 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 @@ [](const std::shared_ptr &p) { return isa(*p); }); + unsigned PopUpPieceCount = + std::count_if(path.begin(), path.end(), + [](const std::shared_ptr &p) { + return isa(*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 IndexMap; + // Stores the different ranges where we have reported something. + std::vector PopUpRanges; for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) { - if (isa(I->get())) { + const auto &Piece = *I->get(); + + if (isa(Piece)) { + ++IndexMap[NumRegularPieces]; + } else if (isa(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(&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 end tag and the message + // (list element) as a row. The 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(Piece)) { + --NumRegularPieces; + } + } + // Add the
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 &PopUpRanges, unsigned num, unsigned max) { // For now, just draw a box above the line in question, and emit the // warning. @@ -689,9 +778,7 @@ 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 @@ 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 @@ // Now highlight the ranges. ArrayRef 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) { Index: clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp +++ clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp @@ -90,6 +90,8 @@ PathDiagnosticNotePiece::~PathDiagnosticNotePiece() = default; +PathDiagnosticPopUpPiece::~PathDiagnosticPopUpPiece() = default; + void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current, bool ShouldFlattenMacros) const { for (auto &Piece : *this) { @@ -119,6 +121,7 @@ case PathDiagnosticPiece::Event: case PathDiagnosticPiece::ControlFlow: case PathDiagnosticPiece::Note: + case PathDiagnosticPiece::PopUp: Current.push_back(Piece); break; } @@ -369,15 +372,16 @@ case PathDiagnosticPiece::ControlFlow: return compareControlFlow(cast(X), cast(Y)); - case PathDiagnosticPiece::Event: - case PathDiagnosticPiece::Note: - return None; case PathDiagnosticPiece::Macro: return compareMacro(cast(X), cast(Y)); case PathDiagnosticPiece::Call: return compareCall(cast(X), cast(Y)); + case PathDiagnosticPiece::Event: + case PathDiagnosticPiece::Note: + case PathDiagnosticPiece::PopUp: + return None; } llvm_unreachable("all cases handled"); } @@ -1270,6 +1274,10 @@ 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); @@ -1395,6 +1403,13 @@ 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() << "\n"; Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp @@ -120,6 +120,9 @@ case PathDiagnosticPiece::Note: ReportNote(o, cast(P), indent); break; + case PathDiagnosticPiece::PopUp: + ReportPopUp(o, cast(P), indent); + break; } } @@ -138,6 +141,9 @@ 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 @@ Indent(o, indent); o << "\n"; } +void PlistPrinter::ReportPopUp(raw_ostream &o, + const PathDiagnosticPopUpPiece &P, + unsigned indent) { + const SourceManager &SM = PP.getSourceManager(); + + Indent(o, indent) << "\n"; + ++indent; + + Indent(o, indent) << "kindpop-up\n"; + + // Output the location. + FullSourceLoc L = P.getLocation().asLocation(); + + Indent(o, indent) << "location\n"; + EmitLocation(o, SM, L, FM, indent); + + // Output the ranges (if any). + ArrayRef Ranges = P.getRanges(); + EmitRanges(o, Ranges, indent); + + // Output the text. + EmitMessage(o, P.getString(), indent); + + // Finish up. + --indent; + Indent(o, indent) << "\n"; +} + //===----------------------------------------------------------------------===// // Static function definitions. //===----------------------------------------------------------------------===// Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp +++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp @@ -191,15 +191,16 @@ 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; Index: clang/test/Analysis/analyzer-config.c =================================================================== --- clang/test/Analysis/analyzer-config.c +++ clang/test/Analysis/analyzer-config.c @@ -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: avoid-suppressing-null-argument-paths = false // CHECK-NEXT: c++-allocator-inlining = true @@ -52,4 +53,4 @@ // CHECK-NEXT: unroll-loops = false // CHECK-NEXT: widen-loops = false // CHECK-NEXT: [stats] -// CHECK-NEXT: num-entries = 49 +// CHECK-NEXT: num-entries = 50