-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
ICE: enum value has invalid tag, no-core-2.rs -Zmir-opt-level=3 --emit=mir -Zdump-mir=all #93688
Comments
For the record, it is due to fn bar() -> Option<()> {
None?
} I've tracked this down (possibly) to |
So actually this doesn't even depend on minimized (ICEs with fn bar() {
match Option::<Option<()>>::None {
Some(v) => {}
None => {}
}
} |
Yeah so it just seems like we're creating invalid scalar values as intermediates in optimization passes (e.g. after const propagation), and then panicking when we pretty-print those invalid values when dumping MIR. I can give a more detailed explanation why we're creating an invalid scalar, but that scalar is being optimized out anyways in subsequent optimization passes, so I don't think this would ever affect codegen... So not sure how scary this ICE is in practice. I would claim this issue, but I don't know what the best fix for this would be. Perhaps making Not sure who to ask -- @RalfJung or @oli-obk, perhaps either of y'all have an opinion on this? reason why we're creating an invalid scalar
|
An optimization pass that introduces invalid scalars literally introduces UB into well-defined code (unless this is in dead code), so -- I would say this is rather scary, on the level of "I-unsound". |
Fairly certain this only happens in unreachable branches (default branches of exhaustive switches), but i'll dig into it. |
Sorry, yes, it only seems to happen (as far as I know) in dead branches. The issue here only seems to be printing MIR with these dead branches still in the code. |
Okay that is less critical then. Arguably printing should work even for invalid code. It still seems interesting to figure out why this happens. I would understand if this was an uninhabited variant, but seeing an entirely non-existing discriminant is surprising. |
I left an explanation above in an collapsible block as to why we're creating this invalid value. I believe the root cause is that we perform operations in const propagation infallibly. It would probably be desirable to prevent this by making const propagation fallible if the const is incompatible with the layout of the result of the operation being applied to it. Then when we apply something like |
Okay so it looks like possibly we should adjust the "downcast" projection(s) to verify that the place has the right variant, and |
@RalfJung yup that sounds correct. do you want me to make that change? |
One thing I am wondering about is if this will work correctly for generators -- the have more complicated variants than enums do, and maybe there are situations there where projections to the "wrong" variant are actually okay? Cc @tmandry |
@RalfJung are you proposing to declare downcast to an inactive variant to be undefined behaviour? This is probably larger change, in a sense that it would be incompatible with how downcast is currently used:
|
Yes, basically. But it seems like that plan won't work. Interesting. I should probably add a comment in the code explaining why this is not currently considered UB. Well then it seems the best we can do for this ICE is to make the printing code more robust. It is anyway a good idea to be able to print invalid values; otherwise, debugging invalid values can be quite frustrating. |
Support pretty printing of invalid constants Make it possible to pretty print invalid constants by introducing a fallible variant of `destructure_const` and falling back to debug formatting when it fails. Closes rust-lang#93688.
Code
./src/test/ui/no-core-2.rs
Meta
rustc --version --verbose
:Error output
rustc ./src/test/ui/no-core-2.rs -Zmir-opt-level=3 --emit=mir -Zdump-mir=all --edition 2018
Backtrace
The text was updated successfully, but these errors were encountered: