[cfe-dev] Analyzer: SVal-building idioms

[Zhongxing Xu]

I find SValuator also constructs SVals. It seems a bit hard to
separate construct from evaluate. We construct when we cannot
evaluate. Combining them is reasonable.

On Wed, Jul 7, 2010 at 8:17 AM, Zhongxing Xu <xuzhongxing at gmail.com> wrote:
> I think these constructions are more appropriate to put into
> ValueManager, since ValueManager constructs SVals, but SValuator
> perform evaluations.
>
> On Wed, Jul 7, 2010 at 6:08 AM, Ted Kremenek <kremenek at apple.com> wrote:
>> Another idea: combine ValueManager and SValuator into a single class, and put the functionality there?
>>
>> I like the idea of moving most SVal construction to an SValBuilder class.  The base implementation would provide essential functionality (such as what is in ValueManager) and subclasses can implement functionality for more more advanced SVal construction.  There doesn't seem to have much value to keeping ValueManager and SValBuilder separate at this point, and most clients who have reference to one have a reference to the other.
>>
>> By unifying them, at defines a consistent place to put this functionality.
>>
>> On Jul 6, 2010, at 2:21 PM, Jordy Rose wrote:
>>
>>>
>>> In trying to excise the remnants of getSizeInElements() from the code, I
>>> realized we don't have a great place for SVal-building idioms. There's
>>> SValuator::EvalEQ, but nothing else. In looking through the code, I see two
>>> more useful idioms that it might be good to box up into methods:
>>>
>>> // perhaps with LengthTy defaulting to SizeTy or ArrayIndexTy
>>> SVal ConvertToCharUnits(const GRState *state, SVal Length,
>>>                        QualType EleTy, QualType LengthTy) {
>>>  // return Length * (LengthTy) sizeof(EleTy)
>>> }
>>>
>>> // To replace the rather useless AssumeInBound,
>>> // since this could be used to change the state
>>> SVal BuildBoundCheck(const GRState *state, SVal Index, SVal Limit,
>>>                     QualType IndexTy = ArrayIndexTy) {
>>>  // 0 <= Index < Limit
>>>  // is the same as
>>>  // Index+MIN < Limit+MIN
>>>  // which the constraint manager can handle now
>>>  // as long as either Index or Limit is constant
>>>  // and Limit is positive.
>>>  // return (Index + getMinValue(IndexTy)) < (Limit +
>>> getMinValue(IndexTy))
>>> }
>>>
>>> It's not that these are so complex that they can't be used on their own,
>>> but it would reduce duplicated code. But where would these methods go? On
>>> SValuator, which Ted's already suggested is more of an SValBuilder? Or on
>>> ValueManager, since the code makes use of its ArrayIndexTy? (Though we
>>> could add a getIndexType() method very easily.)
>>>
>>> Or are these not things that need to be packaged up? I feel like the
>>> bounds-checking would be useful, at least, since that /is/ something used
>>> in multiple places.
>>>
>>> Jordy
>>
>>
>