-
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
help with a Miri ICE #100918
help with a Miri ICE #100918
Conversation
r? @fee1-dead (rust-highfive has picked a reviewer for you, use r? to override) |
@bors try |
⌛ Trying commit 3371a75 with merge 0645e44b518e76c93ea453bf06956214037a6532... |
ui tests are indeed not happy. No idea if those changes are any good or not. |
☀️ Try build successful - checks-actions |
At least I can confirm that this fixes the ICE in Miri using a rustc release build. There's still the failing debug assertion, of course. |
The job Click to see the possible cause of the failure (guessed by this bot)
|
Although I have an idea why this change makes miri build without ICE, I don't know why debug assertions cause problems. |
@ouz-a I am confused by the question. A debug assertion causes problems when it does not hold? That's completely expected. Regular release builds of Miri have debug assertions disabled, that's why we did not notice when this started. |
Ahh okay, this shouldn't be hard to fix then I will try to push it tonight. |
Superseded by #100967 |
This helps with rust-lang/miri#2433, though the example there still ICEs with debug assertions enabled. Due to that, I am not sure what kind of test we could add. Cc @ouz-a @oli-obk
This effectively reverts a behavior change in #99383 that was noticed by @bjorn3 ; I have no clue if that behavior change was intentional or not. I guess the ui test suite will tell us.
Note that I have no clue whatsoever what this change actually does. I don't even understand the words used in this part of the code.