On Thu, Jun 17, 2010 at 8:54 AM, Ted Kremenek <kreme...@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.
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
