-
Notifications
You must be signed in to change notification settings - Fork 12.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sign extend constants in range patterns #49949
Conversation
@@ -1096,7 +1101,11 @@ pub fn compare_const_vals(a: &ConstVal, b: &ConstVal, ty: Ty) -> Option<Ordering | |||
// FIXME(oli-obk): report cmp errors? | |||
l.try_cmp(r).ok() | |||
}, | |||
ty::TyInt(_) => Some((a as i128).cmp(&(b as i128))), | |||
ty::TyInt(_) => { | |||
let a = interpret::sign_extend(tcx, a, ty).expect("layout error for TyInt"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the only actual change in this PR. Everything else is just moving things around.
The issue was that we represent constants in their native bit width, so casting to i128 didn't work out for e.g. -1u8
, which is 0xFF
, and stays that when casting to i128
(meaning it would be 255 instead of -1)
These changes look good to me, but I'd like to r? @eddyb |
@bors r+ |
📌 Commit 45529b5 has been approved by |
@bors p=4 |
⌛ Testing commit 45529b5357cbaa1ac90893547566cd534c6a41a4 with merge 87eeb04b1f0e8b104fe801966ba0653a134e0056... |
💔 Test failed - status-appveyor |
Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem. Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
// must-compile-successfully |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been renamed to // compile-pass
on master (it is still // must-compile-successfully
on beta).
45529b5
to
b22c9c0
Compare
@bors r=eddyb |
📌 Commit b22c9c0 has been approved by |
Sign extend constants in range patterns fixes #49940 r? @Mark-Simulacrum
☀️ Test successful - status-appveyor, status-travis |
Backported in #50027 |
fixes #49940
r? @Mark-Simulacrum