[cfe-commits] [patch] Tracking simple arithmetic constraints (PR2695)

[Zhongxing Xu]

On Thu, Jun 17, 2010 at 8:54 AM, Ted Kremenek <kremenek at apple.com> wrote:

> On Jun 16, 2010, at 5:37 PM, Jordy Rose wrote:
>
> > What I'm worried about is that this is going to be overkill for something
> > that may never grow beyond additive operations. That is,
> > RangeConstraintManager may not be the best way to handle SymSymExprs, or
> > even SymIntExprs with other operations in them. (The solution space for
> > "x%2 == 0" would have O(MAX) ranges. And a naive transformation for "x/2
> >=
> > 1" would take the range [1, MAX] and turn it into [2, MAX*2].) I suppose
> > right now a Transformer would just be a stack object made of a
> > BinaryOperator::Opcode and an APSInt value, while a later one could
> compose
> > operations. Even so, I think it may be a bit of a YAGNI case.
>
> This is a fair point.  There is some simplicity in the current approach,
> although it isn't algorithmically scalable.  Perhaps this is the "limit" of
> "simple" for the the current SimpleConstraintManagers.
>
> >
> > Well. I don't think it would be hard to alter the current code to use a
> > Transformer -- just change all "X - Adjustment" expressions to
> > "T.transform(X)". If you think it's worth it, I can go ahead and do that.
> > (My schedule is currently a little spotty cause I'm on vacation, so I may
> > not actually have that changed until Sunday.) What it won't do is remove
> > any complexity in the disparate AssumeSym*() methods (though allowing
> > wraparound ranges would). But it does allow for more possible expansion
> in
> > the future.
>
> I think we should adopt the change as is, and then refactor with a
> transformer later if we try to further extend the SimpleConstraintManagers.
>  It will also give us an opportunity to think about this some more, and
> understand the design for either a new and more sophisticated
> ConstraintManager or ways to further extend the current ones.
>
> Zhongxing: what do you think?


I agree this patch should go in. It's great job. I haven't verified the
trick in  RangeConstraintManager::AssumeSymLT. It's not straightforward to
get it. But I believe it's right. Maybe some comments are helpful.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100617/87e2fca5/attachment.html>