[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

9 files changed, 243 insertions, 46 deletions

diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index cc8b70bcb2c..40d5d47bbce 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -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",
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
index 547a8ca6434..5230742a4aa 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
@@ -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>>;
 
diff --git a/clang/lib/Rewrite/HTMLRewrite.cpp b/clang/lib/Rewrite/HTMLRewrite.cpp
index 688dd7f7875..e304fbbed72 100644
--- a/clang/lib/Rewrite/HTMLRewrite.cpp
+++ b/clang/lib/Rewrite/HTMLRewrite.cpp
@@ -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());
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 738ad9a062f..6627633f393 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -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());
diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 79aaae8cbb3..8ede3f15e60 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -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) {
diff --git a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
index 5889a979661..1f642064827 100644
--- a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -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";
diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index c03bab0fe16..231db1fa16f 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -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.
 //===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
index a8f529b7d35..4233f25edb3 100644
--- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -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;
diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c
index 38bf5cc60ce..a9c2eaa353b 100644
--- a/clang/test/Analysis/analyzer-config.c
+++ b/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: 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