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

unwrap!-everything #127

Closed
SoniEx2 opened this issue Oct 28, 2022 · 14 comments
Closed

unwrap!-everything #127

SoniEx2 opened this issue Oct 28, 2022 · 14 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@SoniEx2
Copy link

SoniEx2 commented Oct 28, 2022

Proposal

Problem statement

Maybe you wanna unwrap a Result without Debug, or a custom type that doesn't even represent an error but maybe represents a state machine and you wanna assume you're in a specific state and grab the values inside of it. The unwrap!-everything API would allow exactly that.

Motivation, use-cases

We wrote this like 5 times before making it a function:

    fn key_subtree(&self) -> Option<usize> {
        if let PatternElement::Tag { key_subtree } = self.op() {
            key_subtree
        } else {
            unreachable!()
        }
    }

And we have... multiple places where we use this exact pattern.

Solution sketches

We'd like to see some sort of unwrap! macro which basically asserts the expectation and returns the relevant results:

let x = unwrap!(Err(e) = result);
let y = unwrap!(Ok(v) = no_debug_result);
let z = unwrap!(PatternElement::Tag { key_subtree } = frame.op());
let w = unwrap!([a, ..] = my_slice);
unwrap!(None = my_option);

Tho we acknowledge we haven't thought about how this would interact with multiple results like so:

let _ = unwrap!([a, b, ..] = my_slice);

Maybe it should look more like this instead?

let x = unwrap!(Err(use) = result);
let y = unwrap!(Ok(use) = no_debug_result);
let z = unwrap!(PatternElement::Tag { key_subtree: use } = frame.op()); // actually wait is it field: pattern or pattern: field?
let w = unwrap!([use, ..] = my_slice);
unwrap!(None = my_option);
unwrap!(let [a, b, ..] = my_slice); // ?

Links and related work

https://doc.rust-lang.org/std/index.html?search=unwrap

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

@SoniEx2 SoniEx2 added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Oct 28, 2022
@thomcc
Copy link
Member

thomcc commented Oct 28, 2022

The new let...else feature, stabilizing in the next release of Rust (in 5 days), allows

let PatternElement::Tag { key_subtree } = frame.op() else {
    unreachable!()
};
// use `key_subtree`...

which should reduce the need for something like this a lot.

@truppelito
Copy link

truppelito commented Oct 28, 2022

There's also the crate enum-as-inner which I believe would allow (for enums):

let key_subtree = frame.op().into_tag().unwrap()

