[analyzer] [NFC] PathDiagnostic: Create PathDiagnosticPopUpPiece

[Charusso]

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.

[NoQ]

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

Both look good to me, though i'd easily believe that LemonChiffon is indeed indistinguishable from white on some monitors with certain brightness/contrast settings.

clang/lib/Rewrite/HTMLRewrite.cpp

333

Blue over yellow looks a bit acid, maybe use a... "bluer"?... background for our pop-ups? Like, both for the markup and for the actual popup.

431–433

Why is it necessary to re-lex everything in this case? I don't really know why this was necessary for macros, but in this case i'm more confused. The popup piece is supposed to point us to a specific source range; why can't we just highlight that source range directly?

[NoQ]

Let's decide whether we want to display the new notes in the text output. @alexfh: Given that clang-tidy is the primary consumer of the analyzer's text output mode, would you rather have or rather not have these new note:s in the text output? (scroll to the bottom of D53076 to see how they look). In my opinion, i'd rather have them in text output (because text output is hard to navigate, and having the necessary information exactly where you need it would probably be helpful), but rather not have them in tools that parse clang-tidy notes and stuff them into a source code view in some GUI (such tools do exist, right?), so im confused. I'd also prefer to have them be off in text output in tests, because such tests usually test checker-specific notes and having more checker-inspecific notes in the tests is more annoying than useful. But that's pretty minor; i'll be happy to write them down every time i write a test if they are believed to be a good thing in text output in general.

[alexfh]

In D60670#1469494, @NoQ wrote:

Let's decide whether we want to display the new notes in the text output. @alexfh: Given that clang-tidy is the primary consumer of the analyzer's text output mode, would you rather have or rather not have these new note:s in the text output? (scroll to the bottom of D53076 to see how they look). In my opinion, i'd rather have them in text output (because text output is hard to navigate, and having the necessary information exactly where you need it would probably be helpful), but rather not have them in tools that parse clang-tidy notes and stuff them into a source code view in some GUI (such tools do exist, right?), so im confused. I'd also prefer to have them be off in text output in tests, because such tests usually test checker-specific notes and having more checker-inspecific notes in the tests is more annoying than useful. But that's pretty minor; i'll be happy to write them down every time i write a test if they are believed to be a good thing in text output in general.

Can we add an option to enable or disable these notes similar to Clang's -fdiagnostics-show-option, -fdiagnostics-show-note-include-stack, etc?

[Szelethus]

• clang-format PlistDiagnostics.cpp

I wanted to do this for the longest time, but @george.karpenkov was very much against it. Is this desired? D52735#1251344

[Charusso]

Thanks!

@Szelethus, I will revert it.

In D60670#1470849, @alexfh wrote:

In D60670#1469494, @NoQ wrote:

Let's decide whether we want to display the new notes in the text output.

Can we add an option to enable or disable these notes similar to Clang's -fdiagnostics-show-option, -fdiagnostics-show-note-include-stack, etc?

@alexfh, add them as a Clang option with -f? I do not know how Clang Tidy would use that, so that I am not sure. I would create an AnalyzerOptions option as I see that far only.

clang/lib/Rewrite/HTMLRewrite.cpp

333

I am planning to change the entire CSS business as the current representation already too acid. Do you mean to change the PaleGoldenRod to blue on conditions as a new kind of highlight?

431–433

I just did not know about PathDiagnosticRange.

[alexfh]

In D60670#1474333, @Charusso wrote:

In D60670#1470849, @alexfh wrote:

Can we add an option to enable or disable these notes similar to Clang's -fdiagnostics-show-option, -fdiagnostics-show-note-include-stack, etc?

@alexfh, add them as a Clang option with -f? I do not know how Clang Tidy would use that, so that I am not sure. I would create an AnalyzerOptions option as I see that far only.

I gave Clang's -f options as an example only. An AnalyzerOptions option would work too. Just something we can change if needed.

[Charusso]

• add-pop-up-notes=true
• Revert clang-format.

[Charusso]

Feature request test is in D61060.

Now it is blue and the highlight is as wide as the condition:

blue.html1 MBDownload

Before:

before.html1 MBDownload

[NoQ]

Hmm, i just realized that there may be multiple pop-ups for the same expression, with different values. This may happen if the expression is used in a loop or the function is called multiple times. We'd have to include all of them somehow, maybe something like this:

   /------------------\
   | (42.a)  X is 5   |    /-----------------\
   | (69.a)  X is -2  |    | (42.b)  Y is 7  |
   \  /---------------/    \  /--------------/
    \/                      \/

if (X >= 0        &&        Y <= 10) {

  /-------------------------\
  | (43) Taking true branch |
  \-------------------------/
  /--------------------------\
  | (70) Taking false branch |
  \--------------------------/

}

[Charusso]

Fix multiple reports at the same range:

• Plain text became a table.
• Styling is the same as the bubbles.
• mrange (whatever it means) - the grey highlight cannot overlap pop-up ranges.

[Charusso]

Your indexing idea has a problem: zero index would be possible and usually you would compare the gray PathDiagnostic bubbles with these notes which index would get an offset by one.

At line 723 there would be a zero index, which is confusing, and at line 1052 you could see the new approach:

multiple.html236 KBDownload

[NoQ]

Wonderful!!

The reason why i don't want numbers on pop-up notes match numbers on regular notes is that the text on the pop-up note may no longer be true at the moment of time when we reach the regular note. Like, the value of the variable may change in between.

We can do one of the following:

