Fix bounds ordering for captured Range values in Predicate to NSPredicate conversion #1640
Conversation
| // Verify that if bounds were swapped, the predicate would be wrong | ||
| // (This would be i >= upperDate AND i < lowerDate, which is always false) | ||
| let wrongPredicate = NSPredicate(format: "i >= %@ AND i < %@", upperDate as NSDate, lowerDate as NSDate) | ||
| #expect(!wrongPredicate.evaluate(with: objInRange), "Swapped bounds would incorrectly reject valid date") |
There was a problem hiding this comment.
What benefit did you find with adding this additional test? Is it testing Predicate to NSPredicate conversion in some way?
There was a problem hiding this comment.
It proves the ordering of bounds matters.
There was a problem hiding this comment.
Can you elaborate on what you mean by that? I don't fully understand what this is proving here regarding the conversion code that you updated. It doesn't look like this code goes through the Predicate/NSPredicate conversion code but instead looks like it's just testing NSPredicate itself
There was a problem hiding this comment.
It exercises the _RangeValue conversion path for captured ranges.
There was a problem hiding this comment.
It checks that when extracting bounds from a captured Range (via range.lowerBound and range.upperBound at line 326), they are used in the correct order when building the NSPredicate (lower for >=, upper for <).
There was a problem hiding this comment.
But yeah the last two parts are redundant.
AZero13
force-pushed
the
swap
branch
2 times, most recently
from
556e32b to
c7fbf09
Compare
AZero13
changed the title
…cate conversion We seem to be passing upper to lower and lower to upper when converting. That does not seem right.
|
@swift-ci please test |
2 participants
We seem to be passing upper to lower and lower to upper when converting. That does not seem right.