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? _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits