[cfe-commits] r153010 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/MallocChecker.cpp test/Analysis/malloc-plist.c

[Jordan Rose]

On Mar 19, 2012, at 15:59, Ted Kremenek wrote:

> On Mar 18, 2012, at 3:14 PM, Anna Zaks <ganna at apple.com> wrote:
> 
>> I also like the idea of visitors adding to the list of interesting symbols. Further, you could have a bunch of generic visitors and allow the checkers to choose which (of the generic ones) to use. You can also have a default list which gets generated with each BugReport by default.
>> 
>> Before you go down the road of changing the order, try to experiment with it (using the simplest solution) to see if there are any unknown issues that reordering might lead to in terms of desired output. For example, what happens if you have default and custom visitors generating a note for the same pair of nodes? The order might change the quality of output. 
> 
> Instead of changing the order, I propose the following (very simple) algorithm which makes the ordering irrelevant:
> 
> (1) When visiting the BugReporterVisitors for a given node, set a flag "hasModifiedSymbols" to false.
> 
> (2) Visit each BugReporterVisitor, collecting on the side all the PathDiagnosticEvents created for that node.  If any of the BugReporterVisitors modify the set of interesting symbols, throw away the generated PathDiagnosticEvents for that node and go back to (1).
> 
> (3) If the interesting symbols don't change (hasModifiedSymbols"== false), take the generated PathDiagnosticsEvents for that node and add it to PathPieces.
> 
> This at least defines away the problem of which visitors we visit first.  It is entirely possible that a custom visitor may add an interesting symbol that a general visitor will find important, and vis versa.
> 
> The only problem then is ordering the PathDiagnosticEvents generated at specific node.  This problem seems orthogonal to (and now decoupled from) the problem of actually generating the PathDiagnosticEvents in the first place.

I'm fine with this. We're not likely to have to do this often, so the hit of "throw away generated events" isn't so bad.

On the other hand, MallocBugVisitor is currently stateful -- that is, it assumes it's never going to see the same node-pair twice, and that the nodes are always visited end-to-start. I can make it less stateful (i.e. accept the same node-pair twice), but getting it down all the way to no state means a fair amount of rewriting. Is it part of the design that visitors be non-stateful, in case we ever want to traverse the nodes in some other order?