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

panic when calling MaybeUninhabited::into_inner on uninhabited type #57145

Merged
merged 2 commits into from
Jan 5, 2019

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Dec 27, 2018

I do this by adding an internal-only intrinsic panic_if_uninhabited. I have no idea what I am doing here, just mindlessly copying code around, so please review carefully!

@rust-highfive
Copy link
Collaborator

r? @zackmdavis

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 27, 2018
@zackmdavis
Copy link
Member

I have no idea what I am doing here, just mindlessly copying code around, so please review carefully!

I fear I don't have domain expertise here, either. 😰 @oli-obk, since you already commented, are you able to take over the review?

r? @oli-obk

@rust-highfive rust-highfive assigned oli-obk and unassigned zackmdavis Dec 30, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Dec 30, 2018

So... I am confident that the impl does what it promises. I just don't know if I can single handedly sign off on this.

I think it would be enough for @rust-lang/libs to sign off on this, although it kind of touches language and compiler.

@oli-obk oli-obk added I-nominated T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Dec 30, 2018
@alexcrichton
Copy link
Member

@bors: try

@bors
Copy link
Contributor

bors commented Jan 2, 2019

⌛ Trying commit c118b17 with merge c8bd38c4de0d52227df9d7c04afd10f3080e9a6a...

@alexcrichton
Copy link
Member

Er sorry, just now realizing that the implementation already does this, we're just adding something else under that umbrella. Sounds good to me!

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 2, 2019

📌 Commit c118b17 has been approved by alexcrichton

@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 Jan 2, 2019
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jan 3, 2019
…excrichton

panic when calling MaybeUninhabited::into_inner on uninhabited type

I do this by adding an internal-only intrinsic `panic_if_uninhabited`. I have no idea what I am doing here, just mindlessly copying code around, so please review carefully!
@bors
Copy link
Contributor

bors commented Jan 5, 2019

⌛ Testing commit c118b17 with merge 8c2d0f4...

bors added a commit that referenced this pull request Jan 5, 2019
panic when calling MaybeUninhabited::into_inner on uninhabited type

I do this by adding an internal-only intrinsic `panic_if_uninhabited`. I have no idea what I am doing here, just mindlessly copying code around, so please review carefully!
@bors
Copy link
Contributor

bors commented Jan 5, 2019

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing 8c2d0f4 to master...

@bors bors merged commit c118b17 into rust-lang:master Jan 5, 2019
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #57145!

Tested on commit 8c2d0f4.
Direct link to PR: #57145

💔 miri on linux: test-pass → test-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Jan 5, 2019
Tested on commit rust-lang/rust@8c2d0f4.
Direct link to PR: <rust-lang/rust#57145>

💔 miri on linux: test-pass → test-fail (cc @oli-obk @RalfJung @eddyb, @rust-lang/infra).
@RalfJung RalfJung deleted the panic-if-uninhabited branch January 9, 2019 15:04
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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants