[cfe-commits] r137309 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h lib/StaticAnalyzer/Core/SymbolManager.cpp

Anna Zaks ganna at apple.com
Thu Aug 11 16:29:57 PDT 2011


On Aug 11, 2011, at 3:42 PM, Ted Kremenek wrote:

> On Aug 11, 2011, at 3:04 PM, Jordy Rose wrote:
> 
>> I'm curious what the motivation was for choosing to store the symbols as Base -> [list of dependents] instead of Dependent -> Base. I doubt many symbols are going to have multiple dependents, especially when they're being ORed together instead of ANDed. I realize that it makes it easy to clear the map when the base symbol dies, but since every dead symbol sweep hits every symbol anyway that doesn't seem to be a problem. (SymbolDerived works the other way around.)
> 
> I think you have a good point.  Going from base to dependents instead of dependents to base is probably more eager than we need to be.  We only need to care if a dependent's lifetime is extended if we think it might be dead.  If a dependent looks dead, lazily querying to see if a the base symbol is live seems slightly more efficient to me.
> 
> Anna: what do you think?

I think it would be cleaner to leave the Base -> [list of dependents] mapping:

1) The algorithm for determining when a symbol is alive or dead is complex, so we might need to wait until all the symbols are processed and then go though the dead ones checking if the dead symbols have the dependencies. I think the current design is cleaner and shouldn't be much slower given the suggested code review cleanups, in particular, "don't mark a symbol/dependents live if it's already live".

2) I think the checkers might want to specify that multiple symbols depend on one primary one, so keeping the map better represents that.

Anna.
> 
> As for not many symbols having multiple dependents, I don't think that's a valid assumption.  The whole point of this addition was to give checkers the ability to tie two symbols together (e.g., parameter out value and a return value).  There's no reason why there can't be multiple dependents.  Even if it's just two checkers that need to track dependencies between a common symbol and another, two is still more than one.
> 
>> 
>> Also, separate from implementation, would it be worth it to use this to /implement/ SymbolDerived? They're basically SymbolRegionValues with dependencies.
> 
> Yes, I had the same thought.  That would be a nice cleanup.



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110811/a62a367c/attachment.html>


More information about the cfe-commits mailing list