[analyzer] Try to re-apply r283092 "Extend bug reports with extra notes"

Replace SmallVector<IntrusiveRefCntPtr> with a vector of plain pointers.
Would insignificantly increase memory usage.

llvm-svn: 283536

5 files changed, 146 insertions, 46 deletions

diff --git a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index 54c668cd2d6..86c194e8fae 100644
--- a/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -344,3 +344,10 @@ bool AnalyzerOptions::shouldWidenLoops() {
     WidenLoops = getBooleanOption("widen-loops", /*Default=*/false);
   return WidenLoops.getValue();
 }
+
+bool AnalyzerOptions::shouldDisplayNotesAsEvents() {
+  if (!DisplayNotesAsEvents.hasValue())
+    DisplayNotesAsEvents =
+        getBooleanOption("notes-as-events", /*Default=*/false);
+  return DisplayNotesAsEvents.getValue();
+}
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 1800eff76bd..9bc7470c61d 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -114,15 +114,15 @@ static void removeRedundantMsgs(PathPieces &path) {
     path.pop_front();
 
     switch (piece->getKind()) {
-      case clang::ento::PathDiagnosticPiece::Call:
+      case PathDiagnosticPiece::Call:
         removeRedundantMsgs(cast<PathDiagnosticCallPiece>(piece)->path);
         break;
-      case clang::ento::PathDiagnosticPiece::Macro:
+      case PathDiagnosticPiece::Macro:
         removeRedundantMsgs(cast<PathDiagnosticMacroPiece>(piece)->subPieces);
         break;
-      case clang::ento::PathDiagnosticPiece::ControlFlow:
+      case PathDiagnosticPiece::ControlFlow:
         break;
-      case clang::ento::PathDiagnosticPiece::Event: {
+      case PathDiagnosticPiece::Event: {
         if (i == N-1)
           break;
 
@@ -142,6 +142,8 @@ static void removeRedundantMsgs(PathPieces &path) {
         }
         break;
       }
+      case PathDiagnosticPiece::Note:
+        break;
     }
     path.push_back(piece);
   }
@@ -199,6 +201,9 @@ static bool removeUnneededCalls(PathPieces &pieces, BugReport *R,
       }
       case PathDiagnosticPiece::ControlFlow:
         break;
+
+      case PathDiagnosticPiece::Note:
+        break;
     }
 
     pieces.push_back(piece);
@@ -2554,6 +2559,12 @@ BugReport::~BugReport() {
   while (!interestingSymbols.empty()) {
     popInterestingSymbolsAndRegions();
   }
+
+  // FIXME: Replace Notes with a list of shared pointers.
+  for (const auto *P: Notes) {
+    delete P;
+  }
+  Notes.clear();
 }
 
 const Decl *BugReport::getDeclWithIssue() const {
@@ -3405,25 +3416,28 @@ void BugReporter::FlushReport(BugReport *exampleReport,
       exampleReport->getUniqueingLocation(),
       exampleReport->getUniqueingDecl()));
 
-  MaxBugClassSize = std::max(bugReports.size(),
-                             static_cast<size_t>(MaxBugClassSize));
+  if (exampleReport->isPathSensitive()) {
+    // Generate the full path diagnostic, using the generation scheme
+    // specified by the PathDiagnosticConsumer. Note that we have to generate
+    // path diagnostics even for consumers which do not support paths, because
+    // the BugReporterVisitors may mark this bug as a false positive.
+    assert(!bugReports.empty());
+
+    MaxBugClassSize =
+        std::max(bugReports.size(), static_cast<size_t>(MaxBugClassSize));
 
-  // Generate the full path diagnostic, using the generation scheme
-  // specified by the PathDiagnosticConsumer. Note that we have to generate
-  // path diagnostics even for consumers which do not support paths, because
-  // the BugReporterVisitors may mark this bug as a false positive.
-  if (!bugReports.empty())
     if (!generatePathDiagnostic(*D.get(), PD, bugReports))
       return;
 
-  MaxValidBugClassSize = std::max(bugReports.size(),
-                                  static_cast<size_t>(MaxValidBugClassSize));
+    MaxValidBugClassSize =
+        std::max(bugReports.size(), static_cast<size_t>(MaxValidBugClassSize));
 
-  // Examine the report and see if the last piece is in a header. Reset the
-  // report location to the last piece in the main source file.
-  AnalyzerOptions& Opts = getAnalyzerOptions();
-  if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
-    D->resetDiagnosticLocationToMainFile();
+    // Examine the report and see if the last piece is in a header. Reset the
+    // report location to the last piece in the main source file.
+    AnalyzerOptions &Opts = getAnalyzerOptions();
+    if (Opts.shouldReportIssuesInMainSourceFile() && !Opts.AnalyzeAll)
+      D->resetDiagnosticLocationToMainFile();
+  }
 
   // If the path is empty, generate a single step path with the location
   // of the issue.
