Skip to content

Permit const panics in stable const contexts in stdlib #90687

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

Merged
merged 1 commit into from
Nov 17, 2021

Conversation

jhpratt
Copy link
Member

@jhpratt jhpratt commented Nov 8, 2021

Without this change, it is not possible to use panic! and similar (including assert!) in stable const contexts inside of stdlib. See #89542 for a real-world case that currently fails for this reason. This does not affect any user code.

For example, this snippet currently fails to compile:

#[stable(feature = "foo", since = "1.0.0")]
#[rustc_const_stable(feature = "foo", since = "1.0.0")]
const fn foo() {
    assert!(false);
    assert!(false, "foo");
}

With the addition of #[rustc_const_unstable] to core::panicking::panic, the error no longer occurs. This snippet has been added verbatim in this PR as a UI test.

To avoid needing to add #![feature(core_panic)] to libcore, the two instances of direct calls to core::panicking::panic have been switched to use the panic! macro.

I am requesting prioritization because this is holding up other stabilizations such as #89542 (which is otherwise ready to merge and succeeds with this change)

@rustbot rustbot added A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-const-fn C-bug Category: This is a bug. I-prioritize Issue: Indicates that prioritization has been requested for this issue. T-lang Relevant to the language team labels Nov 8, 2021
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 8, 2021
@est31 est31 mentioned this pull request Nov 8, 2021
5 tasks
@nbdd0121
Copy link
Contributor

nbdd0121 commented Nov 8, 2021

I don't think this should or need to be backported.

@jhpratt jhpratt force-pushed the const_panic branch 2 times, most recently from a56c6e8 to c49d741 Compare November 8, 2021 22:05
@jhpratt
Copy link
Member Author

jhpratt commented Nov 8, 2021

As this only affects stdlib and I've confirmed that #89542 compiles after being rebased on this PR, I agree it doesn't need to be backported. I still think this would be nice to prioritize given that it's blocking other things, but feel free to remove the I-prioritize label if wanted. The actual change is all of one line — the rest is just testing and small idiomatic changes in Duration to avoid needing a feature flag. So this should be a trivial review for anyone.

@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Nov 9, 2021
@oli-obk
Copy link
Contributor

oli-obk commented Nov 17, 2021

r? @oli-obk

this is solely an internal change and it is definitely intended that we can use panic! from within libcore in stable const contexts

@bors r+

I don't think we should prioritize/backport things just to unblock stabilizing others. There is no rush to stabilize things, but I get how annoying it is when things are blocked.

@bors
Copy link
Collaborator

bors commented Nov 17, 2021

📌 Commit 6d2f8af has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 17, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2021
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#89610 (warn on must_use use on async fn's)
 - rust-lang#90667 (Improve diagnostics when a static lifetime is expected)
 - rust-lang#90687 (Permit const panics in stable const contexts in stdlib)
 - rust-lang#90772 (Add Vec::retain_mut)
 - rust-lang#90861 (Print escaped string if char literal has multiple characters, but only one printable character)
 - rust-lang#90884 (Fix span for non-satisfied trivial trait bounds)
 - rust-lang#90900 (Remove workaround for the forward progress handling in LLVM)
 - rust-lang#90901 (Improve ManuallyDrop suggestion)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@jhpratt
Copy link
Member Author

jhpratt commented Nov 17, 2021

Just to clarify w.r.t. backporting: don't. Other things don't need to wait for this to hit beta as I originally suspected.

@bors bors merged commit ec84633 into rust-lang:master Nov 17, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 17, 2021
@jhpratt jhpratt deleted the const_panic branch November 17, 2021 22:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-bug Category: This is a bug. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants