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 diagnostics for ? regressed over try! #42694

Closed
skade opened this issue Jun 16, 2017 · 2 comments
Closed

Error diagnostics for ? regressed over try! #42694

skade opened this issue Jun 16, 2017 · 2 comments
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

@skade
Copy link
Contributor

skade commented Jun 16, 2017

This is a thing that occured to me while giving a training on Rust: the diagnostics of ? over try! are worse, despite it being special syntax.

Compare:

fn main() {
    
}

try() {
    try!(std::fs::File::open("foo"));
}


rustc 1.19.0-nightly (258ae6dd9 2017-06-15)
error[E0308]: mismatched types
 --> <anon>:6:5
  |
6 |     try!(std::fs::File::open("foo"));
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found enum `std::result::Result`
  |
  = note: expected type `()`
             found type `std::result::Result<_, _>`
  = note: this error originates in a macro outside of the current crate

error: aborting due to previous error(s)

over:

fn main() {
    
}

fn question() {
    std::fs::File::open("foo")?;
}
rustc 1.19.0-nightly (258ae6dd9 2017-06-15)
error[E0277]: the trait bound `(): std::ops::Try` is not satisfied
 --> <anon>:6:5
  |
6 |     std::fs::File::open("foo")?;
  |     ---------------------------
  |     |
  |     the trait `std::ops::Try` is not implemented for `()`
  |     in this macro invocation
  |
  = note: required by `std::ops::Try::from_error`

While the second makes perfect sense to advanced Rust developers, it is confusing to newcomers and leaks a lot of internal details. People have to know about traits, about the ops module, and similar. Also, it reports a macro invocation, where - from a user perspective - there is none. To my knowledge, this is the only place where we do this.

Considering that ? raises a macro to syntax, I would also think it gives opportunity for better diagnostics. It is a crucial part of the language with non-trivial semantics and making a pass on giving better errors here would help newcomers a lot.

@bstrie bstrie added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 16, 2017
@steveklabnik
Copy link
Member

We have #35946 tracking this sorta

bors added a commit that referenced this issue Jul 6, 2017
Add `rustc_on_unimplemented` message to `std::ops::Try`

#42694, #35946.
@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 26, 2017
@estebank
Copy link
Contributor

Current output on nightly after #43001:

error[E0277]: the trait bound `(): std::ops::Try` is not satisfied
 --> file.rs:6:5
  |
6 |     std::fs::File::open("foo")?;
  |     ---------------------------
  |     |
  |     the `?` operator can only be used in a function that returns `Result` (or another type that implements `std::ops::Try`)
  |     in this macro invocation
  |
  = help: the trait `std::ops::Try` is not implemented for `()`
  = note: required by `std::ops::Try::from_error`

bors added a commit that referenced this issue Jul 28, 2017
Use `rustc_on_unimplemented`'s trait name argument in `try`

Follow up to #43000 and #43001. Fix #42694.
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

No branches or pull requests

5 participants