Analyzer: SVal-building idioms

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

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.

I think these constructions are more appropriate to put into
ValueManager, since ValueManager constructs SVals, but SValuator
perform evaluations.

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.

Right. As "evaluation" becomes more lazy (e.g., by "assuming" SVals as constraints) the distinction between an SVal representing a "value" and one representing an "evaluation of an expression" should intentionally become blurred. In truth no SVal really becomes evaluated until either a constrained is assumed by the ConstraintManager or a load/store is made by the StoreManager.