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

Fix await suggestion on non-future type #90933

Merged
merged 1 commit into from
Nov 17, 2021
Merged

Conversation

compiler-errors
Copy link
Member

@compiler-errors compiler-errors commented Nov 15, 2021

Remove a match block that would suggest to add .await in the case where the expected type's Future::Output equals the found type. We only want to suggest .awaiting in the opposite case (the found type's Future::Output equals the expected type).

The code sample is here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=6ba6b83d4dddda263553b79dca9f6bcb

Before:

➜  ~ rustc --edition=2021 --crate-type=lib test.rs
error[E0308]: `match` arms have incompatible types
 --> test.rs:4:14
  |
2 |       let x = match 1 {
  |  _____________-
3 | |         1 => other(),
  | |              ------- this is found to be of type `impl Future`
4 | |         2 => other().await,
  | |              ^^^^^^^^^^^^^ expected opaque type, found enum `Result`
5 | |     };
  | |_____- `match` arms have incompatible types
  |
  = note: expected type `impl Future`
             found enum `Result<(), ()>`
help: consider `await`ing on the `Future`
  |
4 |         2 => other().await.await,
  |                           ++++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

After:

➜  ~ rustc +stage1 --edition=2021 --crate-type=lib test.rs
error[E0308]: `match` arms have incompatible types
 --> test.rs:4:14
  |
2 |       let x = match 1 {
  |  _____________-
3 | |         1 => other(),
  | |              ------- this is found to be of type `impl Future`
4 | |         2 => other().await,
  | |              ^^^^^^^^^^^^^ expected opaque type, found enum `Result`
5 | |     };
  | |_____- `match` arms have incompatible types
  |
  = note: expected type `impl Future`
             found enum `Result<(), ()>`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

Fixes #90931

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 15, 2021
@compiler-errors
Copy link
Member Author

Requesting review from Esteban K. since you introduced the code in c548511 and probably know best!

r? @estebank

@rust-log-analyzer

This comment has been minimized.

@compiler-errors
Copy link
Member Author

Fixed up the error to only suggest on ObligationCauseCode::Pattern.. I think that'll unbreak this test.

@estebank
Copy link
Contributor

I would expect the output for the test case to be

error[E0308]: `match` arms have incompatible types
 --> test.rs:4:14
  |
2 |       let x = match 1 {
  |  _____________-
3 | |         1 => other(),
  | |              ------- this is found to be of type `impl Future`
4 | |         2 => other().await,
  | |              ^^^^^^^^^^^^^ expected opaque type, found enum `Result`
5 | |     };
  | |_____- `match` arms have incompatible types
  |
  = note: expected type `impl Future`
             found enum `Result<(), ()>`
help: consider `await`ing on the `Future`
  |
4 |         1 => other().await,
  |                     ++++++

Looking at the name of the bindings, it looks like this should have been working correctly, but it's possible that the span being passed in was incorrect in the first place.

I think that where you changed the code, you need to look to see if you can get a MatchExpressionArmCause.prior_arms, and use those spans to suggest where to .await.

ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause {
semi_span,
source,
ref prior_arms,

if prior_arms.len() <= 4 {
for sp in prior_arms {
any_multiline_arm |= source_map.is_multiline(*sp);
err.span_label(*sp, format!("this is found to be of type `{}`", t));
}
} else if let Some(sp) = prior_arms.last() {
any_multiline_arm |= source_map.is_multiline(*sp);
err.span_label(
*sp,
format!("this and all prior arms are found to be of type `{}`", t),
);
}

I would make it a multipart_suggestion_verbose, so that it always shows on its own, and you might have more than one arm that resolved to impl Future, and you would have to .await all of them for the resulting code to be correct.

Would you be able to try that out and see if I'm right?

@compiler-errors
Copy link
Member Author

I applied the changes you asked for, I think! Let me know if I did it wrong.

I also removed the ObligationCauseCode::Pattern case from the branch corresponding to when exp_found.expected type equals exp_found.found's awaited type. I might be totally misunderstanding when we see ObligationCauseCode::Pattern, but I think that case would only ever happen if we could write a pattern matching only the opaque type? Let me know if I should add it back.

@estebank
Copy link
Contributor

The changes look reasonable. Can you add test cases covering all the new codepaths? Adding them to src/test/ui/async-await/suggest-missing-await.rs should suffice.

@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 16, 2021

📌 Commit fc816c3 has been approved by estebank

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 16, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 17, 2021
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90733 (Build musl dist artifacts with debuginfo enabled)
 - rust-lang#90787 (Add `#[inline]`s to `SortedIndexMultiMap`)
 - rust-lang#90920 (:arrow_up: rust-analyzer)
 - rust-lang#90933 (Fix await suggestion on non-future type)
 - rust-lang#90935 (Alphabetize language features)
 - rust-lang#90949 (update miri)
 - rust-lang#90958 (Mark `<*const _>::align_offset` and `<*mut _>::align_offset` as `const fn`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit eb9859f into rust-lang:master Nov 17, 2021
@rustbot rustbot added this to the 1.58.0 milestone Nov 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc suggests awaiting on non-future type on type mismatch with future
7 participants