ConstraintAnalysis: Fix mixed signed/unsigned comparisons - #9045
Merged
Conversation
tlively
approved these changes
Aug 26, 2026
| } | ||
|
|
||
| // Initialize with pairs of coordinates. | ||
| Spans(std::initializer_list<T> init) { |
Member
There was a problem hiding this comment.
Using nested initializer lists might be more readable, e.g. {{a, b}, {x, y}}. Then this could probably be std::initializer_list<Span>.
Member
Author
There was a problem hiding this comment.
Good idea, and actually it turns out this "just works", c++ infers {a, b} must be a Span already. So we can just delete this constructor and rewrite the code. Done.
kripken
enabled auto-merge (squash)
August 26, 2026 21:56
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The IU64 idea was nice in theory - a single representation that includes both
signed and unsigned values - and it makes both of those comparisons work
in a simple and intuitive way, but it fails on mixed comparisons. When you have
x > Csigned, andx < Dunsigned, IU64 represents e.g. the number-1twice, once signed and once unsigned. But what matters is the bit-pattern, in
the end, so we really need to represent it once.
To handle that, bite the bullet and implement Spans, a set of two Span
instances. An unsigned range is then a single Span, and a signed one,
fitted into the unsigned space, sometimes needs two. For example,
x <= 100signed turns into
[0..100]and also[0x80000000, 0xffffffff], that is, smallpositive numbers and also all negative numbers (and negative numbers, in
the unsigned space, are all those with the top bit set).
understand if you skim it (I did go over it carefully + fuzzing + AI).