[analyzer] NFC: Replace intrusive list of bug reports with a vector of pointers.

[NoQ]

Summary

They are said to be more efficient and they're definitely less brain-damaging. Suggested by @gribozavr.

[gribozavr]

Thanks for the simplification!

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

470

I don't think we intend users to use ReportList, so it would be better to not expose it. Instead:

using iterator = SmallVectorImpl<std::unique_ptr<BugReport>>::iterator;
using const_iterator = SmallVectorImpl<std::unique_ptr<BugReport>>::const_iterator;

... and move it closer to the usage point, right above begin() / end(). WDYT?

[gribozavr]

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

470

Could even do away with all these typedefs, and four begin/end overloads, and replace everything with:

ArrayRef<std::unique_ptr<BugReport>> Reports() const;

I believe we don't even need a non-const overload since unique_ptr allows mutations regardless.

470

BugReports would be a better name.

[Szelethus]

Other then the things @gribozavr mentioned, LGTM.

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

468

Prefer using.

[NoQ]

Fxd, thanks!

[gribozavr]

Still LGTM, just some nitpicks to replace iterator usage with index-based accesses (which I believe is simpler).

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

569

getReports()[0] ?

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

2803

assert(!EQ.getReports().empty());
const BugType& BT = EQ.getReports()[0]->getBugType();

?

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

3139

getReports()[0] ?

[NoQ]

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

569

Mm right :)

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

 
 /// A mapping from diagnostic consumers to the diagnostics they should
 /// consume.
 using DiagnosticForConsumerMapTy =
     llvm::DenseMap<PathDiagnosticConsumer *, std::unique_ptr<PathDiagnostic>>;
 
 /// This class provides an interface through which checkers can create
 /// individual bug reports.