@@ -3436,6 +3450,28 @@ void BugReporter::FlushReport(BugReport *exampleReport,
     D->setEndOfPath(std::move(piece));
   }
 
+  PathPieces &Pieces = D->getMutablePieces();
+  bool ShouldConvert = getAnalyzerOptions().shouldDisplayNotesAsEvents();
+  // For path diagnostic consumers that don't support extra notes,
+  // we may optionally convert those to path notes.
+  for (auto I = exampleReport->getNotes().rbegin(),
+            E = exampleReport->getNotes().rend(); I != E; ++I) {
+    const PathDiagnosticNotePiece *Piece = *I;
+    // FIXME: getNotes() was supposed to return a list of shared pointers,
+    // and then we wouldn't normally create a new piece here,
+    // unless ShouldConvert is set.
+    PathDiagnosticPiece *ConvertedPiece =
+        ShouldConvert
+            ? static_cast<PathDiagnosticPiece *>(new PathDiagnosticEventPiece(
+                  Piece->getLocation(), Piece->getString()))
+            : static_cast<PathDiagnosticPiece *>(new PathDiagnosticNotePiece(
+                  Piece->getLocation(), Piece->getString()));
+    for (const auto &R : Piece->getRanges())
+      ConvertedPiece->addRange(R);
+
+    Pieces.push_front(ConvertedPiece);
+  }
+
   // Get the meta data.
   const BugReport::ExtraTextList &Meta = exampleReport->getExtraText();
   for (BugReport::ExtraTextList::const_iterator i = Meta.begin(),
@@ -3520,6 +3556,13 @@ LLVM_DUMP_METHOD void PathDiagnosticMacroPiece::dump() const {
   // FIXME: Print which macro is being invoked.
 }
 
+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 PathDiagnosticLocation::dump() const {
   if (!isValid()) {
     llvm::errs() << "<INVALID>\n";
diff --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 3a18956e413..f157c3dd6ce 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -152,13 +152,30 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
   }
 
   // Process the path.
-  unsigned n = path.size();
-  unsigned max = n;
-
-  for (PathPieces::const_reverse_iterator I = path.rbegin(),
-       E = path.rend();
-        I != E; ++I, --n)
-    HandlePiece(R, FID, **I, n, max);
+  // Maintain the counts of extra note pieces separately.
+  unsigned TotalPieces = path.size();
+  unsigned TotalNotePieces =
+      std::count_if(path.begin(), path.end(),
+                    [](const IntrusiveRefCntPtr<PathDiagnosticPiece> &p) {
+                      return isa<PathDiagnosticNotePiece>(p.get());
+                    });
+
+  unsigned TotalRegularPieces = TotalPieces - TotalNotePieces;
+  unsigned NumRegularPieces = TotalRegularPieces;
+  unsigned NumNotePieces = TotalNotePieces;
+
+  for (auto I = path.rbegin(), E = path.rend(); I != E; ++I) {
+    if (isa<PathDiagnosticNotePiece>(I->get())) {
+      // 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);
+      --NumNotePieces;
+    } else {
+      HandlePiece(R, FID, **I, NumRegularPieces, TotalRegularPieces);
+      --NumRegularPieces;
+    }
+  }
 
   // Add line numbers, header, footer, etc.
 
@@ -192,24 +209,38 @@ void HTMLDiagnostics::ReportDiag(const PathDiagnostic& D,
   int ColumnNumber = path.back()->getLocation().asLocation().getExpansionColumnNumber();
 
   // Add the name of the file as an <h1> tag.
-
   {
     std::string s;
     llvm::raw_string_ostream os(s);
 
     os << "<!-- REPORTHEADER -->\n"
-      << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
+       << "<h3>Bug Summary</h3>\n<table class=\"simpletable\">\n"
           "<tr><td class=\"rowname\">File:</td><td>"
-      << html::EscapeText(DirName)
-      << html::EscapeText(Entry->getName())
-      << "</td></tr>\n<tr><td class=\"rowname\">Location:</td><td>"
-         "<a href=\"#EndPath\">line "
-      << LineNumber
-      << ", column "
-      << ColumnNumber
-      << "</a></td></tr>\n"
-         "<tr><td class=\"rowname\">Description:</td><td>"
-      << D.getVerboseDescription() << "</td></tr>\n";
+       << html::EscapeText(DirName)
+       << html::EscapeText(Entry->getName())
+       << "</td></tr>\n<tr><td class=\"rowname\">Warning:</td><td>"
+          "<a href=\"#EndPath\">line "
+       << LineNumber
+       << ", column "
+       << ColumnNumber
+       << "</a><br />"
+       << D.getVerboseDescription() << "</td></tr>\n";
+
+    // The navigation across the extra notes pieces.
+    unsigned NumExtraPieces = 0;
+    for (const auto &Piece : path) {
+      if (const auto *P = dyn_cast<PathDiagnosticNotePiece>(Piece.get())) {
+        int LineNumber =
+            P->getLocation().asLocation().getExpansionLineNumber();
+        int ColumnNumber =
+            P->getLocation().asLocation().getExpansionColumnNumber();
+        os << "<tr><td class=\"rowname\">Note:</td><td>"
+           << "<a href=\"#Note" << NumExtraPieces << "\">line "
+           << LineNumber << ", column " << ColumnNumber << "</a><br />"
+           << P->getString() << "</td></tr>";
+        ++NumExtraPieces;
+      }
+    }
 
     // Output any other meta data.
 
@@ -385,13 +416,20 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
   // 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 should already be handled");
+      llvm_unreachable("Calls and extra notes should already be handled");
   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;
   }
 
   std::string sbuf;
@@ -399,7 +437,9 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
 
   os << "\n<tr><td class=\"num\"></td><td class=\"line\"><div id=\"";
 
-  if (num == max)
+  if (IsNote)
+    os << "Note" << num;
+  else if (num == max)
     os << "EndPath";
   else
     os << "Path" << num;
@@ -461,7 +501,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
 
   os << "\">";
 
-  if (max > 1) {
+  if (!SuppressIndex) {
     os << "<table class=\"msgT\"><tr><td valign=\"top\">";
     os << "<div class=\"PathIndex";
     if (Kind) os << " PathIndex" << Kind;
@@ -501,7 +541,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
 
     os << "':\n";
 
-    if (max > 1) {
+    if (!SuppressIndex) {
       os << "</td>";
       if (num < max) {
         os << "<td><div class=\"PathNav\"><a href=\"#";
@@ -523,7 +563,7 @@ void HTMLDiagnostics::HandlePiece(Rewriter& R, FileID BugFileID,
   else {
     os << html::EscapeText(P.getString());
 
-    if (max > 1) {
+    if (!SuppressIndex) {
       os << "</td>";
       if (num < max) {
         os << "<td><div class=\"PathNav\"><a href=\"#";
diff --git a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
index 9d2f7e8c2e8..5675cb2026f 100644
--- a/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
@@ -60,6 +60,7 @@ PathDiagnosticEventPiece::~PathDiagnosticEventPiece() {}
 PathDiagnosticCallPiece::~PathDiagnosticCallPiece() {}
 PathDiagnosticControlFlowPiece::~PathDiagnosticControlFlowPiece() {}
 PathDiagnosticMacroPiece::~PathDiagnosticMacroPiece() {}
+PathDiagnosticNotePiece::~PathDiagnosticNotePiece() {}
 
 void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
                            bool ShouldFlattenMacros) const {
@@ -95,6 +96,7 @@ void PathPieces::flattenTo(PathPieces &Primary, PathPieces &Current,
     }
     case PathDiagnosticPiece::Event:
     case PathDiagnosticPiece::ControlFlow:
+    case PathDiagnosticPiece::Note:
       Current.push_back(Piece);
       break;
     }
@@ -354,15 +356,16 @@ static Optional<bool> comparePiece(const PathDiagnosticPiece &X,
   }
 
   switch (X.getKind()) {
-    case clang::ento::PathDiagnosticPiece::ControlFlow:
+    case PathDiagnosticPiece::ControlFlow:
       return compareControlFlow(cast<PathDiagnosticControlFlowPiece>(X),
                                 cast<PathDiagnosticControlFlowPiece>(Y));
-    case clang::ento::PathDiagnosticPiece::Event:
+    case PathDiagnosticPiece::Event:
+    case PathDiagnosticPiece::Note:
       return None;
-    case clang::ento::PathDiagnosticPiece::Macro:
+    case PathDiagnosticPiece::Macro:
       return compareMacro(cast<PathDiagnosticMacroPiece>(X),
                           cast<PathDiagnosticMacroPiece>(Y));
-    case clang::ento::PathDiagnosticPiece::Call:
+    case PathDiagnosticPiece::Call:
       return compareCall(cast<PathDiagnosticCallPiece>(X),
                          cast<PathDiagnosticCallPiece>(Y));
   }
@@ -1110,6 +1113,10 @@ void PathDiagnosticMacroPiece::Profile(llvm::FoldingSetNodeID &ID) const {
     ID.Add(**I);
 }
 
+void PathDiagnosticNotePiece::Profile(llvm::FoldingSetNodeID &ID) const {
+  PathDiagnosticSpotPiece::Profile(ID);
+}
+
 void PathDiagnostic::Profile(llvm::FoldingSetNodeID &ID) const {
   ID.Add(getLocation());
   ID.AddString(BugType);
diff --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index 206156ca22a..c5263ee0e5c 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -281,6 +281,9 @@ static void ReportPiece(raw_ostream &o,
       ReportMacro(o, cast<PathDiagnosticMacroPiece>(P), FM, SM, LangOpts,
                   indent, depth);
       break;
+    case PathDiagnosticPiece::Note:
+      // FIXME: Extend the plist format to support those.
+      break;
   }
 }