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

Error message for question mark operator should message conversion #60917

Closed
sunjay opened this issue May 17, 2019 · 3 comments · Fixed by #60924
Closed

Error message for question mark operator should message conversion #60917

sunjay opened this issue May 17, 2019 · 3 comments · Fixed by #60924
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@sunjay
Copy link
Member

sunjay commented May 17, 2019

I've been working with a lot of people who are either new to Rust or new to its latest features (like the question mark operator) and I'm noticing a lot of confusion around a specific error message.

Suppose you have the following code:

#[derive(Debug)]
struct Error1;
#[derive(Debug)]
struct Error2;

fn foo() -> Result<(), Error1> {
    Err(Error1)
}

fn main() -> Result<(), Error2> {
    foo()?;
    
    Ok(())
}

This gives you the error:

error[E0277]: the trait bound `Error2: std::convert::From<Error1>` is not satisfied
  --> src/main.rs:11:5
   |
11 |     foo()?;
   |     ^^^^^^ the trait `std::convert::From<Error1>` is not implemented for `Error2`
   |
   = note: required by `std::convert::From::from`

However, if you look at the actual code, it isn't obvious where the From trait is even being used.

The documentation for the From trait actually does mention this briefly, though most people I encounter don't go as far as looking that up. The error message is pretty daunting if you're new to the language.

I propose that we add something to the error message when we see that they are using the question mark:

help: the question mark operation (`?`) implicitly performs a conversion
      on the error value using the `From` trait. If no conversion is
      defined, you will see this error message.

This conveys exactly where the From trait is being used and why they may be running into this issue. You certainly don't have to use this exact wording. Feel free to use whatever fits. :)

@jonas-schievink jonas-schievink added A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 17, 2019
@estebank
Copy link
Contributor

For what it's worth, the current output points at the question mark now:

error[E0277]: `?` couldn't convert the error to `Error2`
  --> src/main.rs:11:10
   |
11 |     foo()?;
   |          ^ the trait `std::convert::From<Error1>` is not implemented for `Error2`
   |
   = note: required by `std::convert::From::from`

@sunjay
Copy link
Member Author

sunjay commented May 17, 2019

Thanks! That's a good start. I find that just pointing to it isn't enough because people don't even know that the question mark operator performs a conversion. The [edition guide] (https://doc.rust-lang.org/edition-guide/rust-2018/error-handling-and-panics/the-question-mark-operator-for-easier-error-handling.html) (a top result for looking up "rust question mark operator") doesn't even reference the From trait.

@estebank
Copy link
Contributor

Does #60924 look like what you had in mind, @sunjay?

Centril added a commit to Centril/rust that referenced this issue May 19, 2019
Explain that ? converts the error type using From

Fix rust-lang#60917.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants