-
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
Unconditionally show update nightly hint on ICE #122200
Unconditionally show update nightly hint on ICE #122200
Conversation
This comment has been minimized.
This comment has been minimized.
25086d6
to
a1cb4c2
Compare
This comment has been minimized.
This comment has been minimized.
a1cb4c2
to
1a9606b
Compare
@rustbot ready |
We should link this PR with #122196 if I am not mistaken? |
Unfortunately there's 2 separate bugs here:
|
I'm lacking context about this hint. |
same 😅 |
@@ -19,5 +19,7 @@ LL | same_output(foo, rpit); | |||
LL | same_output(foo, rpit); | |||
| ^^^^^^^^^^^^^^^^^^^^^^ | |||
|
|||
note: if you are using a nightly compiler, please make sure that you have updated to the latest nightly |
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.
Is there any point in having "if you are using a nightly compiler"? Why not check tcx.sess.is_nightly_build()
directly?
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.
I thought install_ice_hook and report_ice were so early in rustc_driver_impl that we didn't have a tcx available?
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.
I believe you can use rustc_feature::UnstableFeatures::from_environment(None).is_nightly_build();
for this instead.
I feel like we would want to unconditionally suggest updating for nightly releases. On stable I am of many minds. We could do a check for current date and try to figure out how long ago the current release went out, and if >6 weeks, tell the user to consider updating. But that wouldn't take into account dot-releases to fix that exact ICE being released. At the same time, I would expect people to already think "maybe there's a newer version that fixes this". On stable we also need to account for non-rustup distributed rustc, like from Linux distro package managers, which would likely not want such a message, or to customize it... For now, can you try to check for the current build release channel and remove the uncertain language from the message? |
Hmm, wouldn't this break ICE tests checking for the exact ICE message if the compiler's channel is |
☔ The latest upstream changes (presumably #123628) made this pull request unmergeable. Please resolve the merge conflicts. |
1a9606b
to
ec2a36b
Compare
@estebank sorry for the ping, I want to unbork cg_clif's tests. Not sure what to do here, maybe we just emit the update nightly note unconditionally, or revert the update note altogether? |
@jieyouxu let's check if |
Thanks, I'll try that |
ec2a36b
to
5d8b8b8
Compare
☔ The latest upstream changes (presumably #122077) made this pull request unmergeable. Please resolve the merge conflicts. |
5d8b8b8
to
09dab38
Compare
@bors r+ |
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#121884 (Port exit-code run-make test to use rust) - rust-lang#122200 (Unconditionally show update nightly hint on ICE) - rust-lang#123568 (Clean up tests/ui by removing `does-nothing.rs`) - rust-lang#123609 (Don't use bytepos offsets when computing semicolon span for removal) - rust-lang#123612 (Set target-abi module flag for RISC-V targets) - rust-lang#123633 (Store all args in the unsupported Command implementation) - rust-lang#123668 (async closure coroutine by move body MirPass refactoring) Failed merges: - rust-lang#123701 (Only assert for child/parent projection compatibility AFTER checking that theyre coming from the same place) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#122200 - jieyouxu:unconditional-nightly-update-hint, r=estebank Unconditionally show update nightly hint on ICE Instead of trying to guess if a update nightly hint should be shown (by checking for system time, querying version and channel info etc.), just show the update nightly hint for nightly compilers. This avoids breaking tests that match on ICE test outputs on nightly/dev channels. > Another issue is that the outdated nightly hint triggers for ICE tests, causing a mismatch with the test expectation. There doesn't seem to be any env var to suppress this. See <https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/stage0.20compiletest.20broken/near/425543681> for context.
Instead of trying to guess if a update nightly hint should be shown (by checking for system time, querying version and channel info etc.), just show the update nightly hint for nightly compilers. This avoids breaking tests that match on ICE test outputs on nightly/dev channels.
See https://rust-lang.zulipchat.com/#narrow/stream/326414-t-infra.2Fbootstrap/topic/stage0.20compiletest.20broken/near/425543681 for context.