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

Make diagnostics clearer for ? operator #75029

Closed
wants to merge 1 commit into from

Conversation

JohnTitor
Copy link
Member

Fixes #71309
r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 1, 2020
Comment on lines +7 to +8
= note: `?` operator cannot convert from `isize` to `std::result::Result<isize, ()>`
= help: `?` operator unwraps `Result` or `Option` and propagates its `Err` or `None`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's verbose?

Copy link
Contributor

@Lonami Lonami Aug 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion this last note is not necessary. The clarification with concrete types of the first note should be enough, and people may be using the try_trait feature with custom types where this won't apply.

Though I can see this as The explanation that makes it clear for some people.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I feel the second note is probably unnecessary, no-one is likely to try to use ? without knowing that Result and Option are supported.

@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 21, 2020
@bors
Copy link
Contributor

bors commented Aug 30, 2020

☔ The latest upstream changes (presumably #74862) made this pull request unmergeable. Please resolve the merge conflicts.

Comment on lines +11 to +12
LL | let y: u32 = x?.try_into().unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestion seems to be using the incorrect Span. It should be replacing the ? with the new code.

@jyn514 jyn514 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. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 15, 2020
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2020
@Dylan-DPC-zz
Copy link

Closing this due to inactivity.

@Dylan-DPC-zz Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 8, 2020
@JohnTitor JohnTitor deleted the try-desugar branch June 7, 2021 05:46
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Sep 17, 2021
Make diagnostics clearer for `?` operators

Re-submission of rust-lang#75029, fixes rust-lang#71309
This also revives the `content` methods removed by rust-lang#83185.
r? `@estebank`
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 S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"try expression alternatives have incompatible types" is confusing wording
9 participants