[PATCH] D66572: [analyzer] BugReporter Separation Ep.I.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ marked 6 inline comments as done.
NoQ added a comment.

Most of the patch is boring but here are a few places that i wanted attract attention to.



================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:225
+
+protected:
+  /// The ExplodedGraph node against which the report was thrown. It corresponds
----------------
Yes, we allow inheriting from these specific `BugReport` sub-classes, because there turned out to be one checker that was using it.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h:56
 
-class RefCountReport : public BugReport {
+class RefCountReport : public PathSensitiveBugReport {
 protected:
----------------
I'm not pointing any fingers...


================
Comment at: clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h:77
 
 class RefLeakReport : public RefCountReport {
   const MemRegion* AllocBinding;
----------------
...and still not pointing any fingers.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2713
+
+  BugReporter::emitReport(std::move(R));
+}
----------------
I vaguely remember that this pattern is frowned upon. Does anybody want me to extract this into a common non-virtual function?


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2983-2984
+  if (isa<BasicBugReport>(exampleReport))
+    return BugReporter::generateDiagnosticForConsumerMap(exampleReport,
+                                                         consumers, bugReports);
 
----------------
Same question about extracting into a non-virtual function.


================
Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2993-2996
+  // Avoid copying the whole array because there may be a lot of reports.
+  ArrayRef<PathSensitiveBugReport *> convertedArrayOfReports(
+      reinterpret_cast<PathSensitiveBugReport *const *>(&*bugReports.begin()),
+      reinterpret_cast<PathSensitiveBugReport *const *>(&*bugReports.end()));
----------------
//*crosses fingers*//


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66572/new/

https://reviews.llvm.org/D66572