-
Notifications
You must be signed in to change notification settings - Fork 13.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
promotion: don't promote int::MIN / -1 #121515
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt |
@bors try |
promotion: don't promote int::MIN / -1 Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail... Cc rust-lang#80619 This is a breaking change, so needs a crater run. r? `@oli-obk`
☀️ Try build successful - checks-actions |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
why is it a breaking change? it was hard erroring in const eval before... |
Hm... I was sure this would not error, but somehow it does: fn main() {
if false {
let _val = &(i32::MIN/-1);
}
} Are we adding promoteds to required-consts? But we can't be doing that for all promoteds because of the const-fn-call-promoted-in-const thing. Not quite sure what is happening here. |
Lol, looks like GVN triggers evaluation. It's good that we can do that if promoteds are truly infallible, but it means we better never run GVN on But I guess it does indeed mean that this is not a breaking change. In fact it unbreaks the case above where the division is in dead code. @craterbot abort |
🗑️ Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
Yea I'd expect the other direction to be a breaking change. Even without GVN I'd assume we eval these promoteds at some point 😅 |
8fc2d2b
to
f32095c
Compare
@bors r+ |
🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened. |
…-obk promotion: don't promote int::MIN / -1 Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail... Cc rust-lang#80619 This is a breaking change, so needs a crater run. r? `@oli-obk`
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#121343 (Add examples for some methods on slices) - rust-lang#121374 (match lowering: Split off `test_candidates` into several functions and improve comments) - rust-lang#121474 (Ignore compiletest test directive migration commits) - rust-lang#121515 (promotion: don't promote int::MIN / -1) - rust-lang#121530 (Fix incorrect doc of ScopedJoinHandle::is_finished) - rust-lang#121551 (Forbid use of `extern "C-unwind"` inside standard library) - rust-lang#121556 (Use `addr_of!`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#121515 - RalfJung:fallible-promotion, r=oli-obk promotion: don't promote int::MIN / -1 Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail... Cc rust-lang#80619 This is a breaking change, so needs a crater run. r? ``@oli-obk``
Looks like I entirely forgot about this case when adding the div-by-zero check, which was supposed to ensure that we never promote operations that can fail...
Cc #80619
This is a breaking change, so needs a crater run.
r? @oli-obk