[PATCH] D103434: [analyzer] Allow visitors to run callbacks on completion

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:95
+  /// This method is run once the \ref Stop method is called.
+  virtual void OnStop(const ExplodedNode *Curr, BugReporterContext &BRC,
+                      PathSensitiveBugReport &BR) const = 0;
----------------
Are we anticipating visitors that do something else other than calling a callback when they are stopped? If that is not the case, I wonder if storing/invoking the visitor in this class would make more sense. 


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:97
+                      PathSensitiveBugReport &BR) const = 0;
+  bool Stopped;
+
----------------
Should we allow derived classes or even others query if a visitor was stopped? Or is this only for catching bugs? 


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:101
+  StoppableBugReporterVisitor() : Stopped{false} {}
+  virtual ~StoppableBugReporterVisitor();
+
----------------
I prefer really small functions, like this dtor to be in header files. Feel free to ignore.


================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:104
+  /// Call this method once the visitor runs out of useful nodes to process.
+  void Stop(const ExplodedNode *Curr, BugReporterContext &BRC,
+            PathSensitiveBugReport &BR);
----------------
Do you anticipate cases where the visitor itself does not know that it needs to stop (e.g. in the Visit functions) so someone needs to call this from the outside?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103434