Fix MIR check for MIN % -1 in exact_div#69138
Closed
elichai wants to merge 1 commit intorust-lang:masterfrom
Closed
Fix MIR check for MIN % -1 in exact_div#69138elichai wants to merge 1 commit intorust-lang:masterfrom
elichai wants to merge 1 commit intorust-lang:masterfrom
Conversation
Member
|
Thanks a lot for helping to fix Miri! However, I'm afraid I already took care of this one... #69126
The old behavior was wrong, though. |
Contributor
Author
|
@RalfJung Ha. That PR wasn't there when I started diving into rustc_mir 😉
I think I don't quite understand what the mir::interpret is (ie when exactly does it run and on what) but maybe we can take this convo offline :) |
Member
|
I suggest https://rust-lang.github.io/rustc-guide/const-eval.html (and its sub-section) to start with :) |
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.
Hi,
I hope I understood the bug correctly, because I'm not too familiar with the code here.
I've found this while playing with Miri, it seems like this if condition is no longer executed: https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/intrinsics.rs#L414
Because this commit 28f85c6 made it return
0in theMIN % -1case, while the if explicitly tests if it's not 0So I reverted to the old behavior which returned the left side (
MIN) instead of 0.r? @RalfJung
FWIW, Before this the
exact_div4test in Miri failed and after this it passes