[cfe-commits] r162246 - in /cfe/trunk: lib/StaticAnalyzer/Core/BasicConstraintManager.cpp lib/StaticAnalyzer/Core/RangeConstraintManager.cpp test/Analysis/reference.cpp

[Ted Kremenek]

On Aug 21, 2012, at 9:29 AM, Jordan Rose <jordan_rose at apple.com> wrote:

> I agree, but there's no obvious place to put it. We can't add an explicit assumption when the symbol is created because just creating new symbols doesn't update the state.

What about a bit in the symbol object that implies this constraint?  If this is truly an invariant of the symbol, then it's fine to associate it with the symbol itself.

> And both constraint managers are lazy, but in different ways:
> 
> - RangeConstraintManager treats no range as "all possible values", but generates an actual RangeSet when it needs to do an intersection. This change takes 0 out of the generated set for references.
> - BasicConstraintManager collects a set of inequalities unless you actually assume an equality constraint. This change added an implicit "!= 0" for references but doesn't actually change the state.
> 

The major negative is now this is essentially hidden requirement of implementing a ConstraintManager.

> One way I could see to do this is to change SimpleConstraintManager's assumeSym?? methods to take an opaque type fetched from the state and return an Optional<opaque type> that would then be set back into the state if the assumption was feasible. Then you could also have an "addImplicitAssumptions" for the first time a state is accessed. But that would slow down every check on unconstrained symbols, and would require being non-lazy (and thus take more memory) to make sure we don't do it over and over. (Then again, we don't often make assumptions on completely unconstrained symbols that don't change the state in some way, and most if not all of those are caught before we even look at the ranges / inequalities.)

If these implicit assumptions are associated with a symbol for it's entire lifetime, I don't see what that information needs to be stored in the state.  The state only needs to track constraints that can vary over time.  Invariants for a symbol can be tracked elsewhere.

> 
> I'm not sure if that's the best way, though, or if it's really worth it at all. No one's actually ~using~ BasicConstraintManager right now.

Yes, we can just remove it, but that's a different issue.  :)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120821/2dccaf7e/attachment.html>