Skip to content
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

Make unreachable_unchecked a const fn #74459

Merged
merged 4 commits into from
Jul 19, 2020

Conversation

canova
Copy link
Contributor

@canova canova commented Jul 17, 2020

This PR makes std::hint::unreachable_unchecked a const fn so we can use it inside a const function.
r? @RalfJung
Fixes #53188.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 17, 2020
error: any use of this value will cause an error
--> $SRC_DIR/libcore/hint.rs:LL:COL
|
LL | unsafe { intrinsics::unreachable() }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error is showing the internal code instead of the const_unsafe_unreachable_ub.rs source code, but I'm not sure how to make it show the source code instead. We still have the source code mentioned in the stack below though. Do you think this is a blocker?

@@ -0,0 +1,15 @@
#![feature(const_fn)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you make this test #[warn(const_err)] you'll get an additional error on the BAR use site, which you can then mark with //~ ERROR erroneous constant

Copy link
Contributor Author

@canova canova Jul 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to add #[warn(const_err)] on top of the BAR constant but I still don't get an error in the use site.

Edit: Okay, I figured out. I was missing the build-fail annotation actually. Updating the code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oli-obk Thanks for the recommendation! I figured out and updated the code. Could you take a look again?
(Added it as a new commit to make it easier to see new changes, but I can squash with the previous commit later if you prefer it that way.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, that build-fail thing is really problematic around const prop and similar. I gotta figure something out for that.

The PR lgtm now, thanks!

@oli-obk
Copy link
Contributor

oli-obk commented Jul 18, 2020

@bors r+

Can you also open a PR against github.com/rust-lang/miri to remove the code for the unreachable intrinsic from it?

@bors
Copy link
Contributor

bors commented Jul 18, 2020

📌 Commit 6cd164f 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 Jul 18, 2020
@canova
Copy link
Contributor Author

canova commented Jul 18, 2020

Thanks for the review! Sure, I'll create a PR on miri as well.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2020
…arth

Rollup of 7 pull requests

Successful merges:

 - rust-lang#70817 (Add core::task::ready! macro)
 - rust-lang#73762 (Document the trait keyword)
 - rust-lang#74021 (impl Index<RangeFrom> for CStr)
 - rust-lang#74071 (rustc_metadata: Make crate loading fully speculative)
 - rust-lang#74445 (add test for rust-lang#62878)
 - rust-lang#74459 (Make unreachable_unchecked a const fn)
 - rust-lang#74478 (Revert "Use an UTF-8 locale for the linker.")

Failed merges:

r? @ghost
@bors bors merged commit 2fad396 into rust-lang:master Jul 19, 2020
@canova canova deleted the const-unreachable-unchecked branch July 19, 2020 10:17
bors added a commit to rust-lang/miri that referenced this pull request Jul 23, 2020
Remove unreachable intrinsic

This is now supported by the interpreter with rust-lang/rust#74459. We can remove this intrinsic implementation here.
This is covered by this test: https://github.com/rust-lang/miri/blob/master/tests/compile-fail/unreachable.rs

I guess we need to wait until the rust-lang/rust PR merges into nightly, and then we can update `rust-version` hash in the PR, right?

r? @oli-obk
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking Issue for const unreachable_unchecked
6 participants