Index: clang/include/clang/Rewrite/Core/HTMLRewrite.h =================================================================== --- clang/include/clang/Rewrite/Core/HTMLRewrite.h +++ clang/include/clang/Rewrite/Core/HTMLRewrite.h @@ -15,6 +15,7 @@ #define LLVM_CLANG_REWRITE_CORE_HTMLREWRITE_H #include "clang/Basic/SourceLocation.h" +#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h" #include namespace clang { @@ -23,6 +24,8 @@ class RewriteBuffer; class Preprocessor; +namespace ento { + namespace html { /// HighlightRange - Highlight a range in the source code with the specified @@ -65,6 +68,11 @@ void AddHeaderFooterInternalBuiltinCSS(Rewriter &R, FileID FID, StringRef title); + /// AddPopUpNotes - Annotate the HTML with pop-up notes (hidden by default, + /// appears on mouse-hover event). It adds information to the given variables + /// similar to 'HighlightMacros()'. + void AddPopUpNotes(Rewriter &R, const PathPieces &Path); + /// SyntaxHighlight - Relex the specified FileID and annotate the HTML with /// information about keywords, comments, etc. void SyntaxHighlight(Rewriter &R, FileID FID, const Preprocessor &PP); @@ -76,6 +84,7 @@ void HighlightMacros(Rewriter &R, FileID FID, const Preprocessor &PP); } // end html namespace +} // end ento namespace } // end clang namespace #endif 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 extra 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/Frontend/Rewrite/HTMLPrint.cpp =================================================================== --- clang/lib/Frontend/Rewrite/HTMLPrint.cpp +++ clang/lib/Frontend/Rewrite/HTMLPrint.cpp @@ -21,7 +21,10 @@ #include "clang/Rewrite/Core/Rewriter.h" #include "clang/Rewrite/Frontend/ASTConsumers.h" #include "llvm/Support/raw_ostream.h" + using namespace clang; +using namespace ento; + //===----------------------------------------------------------------------===// // Functional HTML pretty-printing. Index: clang/lib/Rewrite/HTMLRewrite.cpp =================================================================== --- clang/lib/Rewrite/HTMLRewrite.cpp +++ clang/lib/Rewrite/HTMLRewrite.cpp @@ -21,7 +21,9 @@ #include "llvm/Support/MemoryBuffer.h" #include "llvm/Support/raw_ostream.h" #include + using namespace clang; +using namespace ento; /// HighlightRange - Highlight a range in the source code with the specified @@ -306,13 +308,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 (by 'HighlightMacros()') + - Variable pop-up: value of the variable (by 'AddPopUpNotes()') */ +.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; @@ -324,6 +329,23 @@ z-index: 1 } +.macro_popup { + border: 2px solid red; + background-color:#FFF0F0; +} + +.variable_popup { + border: 2px solid blue; + background-color:#F0F0FF; +} + +/* 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 +358,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 } @@ -419,6 +435,27 @@ R.InsertTextAfter(EndLoc, "\n"); } +/// AddPopUpNotes - Annotate the HTML with pop-up notes (hidden by default, +/// appears on mouse-hover event). It adds information to the given variables +/// similar to 'HighlightMacros()'. +void html::AddPopUpNotes(Rewriter &R, const PathPieces &Path) { + if (Path.empty()) + return; + + for (const auto &Piece : Path) { + if (!isa(*Piece)) + continue; + + std::string PopUpNote = "" + + Piece->getString().str() + ""; + + SourceRange Range(Piece->getLocation().asRange()); + HighlightRange(R, Range.getBegin(), Range.getEnd(), + "", PopUpNote.c_str(), + /*IsTokenRange=*/true); + } +} + /// SyntaxHighlight - Relex the specified FileID and annotate the HTML with /// information about keywords, macro expansions etc. This uses the macro /// table state from the end of the file, so it won't be perfectly perfect, @@ -636,10 +673,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 i = 0; i < Path.size(); ++i) { + auto Piece = std::move(Path.front()); + Path.pop_front(); + if (Piece->getKind() != PathDiagnosticPiece::PopUp) + 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 @@ -615,35 +615,44 @@ [](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; for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) { - if (isa(I->get())) { + auto &Piece = *I->get(); + 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, NumNotePieces, TotalNotePieces); --NumNotePieces; - } else { - HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces); + + // Pop-up pieces handled in 'AddPopUpNotes()'. + } else if (!isa(Piece)) { + HandlePiece(R, FID, Piece, NumRegularPieces, TotalRegularPieces); --NumRegularPieces; } } // 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); + + // Also add pop-up notes to variables. + html::AddPopUpNotes(R, path); } void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID, @@ -689,9 +698,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 +707,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; 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