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 Option::unwrap unstably const #74956

Merged
merged 2 commits into from
Jul 31, 2020

Conversation

ecstatic-morse
Copy link
Contributor

This is lumped into the const_option feature gate (#67441), which enables a potpourri of Option methods.

cc @rust-lang/wg-const-eval

r? @oli-obk

`Result::unwrap` is not eligible becuase it formats the contents of the
`Err` variant. `unwrap_or`, `unwrap_or_else` and friends are not
eligible because they drop things or invoke closures.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 30, 2020
@ecstatic-morse
Copy link
Contributor Author

The error annotations in the test are a bit weird, but it's more of a problem for compiletest than for users.

@leonardo-m
Copy link

This is useful for:

const N: NonZeroU32 = NonZeroU32::new(10).unwrap();

@oli-obk
Copy link
Contributor

oli-obk commented Jul 31, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 31, 2020

📌 Commit 96c84ac 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 31, 2020
@@ -82,6 +82,7 @@
#![feature(const_fn_union)]
#![feature(const_generics)]
#![feature(const_option)]
#![feature(const_precise_live_drops)]
Copy link
Member

Choose a reason for hiding this comment

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

What is this? I'm in the const-eval GH group and I don't think I ever saw this or #73255 ever before.^^

const FOO: i32 = Some(42i32).unwrap();

// This causes an error, but it is attributed to the `panic` *inside* `Option::unwrap` (maybe due
// to `track_caller`?). A note points to the originating `const`.
Copy link
Member

Choose a reason for hiding this comment

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

If anything it is because const-eval panic reporting ignores track_caller.

@bors
Copy link
Contributor

bors commented Jul 31, 2020

⌛ Testing commit 96c84ac with merge 3a92b99...

@bors
Copy link
Contributor

bors commented Jul 31, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: oli-obk
Pushing 3a92b99 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jul 31, 2020
@bors bors merged commit 3a92b99 into rust-lang:master Jul 31, 2020
@leonardo-m
Copy link

This is still not allowed:

    const N: usize = 20;
    const NI: i32 = i32::try_from(N).unwrap();

@oli-obk
Copy link
Contributor

oli-obk commented Aug 1, 2020

The unwrap is not the issue here, it's try_from, which requires #67792

@oli-obk
Copy link
Contributor

oli-obk commented Aug 1, 2020

Once this PR lands in nightly you should be able to do nope, see below for details

#![feature(const_trait_impl, const_option)]
use std::convert::TryFrom;

struct A(usize);
struct B(i32);

impl const TryFrom<A> for B {
    type Error = &'static str;
    fn try_from(x: A) -> Result<Self, &'static str> {
        let converted = x.0 as i32;
        if x.0 == converted as usize {
            Ok(B(converted))
        } else {
            Err("no")
        }
    }
}

const N: usize = 20;
const NI: i32 = B::try_from(A(N)).unwrap().0;

EDIT: ah nevermind, this unwraps a Result, which requires a Debug impl, so this is way more complex. This is tracked under panic! with formatting in https://static.turbo.fish/const-eval-skill-tree/skill-tree.html

We can't use .ok() either, because that drops the error, so we need to have the Drop node in the skill tree

@ecstatic-morse ecstatic-morse deleted the const-option-unwrap branch October 6, 2020 01:42
@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
merged-by-bors This PR was explicitly merged by bors. 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.

7 participants