-class BugReport : public llvm::ilist_node<BugReport> {
+class BugReport {
 public:
   enum class Kind { Basic, PathSensitive };
 
 protected:
   friend class BugReportEquivClass;
   friend class BugReporter;
 
   Kind K;

 //===----------------------------------------------------------------------===//
 // BugTypes (collections of related reports).
 //===----------------------------------------------------------------------===//
 
 class BugReportEquivClass : public llvm::FoldingSetNode {
   friend class BugReporter;
 
   /// List of *owned* BugReport objects.
-  llvm::ilist<BugReport> Reports;
+  llvm::SmallVector<std::unique_ptr<BugReport>, 4> Reports;
 
-  void AddReport(std::unique_ptr<BugReport> R) {
-    Reports.push_back(R.release());
+  void AddReport(std::unique_ptr<BugReport> &&R) {
+    Reports.push_back(std::move(R));
   }
 
 public:
   BugReportEquivClass(std::unique_ptr<BugReport> R) { AddReport(std::move(R)); }
 
+  ArrayRef<std::unique_ptr<BugReport>> getReports() const { return Reports; }
+
   void Profile(llvm::FoldingSetNodeID& ID) const {
     assert(!Reports.empty());
-    Reports.front().Profile(ID);
+    Reports.front()->Profile(ID);
   }
-
-  using iterator = llvm::ilist<BugReport>::iterator;
-  using const_iterator = llvm::ilist<BugReport>::const_iterator;
-
-  iterator begin() { return Reports.begin(); }
-  iterator end() { return Reports.end(); }
-
-  const_iterator begin() const { return Reports.begin(); }
-  const_iterator end() const { return Reports.end(); }
 };
 
 //===----------------------------------------------------------------------===//
 // BugReporter and friends.
 //===----------------------------------------------------------------------===//
 
 class BugReporterData {
 public:
private:
   /// Returns a BugType that is associated with the given name and
   /// category.
   BugType *getBugTypeForName(CheckName CheckName, StringRef name,
                              StringRef category);
 
   virtual BugReport *
   findReportInEquivalenceClass(BugReportEquivClass &eqClass,
                                SmallVectorImpl<BugReport *> &bugReports) {
-    return &*eqClass.begin();
+    return eqClass.getReports().begin()->get();
   }
 
 protected:
   /// Generate the diagnostics for the given bug report.
   virtual std::unique_ptr<DiagnosticForConsumerMapTy>
   generateDiagnosticForConsumerMap(BugReport *exampleReport,
                                    ArrayRef<PathDiagnosticConsumer *> consumers,
                                    ArrayRef<BugReport *> bugReports);

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

struct FRIEC_WLItem {
   FRIEC_WLItem(const ExplodedNode *n)
       : N(n), I(N->succ_begin()), E(N->succ_end()) {}
 };
 
 } // namespace
 
 BugReport *PathSensitiveBugReporter::findReportInEquivalenceClass(
     BugReportEquivClass &EQ, SmallVectorImpl<BugReport *> &bugReports) {
-  BugReportEquivClass::iterator I = EQ.begin(), E = EQ.end();
+  auto I = EQ.getReports().begin(), E = EQ.getReports().end();
   assert(I != E);
-  const BugType& BT = I->getBugType();
+  const BugType& BT = (*I)->getBugType();
 
   // If we don't need to suppress any of the nodes because they are
   // post-dominated by a sink, simply add all the nodes in the equivalence class
   // to 'Nodes'.  Any of the reports will serve as a "representative" report.
   if (!BT.isSuppressOnSink()) {
-    BugReport *R = &*I;
-    for (auto &I : EQ) {
-      if (auto *PR = dyn_cast<PathSensitiveBugReport>(&I)) {
+    BugReport *R = I->get();
+    for (auto &J : EQ.getReports()) {
+      if (auto *PR = dyn_cast<PathSensitiveBugReport>(J.get())) {
         R = PR;
         bugReports.push_back(PR);
       }
     }
     return R;
   }
 
   // For bug reports that should be suppressed when all paths are post-dominated
   // by a sink node, iterate through the reports in the equivalence class
   // until we find one that isn't post-dominated (if one exists).  We use a
   // DFS traversal of the ExplodedGraph to find a non-sink node.  We could write
   // this as a recursive function, but we don't want to risk blowing out the
   // stack for very long paths.
   BugReport *exampleReport = nullptr;
 
   for (; I != E; ++I) {
-    auto *R = dyn_cast<PathSensitiveBugReport>(&*I);
+    auto *R = dyn_cast<PathSensitiveBugReport>(I->get());
     if (!R)
       continue;
 
     const ExplodedNode *errorNode = R->getErrorNode();
     if (errorNode->isSink()) {
       llvm_unreachable(
            "BugType::isSuppressSink() should not be 'true' for sink end nodes");
     }

clang/lib/StaticAnalyzer/Core/ExprEngine.cpp

struct DOTGraphTraits<ExplodedGraph*> : public DefaultDOTGraphTraits {
   static bool nodeHasBugReport(const ExplodedNode *N) {
     BugReporter &BR = static_cast<ExprEngine &>(
       N->getState()->getStateManager().getOwningEngine()).getBugReporter();
 
     const auto EQClasses =
         llvm::make_range(BR.EQClasses_begin(), BR.EQClasses_end());
 
     for (const auto &EQ : EQClasses) {
-      for (const BugReport &R : EQ) {
-        const auto *PR = dyn_cast<PathSensitiveBugReport>(&R);
+      for (const auto &I : EQ.getReports()) {
+        const auto *PR = dyn_cast<PathSensitiveBugReport>(I.get());
         if (!PR)
           continue;
         const ExplodedNode *EN = PR->getErrorNode();
         if (EN->getState() == N->getState() &&
             EN->getLocation() == N->getLocation())
           return true;
       }
     }

 std::string ExprEngine::DumpGraph(bool trim, StringRef Filename) {
 #ifndef NDEBUG
   if (trim) {
     std::vector<const ExplodedNode *> Src;
 
     // Iterate through the reports and get their nodes.
     for (BugReporter::EQClasses_iterator
            EI = BR.EQClasses_begin(), EE = BR.EQClasses_end(); EI != EE; ++EI) {
-      const auto *R = dyn_cast<PathSensitiveBugReport>(&*EI->begin());
+      const auto *R =
+          dyn_cast<PathSensitiveBugReport>(EI->getReports().begin()->get());
       if (!R)
         continue;
       const auto *N = const_cast<ExplodedNode *>(R->getErrorNode());
       Src.push_back(N);
     }
     return DumpGraph(Src, Filename);
   } else {
     return llvm::WriteGraph(&G, "ExprEngine", /*ShortNames=*/false,