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

Add methods for unwrapping Result<T, !> / Result<!, E> #1723

Open
canndrew opened this issue Aug 22, 2016 · 31 comments
Open

Add methods for unwrapping Result<T, !> / Result<!, E> #1723

canndrew opened this issue Aug 22, 2016 · 31 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@canndrew
Copy link
Contributor

One of the main intended uses for the ! type is to be able construct the types Result<T, !> and Result<!, E>. Right now, if you have r: Result<T, !> there are several ways to unpack it's inner T, but none of them are ideal:

  • r.unwrap(): unwrap is scary and should be avoided. This is also fragile if someone later changes the error type to something other than !.
  • r.unwrap_or_else(|e| e): Kinda confusing.
  • let Ok(val) = r;: Requires a separate statement. (Also doesn't currently work).
  • match r { Ok(v) => v, Err(e) => e }: Both long and kinda confusing.
  • match r { Ok(v) => v }: Not as nice as a method on r. (Also doesn't currently work).

The void crate adds the methods void_unwrap and void_unwrap_err to Result<T, Void> and Result<Void, T>. I think methods like these should be added to libcore as well. Perhaps we could call them always_ok() and always_err().

@sfackler
Copy link
Member

I like always_* naming-wise.

@glaebhoerl
Copy link
Contributor

glaebhoerl commented Aug 22, 2016

Another possibility is if enums where only a single variant is inhabited could have their fields accessed directly, in the same way as structs (which likewise "have a single variant"). So in the case of Result<T, !>, you could access the value simply using .0 (and in the case of enums with "structural variants", using .fieldname). This would presumably have a fix for rust-lang/rust#12609 as a precondition just like the two items in @canndrew's list which don't currently work.

@Stebalien
Copy link
Contributor

I believe we could also just use From/Into:

impl<T> From<Result<T, !>> for T {
    fn from(r: Result<T, !>) -> T {
        r.unwrap()
    }
}

@nrc nrc added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Aug 22, 2016
@Sgeo
Copy link

Sgeo commented Aug 22, 2016

@glaebhoerl What if someone changes the type from Result<T, !> to Result<!, T> for some reason? Then .0 continues to work but means something different.

@Stebalien
Copy link
Contributor

@Sgeo

Well, those are different types so that's a breaking change anyways.

@Ericson2314
Copy link
Contributor

For the sake of try! and ?,

impl<T> From<!> for T {
    fn from(r: !) -> T {
        match r { }
    }
}

is my top priority API decision. After that goes whatever else also must also be in stdlib crates to satisfy coherence.

@eddyb
Copy link
Member

eddyb commented Aug 23, 2016

@Ericson2314 Doesn't that overlap with the identity impl of From?

@Ericson2314
Copy link
Contributor

Ericson2314 commented Aug 23, 2016

Uh oh. Any ideas? cause that's a crucial impl.

@Stebalien
Copy link
Contributor

@Ericson2314 not unless we get some way to say where A != B. Feel up for writing an RFC? This is not the same thing as where A: !Trait and doesn't have the same drawbacks (as far as I know at least).

@eddyb Doesn't that overlap with the identity impl of From?

Well..., if we're doing this in core we can use my favorite unstable hack... (although we really shouldn't ever do this):

#![feature(optin_builtin_traits, never_type)]
mod private {
    pub trait NotNever {}
    impl NotNever for .. {}
    impl !NotNever for ! {}
}

trait MyFrom<T> {
    fn from(r: T) -> Self;
}

impl<T> MyFrom<T> for T {
    fn from(r: T) -> T {
        r
    }
}
impl<T: private::NotNever> MyFrom<!> for T {
    fn from(_: !) -> T {
        panic!()
    }
}

@Ericson2314
Copy link
Contributor

@Stebalien that won't work in generic contexts will it?

@Stebalien
Copy link
Contributor

@Ericson2314 It seems to work (gist to avoid polluting this thread with nonsense): https://gist.github.com/85c3af06d9db3a31ba810d9a23792350

Unfortunately, it sets a precedent we can't back out of.

@Ericson2314
Copy link
Contributor

https://is.gd/mU4KWT line 26 fails. I should have been clearer and said I was worried the above wouldn't be as useful due to parametricitiy and a lack of or constraints.

Intriguingly, try! and ? already work, however.

@Stebalien
Copy link
Contributor

Ah 💩. Good point. (My favorite trick failed me 😿)

@Ericson2314
Copy link
Contributor

I mean, now that specialization is here, we don't care about parametricity any more, right? :P

@glaebhoerl
Copy link
Contributor

glaebhoerl commented Aug 23, 2016

Or maybe there could be a way to explicitly designate an impl as incoherent (which would require unsafe), similarly to GHC, for use in cases where two impls overlap for some instantiation of their type variables, but it doesn't matter which one is chosen because they do the same thing. That situation seems to come up quite often around From. (Or would specialization be sufficient to solve this?)

@eddyb
Copy link
Member

eddyb commented Aug 23, 2016

@glaebhoerl Sounds like something lattice-based specialization can solve.
Do keep in mind though that there's at least one example where they're not doing the same thing.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Aug 24, 2016

@eddyb hmm? In the case of ! I think all possible impls must do the same thing.

@eddyb
Copy link
Member

eddyb commented Aug 24, 2016

@Ericson2314 Yes, but there are other overlapping cases where the identity impl would change meaning (in the linked example, there would be extra boxing).

@Ericson2314
Copy link
Contributor

@eddyb sure, just wanted to be clear that that's not strictly relevant for this usecase.

@canndrew
Copy link
Contributor Author

I believe we could also just use From/Into:

I think we should still have explicit methods for doing this conversion though and not just rely on From. I have a bit of a philosophical quarrel with the {,Try}{From,Into} traits in that they're overly generic in their purpose and don't really describe what they do. Sure, they convert from one type into another... but that describes every function ever. For example we don't technically need a TcpStream::connect function we could just impl TryFrom<SocketAddr> for TcpStream.

IMHO, the only time people should impl From<A> for B is if there's a single, canonical, completely obvious conversion from A to B. And they should avoid calling from whenever they know what A and B are and instead call a more explicit method with a more descriptive name. So I think we should have the From impl, but have the always_* methods as well.

@canndrew
Copy link
Contributor Author

canndrew commented Aug 27, 2016

To be a bit clearer about what I mean: I think what counts as "completely obvious" depends on who's reading the code. If someone doesn't entirely grok ! then they might be a little unsure about what's happening when they see Result<T, !>::from() produce a T. Calling always_ok instead makes it a bit clearer what's going on. It also explains what's going on to a reader who understands ! but doesn't know what types are at play in this particular piece of code they're reading.

@mark-i-m
Copy link
Member

mark-i-m commented Sep 2, 2016

@canndrew I think that makes a lot of sense, but I am wondering how to make this more generic... We could just add always_* to Result, but it doesn't answer the question of how denote that some operation is guaranteed to work...

A couple of ideas I had:

  1. impl Deref for Result<T, !> and impl Deref for Result<!, E> (maybe this suffers from the same problems as From?) In some sense, implementing Deref is like guaranteeing the deref coersion to work.
  2. Define an Always trait that denotes that we know something is always true about the types we implement it for (though I'm not sure what the best way to formulate it is). We can then implement Always for Results with !
pub trait Always {
     type Other; // perhaps a better name?
     fn flatten(self) -> Self; // perhaps a better name here too?
}

impl<T> Always<T> for Result<T, !> {
    type Other = T;
    fn flatten(self) -> Self::Other {
          self.ok().unwrap()
    }
}
impl<E> Always<E> for Result<!, E> {
    type Other = E;
    fn flatten(self) -> Self::Other {
          self.err().unwrap()
    }
}

As you can see, this looks very similar to Deref.

@plietar
Copy link

plietar commented Sep 2, 2016

Any solution trying to represent the two operations as a single trait will fail to work on Result<!,!>, in addition to being very confusing.
I find always_ok / always_err very clear and explicit about what they do, and I expect them to produce meaningful error messages if used incorrectly, unlike the other proposed solutions.

@glaebhoerl
Copy link
Contributor

Trying to abstract the common things between Option and Result is a worthy ideal, but essentially none of the existing things do it so always_ok/always_err probably isn't the right place to start.

@mark-i-m
Copy link
Member

mark-i-m commented Sep 2, 2016

the two operations as a single trait will fail to work on Result<!,!>, in addition to being very confusing.

@plietar Could you elaborate, please? I don't think I understand

@glaebhoerl Hmm... I see what you mean

@plietar
Copy link

plietar commented Sep 3, 2016

@mark-i-m say you have the following implementations, like you suggested :

impl<T> Always for Result<T, !> { ... }
impl<E> Always for Result<!, E> { ... }

The are now two implementations of Always for Result<!,!>. By the rust's coherence rules this isn't allowed.

The compiler is actually suprisingly good at describing it : https://is.gd/q9lq04
Using a type parameter for Always instead of an associated type has the same issue: https://is.gd/TXdfc6

@mark-i-m
Copy link
Member

mark-i-m commented Sep 3, 2016

Ah, I see now. Thanks, @plietar

@phaylon
Copy link

phaylon commented Jun 11, 2019

Has there been any movement in this design space recently?

I was helpfully pointed here after proposing an unwrap_infallible member function for unwrapping Result<T, Infallible> into plain T.

@scottmcm
Copy link
Member

I'm not aware of any, but with Infallible being stable I think there's probably more appetite for something here. Any thoughts on how broadly changes here should happen? Is there space for a new library conventions RFC kind of thing around what APIs should exist to deal with infallible things? Is there a trait that should be used to make these general, like bounding on Into<Infallible>?

@phaylon
Copy link

phaylon commented Jun 14, 2019

@scottmcm I don't think Into<Infallible> will be relevant, but at some point Infallible will be !, and Into<!> might be something to consider coming up. I reread the RFC and some of the following discussions, but I'm unsure what the plan is with types like (!, i32) that are also unconstructable.

@mzabaluev
Copy link
Contributor

Uh, looks like I reinvented this in #2799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests