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

Invalid 'compatible variant' suggestion on mismatched types in .await expression #93074

Closed
m-ou-se opened this issue Jan 19, 2022 · 7 comments · Fixed by #93103
Closed

Invalid 'compatible variant' suggestion on mismatched types in .await expression #93074

m-ou-se opened this issue Jan 19, 2022 · 7 comments · Fixed by #93103
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Jan 19, 2022

This code:

async fn a() {}

async fn b() -> Result<(), i32> {
    a().await
}

produces:

error[E0308]: mismatched types
 --> src/main.rs:4:8
  |
4 |     a().await
  |        ^^^^^^ expected enum `Result`, found `()`
  |
  = note:   expected enum `Result<(), i32>`
          found unit type `()`
help: try wrapping the expression in `Ok`
  |
4 |     a()Ok(.await)
  |        +++      +

Which is invalid.

Looks like the span of the desugaring of .await uses only the .await part, instead of the span of the entire expression. This results in invalid suggestions when used by the suggest_compatible_variants code in compiler/rustc_typeck/src/check/demand.rs.

@m-ou-se m-ou-se 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. C-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. labels Jan 19, 2022
@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 19, 2022

I'm not entirely sure if this means that .await desugaring should use a bigger span, or if the suggestion diagnostic needs to be interpreted differently.

@m-ou-se m-ou-se changed the title Fix 'compatible variant' suggestion on mismatched types in .await expression Invalid 'compatible variant' suggestion on mismatched types in .await expression Jan 19, 2022
@compiler-errors
Copy link
Member

hmm.. is .await the only case where the desugared expr's span doesn't cover the whole piece of code it comes from? That doesn't sound right.

If wonder if the expr should have the bigger span by default, and then async-specific code could grab the tighter span for diagnostics that need to point at the .await operator.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 19, 2022

Some of the ? desugaring only uses the ? span, but the parts relevant for diagnostics like this do use the full span. Not sure if there's a problem there too.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 19, 2022

I thought I might have accidentally introduced this bug in #90575, but right after that PR was merged, the output was correct:

help: try wrapping the expression in `Ok`
  |
4 |     Ok(a().await)
  |     +++         +

So something changed in the last two months in what spans are used for await desugarings, it seems.

@compiler-errors
Copy link
Member

@m-ou-se: actually I think it was introduced in #90939 in 7227a87, cc @estebank
Do you need someone to fix this? I'm happy to put up a quick PR.

@m-ou-se
Copy link
Member Author

m-ou-se commented Jan 19, 2022

Yeah, looks like it: I just finished bisecting, which pointed at #91945: a rollup involving #90939.

@estebank
Copy link
Contributor

@compiler-errors this was introduced in the PR linked earlier to try and improve other errors. It is tricky because reducing the span to only .await incurs these kind of bugs. On the other hand, the errors that got improved there became much less noisy, so I wouldn't want to revert the whole change. My fix above will solve this issue, but I'm sure there will be others in the future :-/

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 23, 2022
@bors bors closed this as completed in 7356e28 Jan 23, 2022
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-bug Category: This is a bug. D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. 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