(or something similar, my apologies if this is not 100% correct, I'm not looking in detail at the docs for the crate right now)

@SoniEx2
Copy link
Author

SoniEx2 commented Oct 28, 2022

do any of those produce the nice panic messages that unwrap! would produce?

(which, fwiw, is better than what we have with unreachable!(). it'd actually be closer to those produced by assert!.)

@truppelito
Copy link

truppelito commented Oct 28, 2022

What exactly would unwrap! produce? Using let else the error message can be defined by what is placed in the else block. With the enum-as-inner, we can use frame.op().into_tag().expect("message"), although I will concede that both options are more verbose.

@SoniEx2
Copy link
Author

SoniEx2 commented Oct 28, 2022

using assert! (or assert_eq!) always provides a descriptive error message stating what went wrong.

while we don't exactly want the value after the = to be Debug, just seeing "this is the code that failed" (similar to an assert!) instead of "here's the line number that failed" would help a lot.

@cuviper
Copy link
Member

cuviper commented Oct 28, 2022

Something like this?

macro_rules! unwrap {
    ($pattern:pat_param = $expression:expr) => {
        let $pattern = $expression else {
            panic!("unwrap failed: {}", stringify!($pattern = $expression));
        };
    }
}

Silly example (playground):

fn main() {
    let mut opt = Some(10);
    loop {
        unwrap!(Some(x @ 2..) = opt);
        dbg!(x);
        let y = if x % 2 == 0 { x / 2 } else { 3 * x + 1 };
        opt = Some(y);
    }
}
[src/main.rs:13] x = 10
[src/main.rs:13] x = 5
[src/main.rs:13] x = 16
[src/main.rs:13] x = 8
[src/main.rs:13] x = 4
[src/main.rs:13] x = 2
thread 'main' panicked at 'unwrap failed: Some(x @ 2..) = opt', src/main.rs:12:9

@SoniEx2
Copy link
Author

SoniEx2 commented Oct 28, 2022

yes, tho we'd also like expression/value functionality (tho that's way harder ofc).

@cuviper
Copy link
Member

cuviper commented Oct 28, 2022

How do you expect to see the value if you don't have Debug?

@SoniEx2
Copy link
Author

SoniEx2 commented Oct 28, 2022

no like foo(unwrap!(...)) or unwrap!(...).bar()

@cuviper
Copy link
Member

cuviper commented Oct 28, 2022

Ah -- I think we'd need a compiler builtin to extract arbitrary bindings from a pattern.

@SoniEx2
Copy link
Author

SoniEx2 commented Oct 30, 2022

Additional benefit of unwrap!-everything, is when side-effects and bools are involved:

assert!(self.interp.frames[target].prev()); // bad
self.interp.frames[target].prev().then(|| ()).unwrap(); // awkward but okay
unwrap!(true = self.interp.frames[target].prev()); // perfect

@dead-claudia
Copy link

dead-claudia commented Apr 6, 2023

How about just throwing most the unwrapping methods on Try much like how Iterator has a number of helper methods of its own?

trait Try: FromResidual<Self::Residual> {
    // ...

    fn unwrap(self) -> Self::Output {
        // Ideally, this would use some compiler magic to wire in the type name instead.
        match self.branch() {
            ControlFlow::Break(_) => panic!("called `Try::unwrap()` on a residual value"),
            ControlFlow::Continue(result) => result,
        }
    }

    fn unwrap_or_default(self) -> Self::Output where Self::Output: Default {
        match self.branch() {
            ControlFlow::Break(_) => Self::Wrapped::default(),
            ControlFlow::Continue(result) => result,
        }
    }

    fn unwrap_or(self, default: Self::Output) -> Self::Output {
        match self.branch() {
            ControlFlow::Break(_) => default,
            ControlFlow::Continue(result) => result,
        }
    }

    unsafe fn unwrap_unchecked(self) -> Self::Output {
        match self.branch() {
            ControlFlow::Break(_) => std::hint::unreachable_unchecked(),
            ControlFlow::Continue(result) => result,
        }
    }

    fn expect(self, msg: &str) -> Self::Output {
        match self.branch() {
            ControlFlow::Break(_) => panic!("{}", msg),
            ControlFlow::Continue(result) => result,
        }
    }

    fn map_or<U>(self, default: U, f: FnOnce(Self::Output) -> U) -> U {
        match self.branch() {
            ControlFlow::Break(_) => default,
            ControlFlow::Continue(result) => f(result),
        }
    }

    fn map_or_default<U: Default>(self, f: FnOnce(Self::Output) -> U) -> U {
        match self.branch() {
            ControlFlow::Break(_) => U::default(),
            ControlFlow::Continue(result) => f(result),
        }
    }
}

impl<T> Try for Option<T> {
    // ...

    fn unwrap(self) -> Self::Output {
        self.expect("called `Option::unwrap()` on a `None` value")
    }
}

impl<T, E> Try for Result<T, E> {
    // ...

    fn unwrap(self) -> Self::Output {
        self.expect("called `Result::unwrap()` on an `Err` value")
    }

    fn expect(self, msg: &str) -> Self::Output {
        // Do this for `E: fmt::Debug` and just the default for other types
        // match self {
        //     Err(e) => panic!("{}: {}", msg, e),
        //     Ok(result) => result,
        // }
        SpecExpect::expect(self, msg)
    }
}

This would merge large chunk of Result and Option's APIs and would also allow other Try impls to gain it for free.

This does carry a major con, though: it'd make rust-lang/rust#67441 depend on traits supporting const fns (in some capacity).

@SoniEx2
Copy link
Author

SoniEx2 commented Apr 9, 2023

Try doesn't apply to arbitrary enums, let alone arbitrary patterns. The use-case of unwrap! is not Try-like enums; rather the opposite, it's for when you make assumptions about plain data enums (sum types), and slices.

Op Returns a value Arbitrary patterns Panics
Try ✔️ ⚠️
unwrap! ✔️ ✔️ ✔️
let-else ✔️

@the8472
Copy link
Member

the8472 commented May 16, 2023

We discussed this in a libs meeting. While we do want to make traversal of data structures and patternmatching less cumbersome we have several blocking concerns about this particular proposal and thus are going to close it.

  • We don't want to encourage uses of unwrap
  • This is already served by let-else or if-let. If you have ideas how to improve ergonomics of let-else you could bring that up in a separate proposal
  • The solution sketches lack clarity, this suggests that further discussion (e.g. in internals.rlo or zulip) is needed before bringing such a proposal to the libs team

@the8472 the8472 closed this as completed May 16, 2023
@dtolnay dtolnay closed this as not planned Won't fix, can't repro, duplicate, stale Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

7 participants