[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

Wed Feb 20 02:14:04 PST 2019

baloghadamsoftware added a comment.

Thank you for working this. I totally agree with you: whenever the checker spawns a new node in the exploded graph there is no point to leave it unmarked and then revers engineer it. `BugReporterVisitor`s should only care for nodes which are not spawned by the checker reporting the bug. This way we could get rid of some unnecessary visitors, e.g. `CXXSelfAssignmentBRVisitor`. This also opens an easier way for checkers similar to `CXXSelfAssignmentChecker` which does not report any bug but creates a new execution path which may end in a bug reported by another checker. Using note tags there is no need to create a separate `BugReporterVisitor` for every such checker and add it globally to every bug report.

================
Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:348
+/// additional PathDiagnosticEventPieces along the path.
+class NoteTag : public ProgramPointTag {
+public:
----------------
I am not sure whether `BugReporterVisitors.h` is the best place for this structure. I would rather put this into `ProgramPoint.h` or maybe `BugReporter.h`.

Repository:
  rC Clang

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

https://reviews.llvm.org/D58367