]> granicus.if.org Git - clang/commit
[analyzer][NFC] Refactoring BugReporter.cpp P2.: Clean up the construction of bug...
authorKristof Umann <dkszelethus@gmail.com>
Tue, 13 Aug 2019 13:56:12 +0000 (13:56 +0000)
committerKristof Umann <dkszelethus@gmail.com>
Tue, 13 Aug 2019 13:56:12 +0000 (13:56 +0000)
commit4f5f18624a6e82b35378a37a84ee00d91928cc34
tree63132bdf80b35e96e08fd048e006a2dfd81264a7
parent935f31410ddbc72b1b8563cf0766923b8fd6e9e4
[analyzer][NFC] Refactoring BugReporter.cpp P2.: Clean up the construction of bug paths and finding a valid report

This patch refactors the utility functions and classes around the construction
of a bug path.

At a very high level, this consists of 3 steps:

* For all BugReports in the same BugReportEquivClass, collect all their error
nodes in a set. With that set, create a new, trimmed ExplodedGraph whose leafs
are all error nodes.
* Until a valid report is found, construct a bug path, which is yet another
ExplodedGraph, that is linear from a given error node to the root of the graph.
* Run all visitors on the constructed bug path. If in this process the report
got invalidated, start over from step 2.

Now, to the changes within this patch:

* Do not allow the invalidation of BugReports up to the point where the trimmed
graph is constructed. Checkers shouldn't add bug reports that are known to be
invalid, and should use visitors and argue about the entirety of the bug path if
needed.
* Do not calculate indices. I may be biased, but I personally find code like
this horrible. I'd like to point you to one of the comments in the original code:

SmallVector<const ExplodedNode *, 32> errorNodes;
for (const auto I : bugReports) {
  if (I->isValid()) {
    HasValid = true;
    errorNodes.push_back(I->getErrorNode());
  } else {
    // Keep the errorNodes list in sync with the bugReports list.
    errorNodes.push_back(nullptr);
  }
}

Not on my watch. Instead, use a far easier to follow trick: store a pointer to
the BugReport in question, not an index to it.

* Add range iterators to ExplodedGraph's successors and predecessors, and a
visitor range to BugReporter.
* Rename TrimmedGraph to BugPathGetter. Because that is what it has always been:
no sane graph type should store an iterator-like state, or have an interface not
exposing a single graph-like functionalities.
* Rename ReportGraph to BugPathInfo, because it is only a linear path with some
other context.
* Instead of having both and out and in parameter (which I think isn't ever
excusable unless we use the out-param for caching), return a record object with
descriptive getter methods.
* Where descriptive names weren't sufficient, compliment the code with comments.

Differential Revision: https://reviews.llvm.org/D65379

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@368694 91177308-0d34-0410-b5e6-96231b3b80d8
include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExplodedGraph.h
lib/StaticAnalyzer/Core/BugReporter.cpp