Improve `?` error messages per RFC 1859

[steveklabnik]

RFC 1859 laid out specific error messages for the ? operator, but currently they are not implemented (and instead the compiler's internal desugaring is revealed).

UPDATE: Many parts of this are done, but not all. Here is a post with examples that are not yet great.

UPDATE 2: to close this ticket we need to improve the output of a single case left to point at the return type to explain where usize is coming from.

---

Original text:

try! in main has long been an issue, to the point of trying to adjust the language to support it: rust-lang/rfcs#1176

I was trying out ? today, and hit this error:

error[E0308]: mismatched types
 --> src/main.rs:5:13
  |
5 |     let f = File::open("hello.txt")?;
  |             ^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found enum `std::result::Result`
  |
  = note: expected type `()`
  = note:    found type `std::result::Result<_, _>`

That's the only line, inside of a main() function. Even though I know about this pitfall, and have been using Rust for a long time, I asked a question about it on IRC.

Can we print a better diagnostic here that points out it's the return type of the function that's looking for ()?

[durka]

Now that it's part of the language, we can even print help: the ? operator can only be used in a function that returns a Result.

[steveklabnik]

Yeah, that's exactly what I was hoping.

[nagisa]

Now that it's part of the language, we can even print help: the ? operator can only be used in a function that returns a Result.

Carrier trait.

[durka]

@nagisa can you elaborate? Do we have to wait on this kind of diagnostic until the Carrier stuff shakes out?

[nagisa]

@durka I mean that such diagnostic will make no sense once we implement and accept the carrier trait. First, because then you’ll be able to use ? in function that returns anything implementing Carrier trait (not just Result), but also because it would be possible to implement carrier trait for something like () in case anybody wanted to do so in their standard library.

Implementing such special cased diagnostic now would 100% result in it being forgotten until somebody comes along and complains that diagnostic about Result makes no sense (like it is already a case with a bunch of such help messages).

[durka]

Maybe we can just put a #[rustc_on_unimplemented] on Carrier? It would be neat if that worked, but that attribute has been sorely neglected of late. Regardless I hope that good diagnostics will be taken into account while designing the Carrier trait.

[carols10cents]

As of rustc 1.13.0-nightly (eac41469d 2016-08-30), the error in this case has gotten less understandable, imo:

error[E0277]: the trait bound `(): std::ops::Carrier` is not satisfied
 --> src/main.rs:6:13
  |
6 |     let f = File::open("hello.txt")?;
  |             ^^^^^^^^^^^^^^^^^^^^^^^^ trait `(): std::ops::Carrier` not satisfied
  |
  = note: required by `std::ops::Carrier::from_error`

error: aborting due to previous error

This error message seems to be guiding me towards implementing Carrier on (), but what would be better is what @steveklabnik and @durka suggested: something that mentions the return type is (), and that mentions you can only use ? in a function that returns a Result.

[nagisa]

This is what you can get with rustc_on_unimplemented:

error[E0277]: the trait bound `(): std::ops::Carrier` is not satisfied
 --> test.rs:4:5
  |
4 |     Ok::<()>(42)?;
  |     ^^^^^^^^^^^^^ trait `(): std::ops::Carrier` not satisfied
  |
  = note: the return type `()` cannot be used with the `?` operator
  = note: required by `std::ops::Carrier::from_error`

Bikeshed on wording.

[nikomatsakis]

@nagisa

This is what you can get with rustc_on_unimplemented:

I am not sure if this is good enough. I've long been unsatisfied with rustc_on_implemented for a couple of reasons -- one of them is that the "better" message is not the main thing people see. The other is that the ability to decide when to give the better message is pretty limited: often people write a "better" message for one particular scenario, but the same error (i.e., an unsatisfied trait bound) can appear in other scenarios where that better message sounds wrong.

i.e., here, we talk about return types, but if I wrote some code like:

fn foo<T: Carrier>() { .. .}
foo::<()>() // error

that error message would make any sense to me.

These two things are of course related, since giving a "maybe wrong" message high prominence feels bad.

That said, in this particular case, since Carrier is unstable, we can probably hard-code more -- but it feels like we could do better still. We know that the ? desugars into some particular code. We can probably tag that code and give the resulting trait obligations a more specific "cause", so that we can produce a good error in exactly this situation.

Note that this is valuable even once Carrier gets stabilized.

[durka]

Here's what I have so far:

fn does_not_return_result() -> i32 {
    try!(Ok(42));
}

error[E0308]: mismatched types
  --> src/test/compile-fail/expected-result.rs:14:5
14 |     try!(Ok(42));
   |     ^^^^^^^^^^^^^ expected i32, found enum `std::result::Result`
   |
   = note: `try!` and `?` can only be used in a function that returns a `Result`
   = note: this error originates in a macro outside of the current crate

Need to inhibit that second note. Next step is to actually figure out whether the error came from try! or ?.

[nikomatsakis]

@durka seems good so far!