Skip to content

Put intrinsics::unreachable on a possible path to stabilization #43750

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 11, 2017
Merged

Put intrinsics::unreachable on a possible path to stabilization #43750

merged 3 commits into from
Aug 11, 2017

Conversation

tbu-
Copy link
Contributor

@tbu- tbu- commented Aug 8, 2017

Mark it with the unreachable feature and put it into the mem module.
This is a pretty straight-forward API that can already be simulated in
stable Rust by using transmute to create an uninhabited enum that can
be matched.

@tbu-
Put intrinsics::unreachable on a possible path to stabilization
315de9c 
Mark it with the `unreachable` feature and put it into the `mem` module.
This is a pretty straight-forward API that can already be simulated in
stable Rust by using `transmute` to create an uninhabited enum that can
be matched.
@rust-highfive
Copy link
Contributor

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@tbu-
Copy link
Contributor Author

tbu- commented Aug 8, 2017

I think this is pretty basic functionality for a systems programming language and was a bit surprised to see that it still wasn't stable or on the path to stabilization.

@sfackler
Copy link
Member

sfackler commented Aug 8, 2017

Seems reasonable to me! cc @rust-lang/libs.

Can you make a tracking issue for it and update the stability annotation?

@aturon
Copy link
Contributor

aturon commented Aug 8, 2017

SGTM

@tbu-
Copy link
Contributor Author

tbu- commented Aug 8, 2017

Tracking issue: #43751.

@sfackler
Copy link
Member

sfackler commented Aug 8, 2017

I don't think we need to do a full fcp merge or anything like that - this is still unstable, and it's a pretty straightforward thing to expose IMO.

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 8, 2017

📌 Commit 5704b07 has been approved by sfackler

@sfackler sfackler closed this Aug 8, 2017
@sfackler sfackler reopened this Aug 8, 2017
@notriddle
Copy link
Contributor

@sfackler Don't you need to re-r+ this?

@sfackler
Copy link
Member

sfackler commented Aug 9, 2017

dunno

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 9, 2017

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Collaborator

bors commented Aug 9, 2017

📌 Commit 5704b07 has been approved by sfackler

oli-obk
oli-obk reviewed Aug 9, 2017
View reviewed changes
src/libcore/mem.rs Outdated
/// macro, which panics when it is executed, it is *undefined behavior* to
/// reach code marked with this function.
#[unstable(feature = "unreachable", issue = "43751")]
pub unsafe fn unreachable() -> ! {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be #[inline] in order to allow optimizations to optimize it outside libcore?

@aidanhs aidanhs added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 10, 2017
@alexcrichton
Copy link
Member

@bors: r-

I think @oli-obk is right in that if this isn't #[inline] it's usefulness for optimization may be greatly reduced.

@alexcrichton alexcrichton added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 10, 2017
@tbu-
Copy link
Contributor Author

tbu- commented Aug 11, 2017

@alexcrichton Fixed.

@alexcrichton
Copy link
Member

@bors: r=sfackler

@bors
Copy link
Collaborator

bors commented Aug 11, 2017

📌 Commit faadd35 has been approved by sfackler

@bors
Copy link
Collaborator

bors commented Aug 11, 2017

⌛ Testing commit faadd35 with merge a7e0d3a...

bors added a commit that referenced this pull request Aug 11, 2017
@bors
Auto merge of #43750 - tbu-:pr_fn_unreachable, r=sfackler
a7e0d3a 
Put `intrinsics::unreachable` on a possible path to stabilization

Mark it with the `unreachable` feature and put it into the `mem` module.
This is a pretty straight-forward API that can already be simulated in
stable Rust by using `transmute` to create an uninhabited enum that can
be matched.
@bors
Copy link
Collaborator

bors commented Aug 11, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: sfackler
Pushing a7e0d3a to master...

@bors bors merged commit faadd35 into rust-lang:master Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oli-obk oli-obk oli-obk left review comments

Assignees

@sfackler sfackler

Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@tbu- @rust-highfive @sfackler @aturon @bors @notriddle @alexcrichton @oli-obk @aidanhs