• "42.a", "42.b", ... (still confusing as it if's part of 42);
• "42½", "42⅓", "42⅔", ..., just give up and drop them when there more than 10 popups between two events specific, because that's how many unicode vulgar fractions we have;
• Just "42 → 43" on all pop-ups (indicates that this is somewhere between 42 and 43).

If it turns out to be still confusing, i'd recommend committing it as off-by-default until we come up with a better UI. 'Cause the work is wonderful regardless.

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

208 ↗

(On Diff #197000)

Let's specify "pop-up notes", because "extra notes" refer to the note pieces.

[Charusso]

index → index+1

[Charusso]

Thanks for the review!

I have not seen problematic indexing, that is why I was not sure about your idea. If someone ever find confusing stuff I would change it immediately to off by default.

Final look:

final.html236 KBDownload

[Charusso]

PS: That offset-by-one index + arrow confuse me more, so that I am not sure about that improvement being a useful one.

[NoQ]

Could you have a quick look at how does the vulgar fractions solution look?, i.e., 42½ etc., maybe you can modify the html directly if you don't want to code it, as it might be annoying to code. I don't immediately see any downsides.

[Charusso]

• Sub-state indexing.

[Charusso]

I have not got better idea to just index over each sub-state. Example at line 1052:

indices.html236 KBDownload

[Charusso]

PS: I do not like that fraction stuff as it is difficult to read, ugly and limited.

[Charusso]

@NoQ could you share your final thoughts here, please? I would like to move forward and never look back.

[NoQ]

Mmm, just had a fresh look and i kinda like it. I think in its current shape it's intuitive enough what this means.

Let's make sure that all popups have the extra .digit in them and commit.

[Charusso]

• Add extra state transition indices to every pop-up note.

[Charusso]

Thanks you! I hope this feature will be useful to see what is behind the scenes and create better reports.

Usage could be found at line 2041 with line 2129:

done.html915 KBDownload

[Charusso]

PS: I have no idea why the macro note is purple, but because the entire HTML printing is broken I hope you saw nothing.

[NoQ]

Go for it!

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

644–645

This comment looks outdated.

[Charusso]

Fix a nit in rL362632.

clang/include/clang/Rewrite/Core/HTMLRewrite.h

 //  into beautified HTML.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_REWRITE_CORE_HTMLREWRITE_H
 #define LLVM_CLANG_REWRITE_CORE_HTMLREWRITE_H
 
 #include "clang/Basic/SourceLocation.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include <string>
 
 namespace clang {
 
 class Rewriter;
 class RewriteBuffer;
 class Preprocessor;
 
+namespace ento {
+
 namespace html {
 
   /// HighlightRange - Highlight a range in the source code with the specified
   /// start/end tags.  B/E must be in the same file.  This ensures that
   /// start/end tags are placed at the start/end of each line if the range is
   /// multiline.
   void HighlightRange(Rewriter &R, SourceLocation B, SourceLocation E,
                       const char *StartTag, const char *EndTag,
namespace html {
   std::string EscapeText(StringRef s,
                          bool EscapeSpaces = false, bool ReplaceTabs = false);
 
   void AddLineNumbers(Rewriter& R, FileID FID);
 
   void AddHeaderFooterInternalBuiltinCSS(Rewriter &R, FileID FID,
                                          StringRef title);
 
+  /// AddPopUpNotes - Relex the specified FileID and annotate the HTML with
+  /// pop-up notes (hidden by default, show on mouse-hover event).
+  /// It adds information to given variables similar to 'HighlightMacros()'.
+  void AddPopUpNotes(Rewriter &R, FileID FID, const Preprocessor &PP,
+                     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);
 
   /// HighlightMacros - This uses the macro table state from the end of the
   /// file, to reexpand macros and insert (into the HTML) information about the
   /// macro expansions.  This won't be perfectly perfect, but it will be
   /// reasonably close.
   void HighlightMacros(Rewriter &R, FileID FID, const Preprocessor &PP);
 
 } // end html namespace
+} // end ento namespace
 } // end clang namespace
 
 #endif

clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

 };
 
 //===----------------------------------------------------------------------===//
 // Path "pieces" for path-sensitive diagnostics.
 //===----------------------------------------------------------------------===//
 
 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:
   const std::string str;
   const Kind kind;
   const DisplayHint Hint;
 
   /// In the containing bug report, this piece is the last piece from
public:
 
   PathDiagnosticLocation getLocation() const override { return Pos; }
   void flattenLocations() override { Pos.flatten(); }
 
   void Profile(llvm::FoldingSetNodeID &ID) const override;
 
   static bool classof(const PathDiagnosticPiece *P) {
     return P->getKind() == Event || P->getKind() == Macro ||
-           P->getKind() == Note;
+           P->getKind() == Note || P->getKind() == PopUp;
   }
 };
 
 /// Interface for classes constructing Stack hints.
 ///
 /// If a PathDiagnosticEvent occurs in a different frame than the final
 /// diagnostic the hints can be used to summarize the effect of the call.
 class StackHintGenerator {
public:
   void dump() const override;
 
   void Profile(llvm::FoldingSetNodeID &ID) const override;
 };
 
 class PathDiagnosticNotePiece: public PathDiagnosticSpotPiece {
 public:
   PathDiagnosticNotePiece(const PathDiagnosticLocation &Pos, StringRef S,
-                               bool AddPosRange = true)
       : PathDiagnosticSpotPiece(Pos, S, Note, AddPosRange) {}
   ~PathDiagnosticNotePiece() override;
 
   static bool classof(const PathDiagnosticPiece *P) {
     return P->getKind() == Note;
   }
 
   void dump() const override;
 
   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>>;
 
 /// PathDiagnostic - PathDiagnostic objects represent a single path-sensitive
 ///  diagnostic.  It represents an ordered-collection of PathDiagnosticPieces,
 ///  each which represent the pieces of the path.
 class PathDiagnostic : public llvm::FoldingSetNode {
   std::string CheckName;

clang/lib/Frontend/Rewrite/HTMLPrint.cpp

 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #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.
 //===----------------------------------------------------------------------===//
 
 namespace {
   class HTMLPrinter : public ASTConsumer {
     Rewriter R;

clang/lib/Rewrite/HTMLRewrite.cpp

 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
+
 using namespace clang;
+using namespace ento;
 
 
 /// HighlightRange - Highlight a range in the source code with the specified
 /// start/end tags.  B/E must be in the same file.  This ensures that
 /// start/end tags are placed at the start/end of each line if the range is
 /// multiline.
 void html::HighlightRange(Rewriter &R, SourceLocation B, SourceLocation E,
                           const char *StartTag, const char *EndTag,

 .divider { background-color: gray; }
 .code { border-collapse:collapse; width:100%; }
 .code { font-family: "Monospace", monospace; font-size:10pt }
 .code { line-height: 1.2em }
 .comment { color: green; font-style: oblique }
 .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;
   border-radius:5px;
   box-shadow:1px 1px 7px #000;
   position: absolute;
   top: -1em;
   left:10em;
   z-index: 1
 }
+.macro_popup    { border: 2px solid red;  }
+.variable_popup { border: 2px solid blue; }
+
+/* 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;
   margin-left: -25em;
   left: 50%;
   padding: 10px;
   border: 1px solid #b0b0b0;
   border-radius: 2px;
   box-shadow: 1px 1px 7px black;
   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 }
 .num { color:#444444 }
 .line { padding-left: 1ex; border-left: 3px solid #ccc }
 .line { white-space: pre }
 .msg { -webkit-box-shadow:1px 1px 7px #000 }
 .msg { box-shadow:1px 1px 7px #000 }
<body>)<<<";
 
   // Generate header
   R.InsertTextBefore(StartLoc, os.str());
   // Generate footer
 
   R.InsertTextAfter(EndLoc, "</body></html>\n");
 }
 
+/// AddPopUpNotes - Relex the specified FileID and annotate the HTML with
+/// pop-up notes (hidden by default, show on mouse-hover event).
+/// It adds information to given variables similar to 'HighlightMacros()'.
+void html::AddPopUpNotes(Rewriter &R, FileID FID, const Preprocessor &PP,
+                         const PathPieces &Path) {
+  if (Path.empty())
+    return;
+
+  RewriteBuffer &RB = R.getEditBuffer(FID);
+  const SourceManager &SM = PP.getSourceManager();
+  const llvm::MemoryBuffer *FromFile = SM.getBuffer(FID);
+  Lexer L(FID, FromFile, SM, PP.getLangOpts());
+  const char *BufferStart = L.getBuffer().data();
+
+  // Lex all the tokens in raw mode, to avoid entering #includes or expanding
+  // macros.
+  Token Tok;
+  L.LexFromRawLexer(Tok);
+
+  while (Tok.isNot(tok::eof)) {
+    // Since we are lexing unexpanded tokens, all tokens are from the main
+    // FileID.
+    unsigned TokOffs = SM.getFileOffset(Tok.getLocation());
+    unsigned TokLen = Tok.getLength();
+    switch (Tok.getKind()) {
+    default: break;
+    case tok::identifier:
+      llvm_unreachable("tok::identifier in raw lexing mode!");
+    case tok::raw_identifier: {
+      // Fill in Result.IdentifierInfo and update the token kind,
+      // looking up the identifier in the identifier table.
+      PP.LookUpIdentifierInfo(Tok);
+
+      // If it is not an identifier do not try to add a pop-up note to it.
+      if (Tok.isNot(tok::identifier))
+        break;
+
+      for (const auto &Piece : Path) {
+        if (!isa<PathDiagnosticPopUpPiece>(*Piece) ||
+            Piece->getLocation().asLocation() != Tok.getLocation())
+          continue;
+
+        std::string PopUpNote = "<span class='variable_popup'>" +
+                                Piece->getString().str() + "</span></span>";
+
+        HighlightRange(RB, TokOffs, TokOffs + TokLen, BufferStart,
+                       "<span class='variable'>", PopUpNote.c_str());
+      }
+      break;
+    }
+    }
+
+    L.LexFromRawLexer(Tok);
+  }
+}
+
 /// 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,
 /// but it will be reasonably close.
 void html::SyntaxHighlight(Rewriter &R, FileID FID, const Preprocessor &PP) {
   RewriteBuffer &RB = R.getEditBuffer(FID);
 
   const SourceManager &SM = PP.getSourceManager();
while (!Tok.is(tok::eof) &&
       Expansion += EscapeText(TmpPP.getSpelling(Tok));
       LineLen += Expansion.size();
 
       PrevPrevTok = PrevTok;
       PrevTok = Tok;
       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());
   }
 
   // Restore the preprocessor's old state.
   TmpPP.setDiagnostics(*OldDiags);
   TmpPP.setPragmasEnabled(PragmasPreviouslyEnabled);
 }

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

for (unsigned i = 0; i < N; ++i) {
 
     switch (piece->getKind()) {
       case PathDiagnosticPiece::Call:
         removeRedundantMsgs(cast<PathDiagnosticCallPiece>(*piece).path);
         break;
       case PathDiagnosticPiece::Macro:
         removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(*piece).subPieces);
         break;
-      case PathDiagnosticPiece::ControlFlow:
-        break;
       case PathDiagnosticPiece::Event: {
         if (i == N-1)
           break;
 
         if (auto *nextEvent =
             dyn_cast<PathDiagnosticEventPiece>(path.front().get())) {
           auto *event = cast<PathDiagnosticEventPiece>(piece.get());
           // Check to see if we should keep one of the two pieces.  If we
           // come up with a preference, record which piece to keep, and consume
           // another piece from the path.
           if (auto *pieceToKeep =
                   eventsDescribeSameCondition(event, nextEvent)) {
             piece = std::move(pieceToKeep == event ? piece : path.front());
             path.pop_front();
             ++i;
           }
         }
         break;
       }
+      case PathDiagnosticPiece::ControlFlow:
       case PathDiagnosticPiece::Note:
+      case PathDiagnosticPiece::PopUp:
         break;
     }
     path.push_back(std::move(piece));
   }
 }
 
 /// A map from PathDiagnosticPiece to the LocationContext of the inlined
 /// function call it represents.
switch (piece->getKind()) {
         auto &event = cast<PathDiagnosticEventPiece>(*piece);
 
         // We never throw away an event, but we do throw it away wholesale
         // as part of a path if we throw the entire path away.
         containsSomethingInteresting |= !event.isPrunable();
         break;
       }
       case PathDiagnosticPiece::ControlFlow:
-        break;
-
       case PathDiagnosticPiece::Note:
+      case PathDiagnosticPiece::PopUp:
         break;
     }
 
     pieces.push_back(std::move(piece));
   }
 
   return containsSomethingInteresting;
 }

clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp

void HTMLDiagnostics::RewriteFile(Rewriter &R,
   // Process the path.
   // Maintain the counts of extra note pieces separately.
   unsigned TotalPieces = path.size();
   unsigned TotalNotePieces =
       std::count_if(path.begin(), path.end(),
                     [](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;
 
   for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
-    if (isa<PathDiagnosticNotePiece>(I->get())) {
+    auto &Piece = *I->get();
+    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, NumNotePieces, TotalNotePieces);
       --NumNotePieces;
-    } else {
-      HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces);
+
+      // Pop-up pieces handled in 'AddPopUpNotes()'.
+    } else if (!isa<PathDiagnosticPopUpPiece>(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.
-
+  // Also add pop-up notes on variables with 'AddPopUpNotes()'.
+  // (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);
+  html::AddPopUpNotes(R, FID, PP, path);
 }
 
 void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
                                   const PathDiagnosticPiece& P,
                                   unsigned num, unsigned max) {
   // For now, just draw a box above the line in question, and emit the
   // warning.
   FullSourceLoc Pos = P.getLocation().asLocation();
for (const char* c = LineStart; c != TokInstantiationPtr; ++c)
     PosNo += *c == '\t' ? 8 : 1;
 
   // Create the html for the message.
 
   const char *Kind = nullptr;
   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;
   case PathDiagnosticPiece::Note:
     Kind = "Note";
     IsNote = true;
     SuppressIndex = true;
     break;
+  case PathDiagnosticPiece::Call:
+  case PathDiagnosticPiece::PopUp:
+    llvm_unreachable("Calls and extra notes should already be handled");
   }
 
   std::string sbuf;
   llvm::raw_string_ostream os(sbuf);
 
   os << "\n<tr><td class=\"num\"></td><td class=\"line\"><div id=\"";
 
   if (IsNote)

clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp

 PathDiagnosticCallPiece::~PathDiagnosticCallPiece() = default;
 
 PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() = default;
 
 PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() = default;
 
 PathDiagnosticNotePiece::~PathDiagnosticNotePiece() = default;
 
+PathDiagnosticPopUpPiece::~PathDiagnosticPopUpPiece() = default;
+
 void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
                            bool ShouldFlattenMacros) const {
   for (auto &Piece : *this) {
     switch (Piece->getKind()) {
     case PathDiagnosticPiece::Call: {
       auto &Call = cast<PathDiagnosticCallPiece>(*Piece);
       if (auto CallEnter = Call.getCallEnterEvent())
         Current.push_back(std::move(CallEnter));
case PathDiagnosticPiece::Macro: {
         // FIXME: This probably shouldn't mutate the original path piece.
         Macro.subPieces = NewPath;
       }
       break;
     }
     case PathDiagnosticPiece::Event:
     case PathDiagnosticPiece::ControlFlow:
     case PathDiagnosticPiece::Note:
+    case PathDiagnosticPiece::PopUp:
       Current.push_back(Piece);
       break;
     }
   }
 }
 
 PathDiagnostic::~PathDiagnostic() = default;
 
if (XR != YR) {
       return SM.isBeforeInTranslationUnit(XR.getEnd(), YR.getEnd());
     }
   }
 
   switch (X.getKind()) {
     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");
 }
 
 static Optional<bool> comparePath(const PathPieces &X, const PathPieces &Y) {
   if (X.size() != Y.size())
     return X.size() < Y.size();
 
void PathDiagnosticMacroPiece::Profile(llvm::FoldingSetNodeID &ID) const {
   for (const auto &I : subPieces)
     ID.Add(*I);
 }
 
 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);
   ID.AddString(VerboseDesc);
   ID.AddString(Category);
 }
 
 void PathDiagnostic::FullProfile(llvm::FoldingSetNodeID &ID) const {

 
 LLVM_DUMP_METHOD void PathDiagnosticNotePiece::dump() const {
   llvm::errs() << "NOTE\n--------------\n";
   llvm::errs() << getString() << "\n";
   llvm::errs() << " ---- at ----\n";
   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";
     return;
   }
 
   switch (K) {
   case RangeK:

clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp

 #include "clang/Basic/Version.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/IssueHash.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
-#include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/Statistic.h"
 #include "llvm/Support/Casting.h"
 
 using namespace clang;
 using namespace ento;
 using namespace markup;
 
 //===----------------------------------------------------------------------===//
 // Declarations of helper classes and functions for emitting bug reports in
 // plist format.
 //===----------------------------------------------------------------------===//
 
 namespace {
-  class PlistDiagnostics : public PathDiagnosticConsumer {
-    const std::string OutputFile;
-    const Preprocessor &PP;
-    AnalyzerOptions &AnOpts;
-    const bool SupportsCrossFileDiagnostics;
-  public:
-    PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
-                     const std::string& prefix,
-                     const Preprocessor &PP,
-                     bool supportsMultipleFiles);
+
+  PlistDiagnostics(AnalyzerOptions &AnalyzerOpts, const std::string &prefix,
+                   const Preprocessor &PP, bool supportsMultipleFiles);
 
-    ~PlistDiagnostics() override {}
 
-    void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
-                              FilesMade *filesMade) override;
 
-    StringRef getName() const override {
-      return "PlistDiagnostics";
-    }
+  StringRef getName() const override { return "PlistDiagnostics"; }
 
-    PathGenerationScheme getGenerationScheme() const override {
-      return Extensive;
-    }
-    bool supportsLogicalOpControlFlow() const override { return true; }
-    bool supportsCrossFileDiagnostics() const override {
-      return SupportsCrossFileDiagnostics;
-    }
-  };
 } // end anonymous namespace
 
 namespace {
 
 /// A helper class for emitting a single report.
 class PlistPrinter {
-  const FIDMap& FM;
+  const FIDMap &FM;
   AnalyzerOptions &AnOpts;
   const Preprocessor &PP;
   llvm::SmallVector<const PathDiagnosticMacroPiece *, 0> MacroPieces;
 
 public:
-  PlistPrinter(const FIDMap& FM, AnalyzerOptions &AnOpts,
+  PlistPrinter(const FIDMap &FM, AnalyzerOptions &AnOpts,
                const Preprocessor &PP)
-    : FM(FM), AnOpts(AnOpts), PP(PP) {
-  }
+      : FM(FM), AnOpts(AnOpts), PP(PP) {}
 
-  void ReportDiag(raw_ostream &o, const PathDiagnosticPiece& P) {
+  void ReportDiag(raw_ostream &o, const PathDiagnosticPiece &P) {
     ReportPiece(o, P, /*indent*/ 4, /*depth*/ 0, /*includeControlFlow*/ true);
 
     // Don't emit a warning about an unused private field.
     (void)AnOpts;
   }
 
   /// Print the expansions of the collected macro pieces.
   ///
   /// Each time ReportDiag is called on a PathDiagnosticMacroPiece (or, if one
   /// is found through a call piece, etc), it's subpieces are reported, and the
   /// piece itself is collected. Call this function after the entire bugpath
   /// was reported.
   void ReportMacroExpansions(raw_ostream &o, unsigned indent);
 
 private:
   void ReportPiece(raw_ostream &o, const PathDiagnosticPiece &P,
                    unsigned indent, unsigned depth, bool includeControlFlow,
                    bool isKeyEvent = false) {
     switch (P.getKind()) {
-      case PathDiagnosticPiece::ControlFlow:
-        if (includeControlFlow)
-          ReportControlFlow(o, cast<PathDiagnosticControlFlowPiece>(P), indent);
-        break;
-      case PathDiagnosticPiece::Call:
-        ReportCall(o, cast<PathDiagnosticCallPiece>(P), indent,
-                   depth);
-        break;
-      case PathDiagnosticPiece::Event:
-        ReportEvent(o, cast<PathDiagnosticEventPiece>(P), indent, depth,
-                    isKeyEvent);
-        break;
-      case PathDiagnosticPiece::Macro:
-        ReportMacroSubPieces(o, cast<PathDiagnosticMacroPiece>(P), indent,
-                             depth);
-        break;
-      case PathDiagnosticPiece::Note:
-        ReportNote(o, cast<PathDiagnosticNotePiece>(P), indent);
-        break;
+      ReportCall(o, cast<PathDiagnosticCallPiece>(P), indent, depth);
+      ReportMacroSubPieces(o, cast<PathDiagnosticMacroPiece>(P), indent, depth);
+    case PathDiagnosticPiece::PopUp:
+      ReportPopUp(o, cast<PathDiagnosticPopUpPiece>(P), indent);
+      break;
     }
   }
 
   void EmitRanges(raw_ostream &o, const ArrayRef<SourceRange> Ranges,
                   unsigned indent);
   void EmitMessage(raw_ostream &o, StringRef Message, unsigned indent);
 
   void ReportControlFlow(raw_ostream &o,
-                         const PathDiagnosticControlFlowPiece& P,
+                         const PathDiagnosticControlFlowPiece &P,
                          unsigned indent);
-  void ReportEvent(raw_ostream &o, const PathDiagnosticEventPiece& P,
+  void ReportEvent(raw_ostream &o, const PathDiagnosticEventPiece &P,
                    unsigned indent, unsigned depth, bool isKeyEvent = false);
   void ReportCall(raw_ostream &o, const PathDiagnosticCallPiece &P,
                   unsigned indent, unsigned depth);
-  void ReportMacroSubPieces(raw_ostream &o, const PathDiagnosticMacroPiece& P,
+  void ReportMacroSubPieces(raw_ostream &o, const PathDiagnosticMacroPiece &P,
                             unsigned indent, unsigned depth);
-  void ReportNote(raw_ostream &o, const PathDiagnosticNotePiece& P,
+  void ReportNote(raw_ostream &o, const PathDiagnosticNotePiece &P,
                   unsigned indent);
+  void ReportPopUp(raw_ostream &o, const PathDiagnosticPopUpPiece &P,
+                   unsigned indent);
 };
 
 } // end of anonymous namespace
 
 namespace {
 
 struct ExpansionInfo {
   std::string MacroName;
   std::string Expansion;
   ExpansionInfo(std::string N, std::string E)
       : MacroName(std::move(N)), Expansion(std::move(E)) {}
 };
 
 } // end of anonymous namespace
 
-static void printBugPath(llvm::raw_ostream &o, const FIDMap& FM,
-                         AnalyzerOptions &AnOpts,
-                         const Preprocessor &PP,
+static void printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
+                         AnalyzerOptions &AnOpts, const Preprocessor &PP,
                          const PathPieces &Path);
 
 /// Print coverage information to output stream {@code o}.
 /// May modify the used list of files {@code Fids} by inserting new ones.
-static void printCoverage(const PathDiagnostic *D,
-                          unsigned InputIndentLevel,
-                          SmallVectorImpl<FileID> &Fids,
-                          FIDMap &FM,
+static void printCoverage(const PathDiagnostic *D, unsigned InputIndentLevel,
+                          SmallVectorImpl<FileID> &Fids, FIDMap &FM,
                           llvm::raw_fd_ostream &o);
 
 static ExpansionInfo getExpandedMacro(SourceLocation MacroLoc,
                                       const Preprocessor &PP);
 
 //===----------------------------------------------------------------------===//
 // Methods of PlistPrinter.
 //===----------------------------------------------------------------------===//
void PlistPrinter::EmitRanges(raw_ostream &o,
   Indent(o, indent) << "<array>\n";
   ++indent;
 
   const SourceManager &SM = PP.getSourceManager();
   const LangOptions &LangOpts = PP.getLangOpts();
 
   for (auto &R : Ranges)
     EmitRange(o, SM,
-              Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts),
-              FM, indent + 1);
+              Lexer::getAsCharRange(SM.getExpansionRange(R), SM, LangOpts), FM,
+              indent + 1);
   --indent;
   Indent(o, indent) << "</array>\n";
 }
 
 void PlistPrinter::EmitMessage(raw_ostream &o, StringRef Message,
                                unsigned indent) {
   // Output the text.
   assert(!Message.empty());
   Indent(o, indent) << "<key>extended_message</key>\n";
   Indent(o, indent);
   EmitString(o, Message) << '\n';
 
   // Output the short text.
   // FIXME: Really use a short string.
   Indent(o, indent) << "<key>message</key>\n";
   Indent(o, indent);
   EmitString(o, Message) << '\n';
 }
 
 void PlistPrinter::ReportControlFlow(raw_ostream &o,
-                                     const PathDiagnosticControlFlowPiece& P,
+                                     const PathDiagnosticControlFlowPiece &P,
                                      unsigned indent) {
 
   const SourceManager &SM = PP.getSourceManager();
   const LangOptions &LangOpts = PP.getLangOpts();
 
   Indent(o, indent) << "<dict>\n";
   ++indent;
 
   Indent(o, indent) << "<key>kind</key><string>control</string>\n";
 
   // Emit edges.
   Indent(o, indent) << "<key>edges</key>\n";
   ++indent;
   Indent(o, indent) << "<array>\n";
   ++indent;
-  for (PathDiagnosticControlFlowPiece::const_iterator I=P.begin(), E=P.end();
-       I!=E; ++I) {
+  for (PathDiagnosticControlFlowPiece::const_iterator I = P.begin(),
+                                                      E = P.end();
+       I != E; ++I) {
     Indent(o, indent) << "<dict>\n";
     ++indent;
 
-    // Make the ranges of the start and end point self-consistent with adjacent edges
-    // by forcing to use only the beginning of the range.  This simplifies the layout
-    // logic for clients.
+    // Make the ranges of the start and end point self-consistent with adjacent
+    // edges by forcing to use only the beginning of the range.  This simplifies
+    // the layout logic for clients.
     Indent(o, indent) << "<key>start</key>\n";
     SourceRange StartEdge(
         SM.getExpansionLoc(I->getStart().asRange().getBegin()));
     EmitRange(o, SM, Lexer::getAsCharRange(StartEdge, SM, LangOpts), FM,
               indent + 1);
 
     Indent(o, indent) << "<key>end</key>\n";
     SourceRange EndEdge(SM.getExpansionLoc(I->getEnd().asRange().getBegin()));
if (!s.empty()) {
     Indent(o, indent) << "<key>alternate</key>";
     EmitString(o, s) << '\n';
   }
 
   --indent;
   Indent(o, indent) << "</dict>\n";
 }
 
-void PlistPrinter::ReportEvent(raw_ostream &o, const PathDiagnosticEventPiece& P,
+void PlistPrinter::ReportEvent(raw_ostream &o,
+                               const PathDiagnosticEventPiece &P,
                                unsigned indent, unsigned depth,
                                bool isKeyEvent) {
 
   const SourceManager &SM = PP.getSourceManager();
 
   Indent(o, indent) << "<dict>\n";
   ++indent;
 
void PlistPrinter::ReportEvent(raw_ostream &o,
   Indent(o, indent) << "<key>depth</key>";
   EmitInteger(o, depth) << '\n';
 
   // Output the text.
   EmitMessage(o, P.getString(), indent);
 
   // Finish up.
   --indent;
-  Indent(o, indent); o << "</dict>\n";
+  Indent(o, indent) << "</dict>\n";
 }
 
 void PlistPrinter::ReportCall(raw_ostream &o, const PathDiagnosticCallPiece &P,
-                              unsigned indent,
-                              unsigned depth) {
+                              unsigned indent, unsigned depth) {
 
   if (auto callEnter = P.getCallEnterEvent())
     ReportPiece(o, *callEnter, indent, depth, /*includeControlFlow*/ true,
                 P.isLastInMainSourceFile());
 
-
   ++depth;
 
   if (auto callEnterWithinCaller = P.getCallEnterWithinCallerEvent())
     ReportPiece(o, *callEnterWithinCaller, indent, depth,
                 /*includeControlFlow*/ true);
 
-  for (PathPieces::const_iterator I = P.path.begin(), E = P.path.end();I!=E;++I)
+  for (PathPieces::const_iterator I = P.path.begin(), E = P.path.end(); I != E;
+       ++I)
     ReportPiece(o, **I, indent, depth, /*includeControlFlow*/ true);
 
   --depth;
 
   if (auto callExit = P.getCallExitEvent())
     ReportPiece(o, *callExit, indent, depth, /*includeControlFlow*/ true);
 }
 
 void PlistPrinter::ReportMacroSubPieces(raw_ostream &o,
-                                        const PathDiagnosticMacroPiece& P,
+                                        const PathDiagnosticMacroPiece &P,
                                         unsigned indent, unsigned depth) {
   MacroPieces.push_back(&P);
 
   for (PathPieces::const_iterator I = P.subPieces.begin(),
                                   E = P.subPieces.end();
        I != E; ++I) {
     ReportPiece(o, **I, indent, depth, /*includeControlFlow*/ false);
   }
for (const PathDiagnosticMacroPiece *P : MacroPieces) {
     EmitString(o, EI.MacroName) << '\n';
 
     // Output what it expands into.
     Indent(o, indent) << "<key>expansion</key>";
     EmitString(o, EI.Expansion) << '\n';
 
     // Finish up.
     --indent;
-    Indent(o, indent);
-    o << "</dict>\n";
+    Indent(o, indent) << "</dict>\n";
   }
 }
 
-void PlistPrinter::ReportNote(raw_ostream &o, const PathDiagnosticNotePiece& P,
+void PlistPrinter::ReportNote(raw_ostream &o, const PathDiagnosticNotePiece &P,
                               unsigned indent) {
 
   const SourceManager &SM = PP.getSourceManager();
 
   Indent(o, indent) << "<dict>\n";
   ++indent;
 
   // 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); o << "</dict>\n";
+  Indent(o, indent) << "</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.
 //===----------------------------------------------------------------------===//
 
 /// Print coverage information to output stream {@code o}.
 /// May modify the used list of files {@code Fids} by inserting new ones.
-static void printCoverage(const PathDiagnostic *D,
-                          unsigned InputIndentLevel,
-                          SmallVectorImpl<FileID> &Fids,
-                          FIDMap &FM,
+static void printCoverage(const PathDiagnostic *D, unsigned InputIndentLevel,
+                          SmallVectorImpl<FileID> &Fids, FIDMap &FM,
                           llvm::raw_fd_ostream &o) {
   unsigned IndentLevel = InputIndentLevel;
 
   Indent(o, IndentLevel) << "<key>ExecutedLines</key>\n";
   Indent(o, IndentLevel) << "<dict>\n";
   IndentLevel++;
 
   // Mapping from file IDs to executed lines.
for (auto I = ExecutedLines.begin(), E = ExecutedLines.end(); I != E; ++I) {
     Indent(o, IndentLevel) << "</array>\n";
   }
   IndentLevel--;
   Indent(o, IndentLevel) << "</dict>\n";
 
   assert(IndentLevel == InputIndentLevel);
 }
 
-static void printBugPath(llvm::raw_ostream &o, const FIDMap& FM,
-                         AnalyzerOptions &AnOpts,
-                         const Preprocessor &PP,
+static void printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
+                         AnalyzerOptions &AnOpts, const Preprocessor &PP,
                          const PathPieces &Path) {
   PlistPrinter Printer(FM, AnOpts, PP);
-  assert(std::is_partitioned(
-           Path.begin(), Path.end(),
-           [](const std::shared_ptr<PathDiagnosticPiece> &E)
-             { return E->getKind() == PathDiagnosticPiece::Note; }) &&
+  assert(std::is_partitioned(Path.begin(), Path.end(),
+                             [](const std::shared_ptr<PathDiagnosticPiece> &E) {
+                               return E->getKind() == PathDiagnosticPiece::Note;
+                             }) &&
          "PathDiagnostic is not partitioned so that notes precede the rest");
 
-  PathPieces::const_iterator FirstNonNote = std::partition_point(
-      Path.begin(), Path.end(),
-      [](const std::shared_ptr<PathDiagnosticPiece> &E)
-        { return E->getKind() == PathDiagnosticPiece::Note; });
+  PathPieces::const_iterator FirstNonNote =
+      std::partition_point(Path.begin(), Path.end(),
+                           [](const std::shared_ptr<PathDiagnosticPiece> &E) {
+                             return E->getKind() == PathDiagnosticPiece::Note;
+                           });
 
   PathPieces::const_iterator I = Path.begin();
 
   if (FirstNonNote != Path.begin()) {
     o << "   <key>notes</key>\n"
          "   <array>\n";
 
     for (; I != FirstNonNote; ++I)
static void printBugPath(llvm::raw_ostream &o, const FIDMap &FM,
   o << "   </array>\n";
 }
 
 //===----------------------------------------------------------------------===//
 // Methods of PlistDiagnostics.
 //===----------------------------------------------------------------------===//
 
 PlistDiagnostics::PlistDiagnostics(AnalyzerOptions &AnalyzerOpts,
-                                   const std::string& output,
+                                   const std::string &output,
                                    const Preprocessor &PP,
                                    bool supportsMultipleFiles)
     : OutputFile(output), PP(PP), AnOpts(AnalyzerOpts),
       SupportsCrossFileDiagnostics(supportsMultipleFiles) {}
 
 void ento::createPlistDiagnosticConsumer(AnalyzerOptions &AnalyzerOpts,
                                          PathDiagnosticConsumers &C,
-                                         const std::string& s,
+                                         const std::string &s,
                                          const Preprocessor &PP) {
   C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
                                    /*supportsMultipleFiles*/ false));
 }
 
 void ento::createPlistMultiFileDiagnosticConsumer(AnalyzerOptions &AnalyzerOpts,
                                                   PathDiagnosticConsumers &C,
                                                   const std::string &s,
                                                   const Preprocessor &PP) {
   C.push_back(new PlistDiagnostics(AnalyzerOpts, s, PP,
                                    /*supportsMultipleFiles*/ true));
 }
 void PlistDiagnostics::FlushDiagnosticsImpl(
-                                    std::vector<const PathDiagnostic *> &Diags,
-                                    FilesMade *filesMade) {
+    std::vector<const PathDiagnostic *> &Diags, FilesMade *filesMade) {
   // Build up a set of FIDs that we use by scanning the locations and
   // ranges of the diagnostics.
   FIDMap FM;
   SmallVector<FileID, 10> Fids;
-  const SourceManager& SM = PP.getSourceManager();
+  const SourceManager &SM = PP.getSourceManager();
   const LangOptions &LangOpts = PP.getLangOpts();
 
   auto AddPieceFID = [&FM, &Fids, &SM](const PathDiagnosticPiece &Piece) {
     AddFID(FM, Fids, SM, Piece.getLocation().asLocation());
     ArrayRef<SourceRange> Ranges = Piece.getRanges();
     for (const SourceRange &Range : Ranges) {
       AddFID(FM, Fids, SM, Range.getBegin());
       AddFID(FM, Fids, SM, Range.getEnd());
void PlistDiagnostics::FlushDiagnosticsImpl(
   }
 
   EmitPlistHeader(o);
 
   // Write the root object: a <dict> containing...
   //  - "clang_version", the string representation of clang version
   //  - "files", an <array> mapping from FIDs to file names
   //  - "diagnostics", an <array> containing the path diagnostics
-  o << "<dict>\n" <<
-       " <key>clang_version</key>\n";
+  o << "<dict>\n"
+    << " <key>clang_version</key>\n";
   EmitString(o, getClangFullVersion()) << '\n';
   o << " <key>diagnostics</key>\n"
        " <array>\n";
 
-  for (std::vector<const PathDiagnostic*>::iterator DI=Diags.begin(),
-       DE = Diags.end(); DI!=DE; ++DI) {
+  for (std::vector<const PathDiagnostic *>::iterator DI = Diags.begin(),
+                                                     DE = Diags.end();
+       DI != DE; ++DI) {
 
     o << "  <dict>\n";
 
     const PathDiagnostic *D = *DI;
     printBugPath(o, FM, AnOpts, PP, D->path);
 
     // Output the bug type and bug category.
     o << "   <key>description</key>";
     EmitString(o, D->getShortDescription()) << '\n';
     o << "   <key>category</key>";
     EmitString(o, D->getCategory()) << '\n';
     o << "   <key>type</key>";
     EmitString(o, D->getBugType()) << '\n';
     o << "   <key>check_name</key>";
     EmitString(o, D->getCheckName()) << '\n';
 
     o << "   <!-- This hash is experimental and going to change! -->\n";
     o << "   <key>issue_hash_content_of_line_in_context</key>";
     PathDiagnosticLocation UPDLoc = D->getUniqueingLoc();
     FullSourceLoc L(SM.getExpansionLoc(UPDLoc.isValid()
-                                            ? UPDLoc.asLocation()
-                                            : D->getLocation().asLocation()),
                     SM);
     const Decl *DeclWithIssue = D->getDeclWithIssue();
     EmitString(o, GetIssueHash(SM, L, D->getCheckName(), D->getBugType(),
                                DeclWithIssue, LangOpts))
         << '\n';
 
     // Output information about the semantic context where
     // the issue occurred.
     if (const Decl *DeclWithIssue = D->getDeclWithIssue()) {
       // FIXME: handle blocks, which have no name.
       if (const NamedDecl *ND = dyn_cast<NamedDecl>(DeclWithIssue)) {
         StringRef declKind;
         switch (ND->getKind()) {
-          case Decl::CXXRecord:
-            declKind = "C++ class";
-            break;
-          case Decl::CXXMethod:
-            declKind = "C++ method";
-            break;
-          case Decl::ObjCMethod:
-            declKind = "Objective-C method";
-            break;
-          case Decl::Function:
-            declKind = "function";
-            break;
-          default:
-            break;
         }
         if (!declKind.empty()) {
           const std::string &declName = ND->getDeclName().getAsString();
           o << "  <key>issue_context_kind</key>";
           EmitString(o, declKind) << '\n';
           o << "  <key>issue_context</key>";
           EmitString(o, declName) << '\n';
         }
if (const Decl *DeclWithIssue = D->getDeclWithIssue()) {
             FullSourceLoc UFunL(
                 SM.getExpansionLoc(
                     D->getUniqueingDecl()->getBody()->getBeginLoc()),
                 SM);
             o << "  <key>issue_hash_function_offset</key><string>"
               << L.getExpansionLineNumber() - UFunL.getExpansionLineNumber()
               << "</string>\n";
 
             // Otherwise, use the location on which the bug is reported.
           } else {
             FullSourceLoc FunL(SM.getExpansionLoc(Body->getBeginLoc()), SM);
             o << "  <key>issue_hash_function_offset</key><string>"
               << L.getExpansionLineNumber() - FunL.getExpansionLineNumber()
               << "</string>\n";
           }
-
         }
       }
     }
 
     // Output the location of the bug.
     o << "  <key>location</key>\n";
     EmitLocation(o, SM, D->getLocation().asLocation(), FM, 2);
 
     // Output the diagnostic to the sub-diagnostic client, if any.
     if (!filesMade->empty()) {
       StringRef lastName;
       PDFileEntry::ConsumerFiles *files = filesMade->getFiles(*D);
       if (files) {
         for (PDFileEntry::ConsumerFiles::const_iterator CI = files->begin(),
-                CE = files->end(); CI != CE; ++CI) {
+                                                        CE = files->end();
+             CI != CE; ++CI) {
           StringRef newName = CI->first;
           if (newName != lastName) {
             if (!lastName.empty()) {
               o << "  </array>\n";
             }
             lastName = newName;
-            o <<  "  <key>" << lastName << "_files</key>\n";
+            o << "  <key>" << lastName << "_files</key>\n";
             o << "  <array>\n";
           }
           o << "   <string>" << CI->second << "</string>\n";
         }
         o << "  </array>\n";
       }
     }
 

 };
 
 struct MacroNameAndArgs {
   std::string Name;
   const MacroInfo *MI = nullptr;
   MacroArgMap Args;
 
   MacroNameAndArgs(std::string N, const MacroInfo *MI, MacroArgMap M)
       : Name(std::move(N)), MI(MI), Args(std::move(M)) {}
 };
 
 class TokenPrinter {
   llvm::raw_ostream &OS;
   const Preprocessor &PP;
 
   Token PrevTok, PrevPrevTok;
   TokenConcatenation ConcatInfo;
 
 public:
   TokenPrinter(llvm::raw_ostream &OS, const Preprocessor &PP)
       : OS(OS), PP(PP), ConcatInfo(PP) {
     PrevTok.setKind(tok::unknown);
     PrevPrevTok.setKind(tok::unknown);
   }
 
   void printToken(const Token &Tok);
 };
 
 } // end of anonymous namespace

 /// To avoid infinite recursion we maintain the already processed tokens in
 /// a set. This is carried as a parameter through the recursive calls. The set
 /// is extended with the currently processed token and after processing it, the
 /// token is removed. If the token is already in the set, then recursion stops:
 ///
 /// #define f(y) x
 /// #define x f(x)
 static std::string getMacroNameAndPrintExpansion(
-    TokenPrinter &Printer,
-    SourceLocation MacroLoc,
-    const Preprocessor &PP,
+    TokenPrinter &Printer, SourceLocation MacroLoc, const Preprocessor &PP,
     const MacroArgMap &PrevArgs,
     llvm::SmallPtrSet<IdentifierInfo *, 8> &AlreadyProcessedTokens);
 
 /// Retrieves the name of the macro and what it's arguments expand into
 /// at \p ExpanLoc.
 ///
 /// For example, for the following macro expansion:
 ///

 ///
 /// When \p ExpanLoc references "SET_TO_NULL(a)" within the definition of
 /// "NOT_SUSPICOUS", the macro name "SET_TO_NULL" and the MacroArgMap map
 /// { (x, a) } will be returned.
 static MacroNameAndArgs getMacroNameAndArgs(SourceLocation ExpanLoc,
                                             const Preprocessor &PP);
 
 /// Retrieves the ')' token that matches '(' \p It points to.
-static MacroInfo::tokens_iterator getMatchingRParen(
-    MacroInfo::tokens_iterator It,
+static MacroInfo::tokens_iterator
+getMatchingRParen(MacroInfo::tokens_iterator It,
                   MacroInfo::tokens_iterator End);
 
 /// Retrieves the macro info for \p II refers to at \p Loc. This is important
 /// because macros can be redefined or undefined.
 static const MacroInfo *getMacroInfoForLocation(const Preprocessor &PP,
                                                 const SourceManager &SM,
                                                 const IdentifierInfo *II,
                                                 SourceLocation Loc);
 
 //===----------------------------------------------------------------------===//
 // Definitions of helper functions and methods for expanding macros.
 //===----------------------------------------------------------------------===//
 
 static ExpansionInfo getExpandedMacro(SourceLocation MacroLoc,
                                       const Preprocessor &PP) {
 
   llvm::SmallString<200> ExpansionBuf;
   llvm::raw_svector_ostream OS(ExpansionBuf);
   TokenPrinter Printer(OS, PP);
-  llvm::SmallPtrSet<IdentifierInfo*, 8> AlreadyProcessedTokens;
+  llvm::SmallPtrSet<IdentifierInfo *, 8> AlreadyProcessedTokens;
 
-  std::string MacroName =
-            getMacroNameAndPrintExpansion(Printer, MacroLoc, PP, MacroArgMap{},
-                                         AlreadyProcessedTokens);
-  return { MacroName, OS.str() };
+  std::string MacroName = getMacroNameAndPrintExpansion(
+      Printer, MacroLoc, PP, MacroArgMap{}, AlreadyProcessedTokens);
+  return {MacroName, OS.str()};
 }
 
 static std::string getMacroNameAndPrintExpansion(
-    TokenPrinter &Printer,
-    SourceLocation MacroLoc,
-    const Preprocessor &PP,
+    TokenPrinter &Printer, SourceLocation MacroLoc, const Preprocessor &PP,
     const MacroArgMap &PrevArgs,
     llvm::SmallPtrSet<IdentifierInfo *, 8> &AlreadyProcessedTokens) {
 
   const SourceManager &SM = PP.getSourceManager();
 
   MacroNameAndArgs Info = getMacroNameAndArgs(SM.getExpansionLoc(MacroLoc), PP);
-  IdentifierInfo* IDInfo = PP.getIdentifierInfo(Info.Name);
+  IdentifierInfo *IDInfo = PP.getIdentifierInfo(Info.Name);
 
   // TODO: If the macro definition contains another symbol then this function is
   // called recursively. In case this symbol is the one being defined, it will
   // be an infinite recursion which is stopped by this "if" statement. However,
   // in this case we don't get the full expansion text in the Plist file. See
   // the test file where "value" is expanded to "garbage_" instead of
   // "garbage_value".
   if (AlreadyProcessedTokens.find(IDInfo) != AlreadyProcessedTokens.end())
for (auto It = Info.MI->tokens_begin(), E = Info.MI->tokens_end(); It != E;
 
     // If this token is not an identifier, we only need to print it.
     if (T.isNot(tok::identifier)) {
       Printer.printToken(T);
       continue;
     }
 
     const auto *II = T.getIdentifierInfo();
-    assert(II &&
-          "This token is an identifier but has no IdentifierInfo!");
+    assert(II && "This token is an identifier but has no IdentifierInfo!");
 
     // If this token is a macro that should be expanded inside the current
     // macro.
     if (getMacroInfoForLocation(PP, SM, II, T.getLocation())) {
       getMacroNameAndPrintExpansion(Printer, T.getLocation(), PP, Info.Args,
                                     AlreadyProcessedTokens);
 
       // If this is a function-like macro, skip its arguments, as
static MacroNameAndArgs getMacroNameAndArgs(SourceLocation ExpanLoc,
   const MacroInfo *MI = getMacroInfoForLocation(PP, SM, II, ExpanLoc);
   // assert(MI && "The macro must've been defined at it's expansion location!");
   //
   // We should always be able to obtain the MacroInfo in a given TU, but if
   // we're running the analyzer with CTU, the Preprocessor won't contain the
   // directive history (or anything for that matter) from another TU.
   // TODO: assert when we're not running with CTU.
   if (!MI)
-    return { MacroName, MI, {} };
+    return {MacroName, MI, {}};
 
   // Acquire the macro's arguments.
   //
   // The rough idea here is to lex from the first left parentheses to the last
   // right parentheses, and map the macro's unexpanded arguments to what they
   // will be expanded to. An expanded macro argument may contain several tokens
   // (like '3 + 4'), so we'll lex until we find a tok::comma or tok::r_paren, at
   // which point we start lexing the next argument or finish.
   ArrayRef<const IdentifierInfo *> MacroArgs = MI->params();
   if (MacroArgs.empty())
-    return { MacroName, MI, {} };
+    return {MacroName, MI, {}};
 
   RawLexer.LexFromRawLexer(TheTok);
   // When this is a token which expands to another macro function then its
   // parentheses are not at its expansion locaiton. For example:
   //
   // #define foo(x) int bar() { return x; }
   // #define apply_zero(f) f(0)
   // apply_zero(foo)
   //               ^
   //               This is not a tok::l_paren, but foo is a function.
   if (TheTok.isNot(tok::l_paren))
-    return { MacroName, MI, {} };
+    return {MacroName, MI, {}};
 
   MacroArgMap Args;
 
   // When the macro's argument is a function call, like
   //   CALL_FN(someFunctionName(param1, param2))
   // we will find tok::l_paren, tok::r_paren, and tok::comma that do not divide
   // actual macro arguments, or do not represent the macro argument's closing
   // parentheses, so we'll count how many parentheses aren't closed yet.
for (const IdentifierInfo *UnexpArgII : MacroArgs) {
     //                  // empty parameter -- there are no parameters for '...'.
     // In any other case, ParenthesesDepth mustn't be 0 here.
     if (ParenthesesDepth != 0) {
 
       // Lex the first token of the next macro parameter.
       RawLexer.LexFromRawLexer(TheTok);
 
       while (!(ParenthesesDepth == 1 &&
                (UnexpArgII == __VA_ARGS__II ? false : TheTok.is(tok::comma)))) {
         assert(TheTok.isNot(tok::eof) &&
                "EOF encountered while looking for expanded macro args!");
 
         if (TheTok.is(tok::l_paren))
           ++ParenthesesDepth;
 
         if (TheTok.is(tok::r_paren))
           --ParenthesesDepth;
for (const IdentifierInfo *UnexpArgII : MacroArgs) {
 
     Args.emplace(UnexpArgII, std::move(ExpandedArgTokens));
   }
 
   assert(TheTok.is(tok::r_paren) &&
          "Expanded macro argument acquisition failed! After the end of the loop"
          " this token should be ')'!");
 
-  return { MacroName, MI, Args };
+  return {MacroName, MI, Args};
 }
 
-static MacroInfo::tokens_iterator getMatchingRParen(
-    MacroInfo::tokens_iterator It,
+static MacroInfo::tokens_iterator
+getMatchingRParen(MacroInfo::tokens_iterator It,
                   MacroInfo::tokens_iterator End) {
 
   assert(It->is(tok::l_paren) && "This token should be '('!");
 
   // Skip until we find the closing ')'.
   int ParenthesesDepth = 1;
   while (ParenthesesDepth != 0) {
     ++It;
 
while (It != CurrExpArgTokens.end()) {
       // Is this an argument that "Super" expands further?
       if (!Super.count(II)) {
         ++It;
         continue;
       }
 
       const ExpArgTokens &SuperExpArgTokens = Super.at(II);
 
-      It = CurrExpArgTokens.insert(
-          It, SuperExpArgTokens.begin(), SuperExpArgTokens.end());
+      It = CurrExpArgTokens.insert(It, SuperExpArgTokens.begin(),
+                                   SuperExpArgTokens.end());
       std::advance(It, SuperExpArgTokens.size());
       It = CurrExpArgTokens.erase(It);
     }
   }
 }
 
 void TokenPrinter::printToken(const Token &Tok) {
   // If this is the first token to be printed, don't print space.
   if (PrevTok.isNot(tok::unknown)) {
     // If the tokens were already space separated, or if they must be to avoid
     // them being implicitly pasted, add a space between them.
-    if(Tok.hasLeadingSpace() || ConcatInfo.AvoidConcat(PrevPrevTok, PrevTok,
-                                                       Tok)) {
+    if (Tok.hasLeadingSpace() ||
+        ConcatInfo.AvoidConcat(PrevPrevTok, PrevTok, Tok)) {
       // AvoidConcat doesn't check for ##, don't print a space around it.
       if (PrevTok.isNot(tok::hashhash) && Tok.isNot(tok::hashhash)) {
         OS << ' ';
       }
     }
   }
 
   if (!Tok.isOneOf(tok::hash, tok::hashhash)) {

clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp

static json::Object createLocation(json::Object &&PhysicalLocation,
   json::Object Ret{{"physicalLocation", std::move(PhysicalLocation)}};
   if (!Message.empty())
     Ret.insert({"message", createMessage(Message)});
   return Ret;
 }
 
 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;
 }
 
 static json::Object createThreadFlow(const PathPieces &Pieces,
                                      json::Array &Files) {
   const SourceManager &SMgr = Pieces.front()->getLocation().getManager();