Quick draft "Result::expect" rfc

[thepowersgang]

See also rust-lang/rust#25359 for implementation

Rendered

[nagisa]

Not a drawback per se.

[bluss]

Seems logical to me. After you get to know .expect() on Option, you'll expect to find the method on Result too.

[Manishearth]

@gankro @sfackler @huonw This needs a sheriff

[nielsle]

This would be great for quick and dirty scripts. It would also make one of the first examples in the rust book a little easier to read:

   io::stdin().read_line(&mut guess)
        .ok()
        .expect("Failed to read line");

https://doc.rust-lang.org/book/guessing-game.html

[emberian]

I dislike this signature, and much prefer:

impl<T, U: Debug, V: Display> Foo<T, V> for Result<T, U> {
    fn expect(self, msg: V) -> T {
        match self {
            Ok(v) => v,
            Err(e) => panic!("Error: {} ({:?})", msg, e)
        }
    }
}

[emberian]

(And I think Option should be the same)

[thepowersgang]

Ooh, interesting idea. This would allow passing format_args! for formatted messages

[grissiom]

I think .expect("Failed to read line") mean to be "expect to Fail..." which is logically inverse to what it is done. I think .on_error("Failed to read line") should be more logical. Am I the only one thinking this way?...

[thepowersgang]

@grissiom Possibly, this RFC just brings the avaliable methods into line with Option's methods.

[gsingh93]

@grissiom, I completely agree. I think this should at least be discussed before accepting this RFC so we'd only have to deprecate one method, not two, if this were accepted.

[thepowersgang]

@grissiom @gsingh93 The .expect() method is part of the stable API for Option, and thus cannot be changed without incrementing the language's major version.

[gsingh93]

Yes, but it can be deprecated.

[grissiom]

@gsingh93 @thepowersgang OK, when I have time, I will open an other RFC to discuss Option::expect and reference this RFC. Thanks.

[alexcrichton]

This RFC is now entering the week-long final comment period.

[emberian]

I still REALLY think that this should take msg: Display, but that can be added after the fact. I'm happy with this RFC in its entirety (including the format string).

[gkoz]

I don't think the point about Debug formatting for errors being inferior to Display has been addressed.

Which of these are more helpful?
Error { repr: Os(2) } vs No such file or directory (os error 2)
JoinPathsError { inner: JoinPathsError } vs path segment contains separator ':'

Maybe this issue should be raised elsewhere though because it's not a problem with expect per se.

[BurntSushi]

Maybe this issue should be raised elsewhere though because it's not a problem with expect per se.

I suspect this is the right way to think about it. For better or worse, Option::unwrap, Option::expect, Result::unwrap and Result::unwrap_err are all bounded with Debug. Making Result::expect use Display instead seems pretty surprising to me.

[Gankra]

The text as written now is a bit unclear. I'd really like it to explicitly provide the signature and implementation (since it should be trivial to provide, and is the entire point of the RFC).

[thepowersgang]

@gkoz That should be raised somewhere, probably on the offending error type, making Debug include the error text as well as the number.

[sfackler]

Maybe this issue should be raised elsewhere though because it's not a problem with expect per se.

Agreed. We could change the conventions for error types to have more informative Debug output, or, when specialization is implemented, use Display output for objects implementing Debug+Display and Debug output for objects only implementing Debug as a couple of options.

[bluss]

Edit: see below.

@BurntSushi: I think the cases are different. The Debug bound is on the value inside the Result. The Display bound would be on the message you pass to .expect(). Display is a (natural?) generalization of passing just a &str for a message. Debug is not a generalization of passing a string at all.

Also, we can demand a rarer trait like Display when the user must consciously choose to pass in a fitting message. We need Debug for the other cases to be able to work with a broader range of values contained in the Result.

[gkoz]

@bluss but that part of discussion was about the value inside the Result, not the message.

[bluss]

@gkoz Oops! I'm sorry for the noise.

[alexcrichton]

The consensus among the libs team is to accept this RFC now. The matter of generalizing the argument of expect to T: Display can be handled at a later date (and should also affect Option as well). Additionally, using Debug instead of Display for the error type in the result follows the precedent of the unwrap method, which while without downsides covers the most possible types today.

Thanks for the RFC @thepowersgang!