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

[Jordan Rose]

On Mar 19, 2012, at 18:11, Anna Zaks wrote:

> Here is another spin on Ted's idea. This introduces even more performance overhead, but it might not matter anyway because the diagnostic generation is not where the analyzer performance bottleneck is.
> 
> (1) Before generating the whole path, set the "hasModifiedSymbols" to false.
> (2) During path generation, if any of the BugReporterVisitors modifies the set of interesting symbols, throw away the generated events and go back to (1).
> (3) If the interesting symbols don't change (hasModifiedSymbols == false), use the path.
> 
> To make this work, visitors would have to implement some sort of clone method (aka virtual copy constructor). 

Or have a reset() method. Or require that visitors be stateless. I think this is probably overkill for now, though. We can go with Ted's idea and change to redoing the whole path later if it turns out to be useful, but it would probably be more work to go the other way around.

> We also considered splitting the visitors/callbacks for generation of interesting symbols and diagnostic pieces. This idea would result in more strain on the checker-writer.

I don't like this so much -- it seems like unnecessary work. On the one hand, the two don't have much to do with each other. On the other, checkers that need to troll back through the ExplodedGraph to find interesting symbols can do so themselves, like getAllocationSite() already does. The benefit of having the BugReporter manage all this is to have only one traversal for many visitors, but at least for now, all the custom symbols come, directly or indirectly, from a checker.

All right, I'll start implementing this, but I'll send a patch before committing.