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

Diagnostic about try could be better on edition 2018 #54774

Closed
Centril opened this issue Oct 3, 2018 · 8 comments
Closed

Diagnostic about try could be better on edition 2018 #54774

Centril opened this issue Oct 3, 2018 · 8 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-frontend Area: Compiler frontend (errors, parsing and HIR) 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

@Centril
Copy link
Contributor

Centril commented Oct 3, 2018

Given:

fn main() -> Result<(), ()> {
    let x = try!(Ok(1u8));
    Ok(())
}

the compiler emits the standard "reserved keyword error":

error: expected expression, found reserved keyword `try`
 --> src/main.rs:2:13
  |
2 |     let x = try!(Ok(1u8));
  |             ^^^ expected expression

However, since try! was once a macro people used (wherefore it stands to reason that some old documentation may folk astray), it might be prudent to catch this pattern somehow and refer the user to the documentation about ? somewhere.

cc @scottmcm, @estebank

@Centril Centril added A-frontend Area: Compiler frontend (errors, parsing and HIR) 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. WG-compiler-front labels Oct 3, 2018
@zackmdavis
Copy link
Member

refer the user to the documentation about ? somewhere.

Or write a cargo fix-able lint for the rust-2018-compatibility lint group.

@estebank
Copy link
Contributor

estebank commented Oct 3, 2018

Is the macro still available in 2018 as r#try? If it is we should also suggest it, but otherwise just

error: expected expression, found reserved keyword `try`
 --> src/main.rs:2:13
  |
2 |     let x = try!(Ok(1u8));
  |             ^^^ expected expression
  help: the `try!` macro has been removed from the language, use the `?` operator instead
  |
2 |     let x = Ok(1u8)?;
  |             ^^^^^^^^

The actual implementation of the MachineApplicable suggestion needs to attempt to parse try!(.*), and if successful suggest accounting for operator precedence (injecting braces as needed).

As a stretch goal, if we encounter any keyword being used and there's an r#{keyword} binding in scope, or if we're in the name of a let statement, suggest using the r#{keyword} version.

error: expected expression, found reserved keyword `try`
 --> src/main.rs:2:13
  |
2 |     let try = 42;
  |         ^^^ 
  |         |
  |         expected expression
  |         help: you can use the raw identifier format: `r#try`
error: expected expression, found reserved keyword `try`
 --> src/main.rs:2:13
  |
2 |     let x = try;
  |             ^^^ 
  |             |
  |             expected expression
  |             help: a binding with that name using the raw identifier format is available: `r#try`

@Centril
Copy link
Contributor Author

Centril commented Oct 3, 2018

Is the macro still available in 2018 as r#try?

Yes; it has to be and this is what cargo fix --edition will change try!(..) to.

help: the try! macro has been removed from the language, use the ? operator instead

This seems good; tho technically it hasn't been removed so perhaps emit:

if you were trying to use the try! macro, use the ? operator instead or refer to the macro with r#try!.

We should try to nudge folks towards ? here in some way that is.

As a stretch goal, if we encounter any keyword being used and there's an r#{keyword} binding in scope, or if we're in the name of a let statement, suggest using the r#{keyword} version.

This seems like a great idea.

@estebank
Copy link
Contributor

estebank commented Oct 3, 2018

Given that r#try!() is still available in 2018, then the error should be closer to the following in that case

error: expected expression, found reserved keyword `try`
 --> src/main.rs:2:13
  |
2 |     let x = try!(Ok(1u8));
  |             ^^^ expected expression
  help: `try` is a reserved keyword in the 2018 edition, use the `?` operator instead
  |
2 |     let x = Ok(1u8)?;
  |             ^^^^^^^^
  help: alternatively, you can still access the `try!` macro using the raw identifier format
  |
2 |     let x = r#try!(Ok(1u8));
  |             ^^

@Centril
Copy link
Contributor Author

Centril commented Oct 3, 2018

That seems better / quite good :)

@philippludwig
Copy link

Is there any documentation somewhere why try! was removed? All I can find is this issue.

@Centril
Copy link
Contributor Author

Centril commented Jan 11, 2019

@philippludwig it was decided here: #53686 (comment).

@crlf0710 crlf0710 added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jun 11, 2020
@scottmcm
Copy link
Member

Looks like the improved version happened, so I'll close this:

error: use of deprecated `try` macro
 --> src/main.rs:2:13
  |
2 |     let x = try!(Ok(1u8));
  |             ^^^^^^^^^^^^^
  |
  = note: in the 2018 edition `try` is a reserved keyword, and the `try!()` macro is deprecated
help: you can use the `?` operator instead
  |
2 |     let x = Ok(1u8)?;
  |            --      ^
help: alternatively, you can still access the deprecated `try!()` macro using the "raw identifier" syntax
  |
2 |     let x = r#try!(Ok(1u8));
  |             ^^

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 A-frontend Area: Compiler frontend (errors, parsing and HIR) 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

6 participants