NoQ added a comment. Ok, code makes sense to me now!
I think we still need a few new tests to cover the corner cases. In https://reviews.llvm.org/D35110#1135306, @baloghadamsoftware wrote: > I added extra assertion into the test for the difference. Interestingly, it > also works if I assert `n-m` is in the range instead of `m-n`. However, I > wonder how can I apply such constraint to the difference of iterator > positions without decomposing them to symbols and constants. I don't see how iterator checker is different from the tests. The code of the program in your tests doesn't decompose `m - n` into symbols and constants, it simply subtracts an opaque value `n` (whatever it is) from an opaque value `m` (whatever it is) and makes assumptions on the opaque result of the subtraction (whatever it is). The checker should do the same, i guess? ================ Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:177-178 +// Turn all [A, B] ranges to [-B, -A]. Turn minimal signed value to maximal +// signed value. +RangeSet RangeSet::Negate(BasicValueFactory &BV, Factory &F) const { ---------------- I guess the comment needs to be updated. ================ Comment at: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:189 + newRanges.begin()->From().isMinSignedValue()) { + const llvm::APSInt &newFrom = newRanges.begin()->From(); + newRanges = ---------------- I suggest a few sanity checks here (untested): `assert(newRanges.begin()->To().isMinSignedValue());` because we shouldn't ever get an overlap. `assert(!from.isMinSignedValue())` for the same reason; it's good to know because it tells us what `newTo` is equal to on this path. https://reviews.llvm.org/D35110 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
