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: improve the suggestion on future not awaited #107902

Merged
merged 1 commit into from
Feb 14, 2023

Conversation

vincenzopalazzo
Copy link
Member

Considering the following code

fn foo() -> u8 {
    async fn async_fn() -> u8 {  22 }

    async_fn()
}

fn main() {}

the error generated before this commit from the compiler is

➜  rust git:(macros/async_fn_suggestion) ✗ rustc test.rs --edition 2021
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
...
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note:     expected type `u8`
          found opaque type `impl Future<Output = u8>`
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ++++++

error: aborting due to previous error

In this case the error is nor perfect, and can confuse the user that do not know that the opaque type is the future.

So this commit will propose (and conclude the work start in #80658)
to change the string opaque type to future when applicable and also remove the Expected vs Received note by adding a more specific one regarding the async function that return a future type.

So the new error emitted by the compiler is

error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
...
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found future
  |
note: calling an async function returns a future
 --> test.rs:4:5
  |
4 |     async_fn()
  |     ^^^^^^^^^^
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ++++++

error: aborting due to previous error

Fixes #80658

It remains to rework the case described in the following issue #107899 but I think this deserves its own PR after we discuss a little bit how to handle these kinds of cases.

r? @eholk

@rustbot label +I-async-nominated

Signed-off-by: Vincenzo Palazzo vincenzopalazzodev@gmail.com

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-async-nominated Nominated for discussion during an async working group meeting. labels Feb 10, 2023
compiler/rustc_infer/src/infer/error_reporting/suggest.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/error_reporting/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/error_reporting/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_infer/src/infer/error_reporting/suggest.rs Outdated Show resolved Hide resolved
| |
| arguments to this function are incorrect
|
= note: expected opaque type `impl Future<Output = ()>` (opaque type at <$DIR/generator-desc.rs:5:16>)
found opaque type `impl Future<Output = ()>` (opaque type at <$DIR/generator-desc.rs:6:16>)
= help: consider `await`ing on both `Future`s
= note: distinct uses of `impl Trait` result in different opaque types
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this message should also be specialized for futures.

Suggested change
= note: distinct uses of `impl Trait` result in different opaque types
= note: each `async fn` produces a future that is a unique type

That message sucks but something like that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with you, but this suggestion looks unrelated to this PR? maybe the right position is a followup PR?

@eholk
Copy link
Contributor

eholk commented Feb 10, 2023

@bors r? compiler-errors

(since he's already adding comments on it)

@rustbot rustbot assigned compiler-errors and unassigned eholk Feb 10, 2023
Considering the following code

```rust
fn foo() -> u8 {
    async fn async_fn() -> u8 {  22 }

    async_fn()
}

fn main() {}
```

the error generated before this commit from the compiler is

```
➜  rust git:(macros/async_fn_suggestion) ✗ rustc test.rs --edition 2021
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
...
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note:     expected type `u8`
          found opaque type `impl Future<Output = u8>`
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ++++++

error: aborting due to previous error
```

In this case the error is nor perfect, and can confuse the user
that do not know that the opaque type is the future.

So this commit will propose (and conclude the work start in
rust-lang#80658)
to change the string `opaque type` to `future` when applicable
and also remove the Expected vs Received note by adding a more
specific one regarding the async function that return a future type.

So the new error emitted by the compiler is

```
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
...
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found future
  |
note: calling an async function returns a future
 --> test.rs:4:5
  |
4 |     async_fn()
  |     ^^^^^^^^^^
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ++++++

error: aborting due to previous error
```

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@vincenzopalazzo vincenzopalazzo force-pushed the macros/async_fn_suggestion branch from 2bbceaa to 2bdc9a0 Compare February 13, 2023 15:23
@eholk
Copy link
Contributor

eholk commented Feb 13, 2023

We discussed this in triage today. We think saying "future" rather than "opaque type" is an improvement. The only caveat we had was that we should try to do that only when you're calling an async fn rather than an -> impl Future. Suggesting .await is good too, but we should try to do that only when it fixes the issue (i.e. you're in an async context and the future resolves to a type that coerces to the return type).

@compiler-errors
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Feb 13, 2023

📌 Commit 2bdc9a0 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Feb 13, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#107902 (fix: improve the suggestion on future not awaited)
 - rust-lang#107913 (Update broken link in cargo style guide)
 - rust-lang#107942 (Tighter spans for bad inherent `impl` self types)
 - rust-lang#107948 (Allow shortcuts to directories to be used for ./x.py fmt)
 - rust-lang#107971 (Clearly document intentional UB in mir-opt tests)
 - rust-lang#107985 (Added another error to be processed in fallback)
 - rust-lang#108002 (Update books)
 - rust-lang#108013 (rustdoc: use a string with one-character codes for search index types)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 73b022b into rust-lang:master Feb 14, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 14, 2023
@vincenzopalazzo vincenzopalazzo deleted the macros/async_fn_suggestion branch March 15, 2023 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-async-nominated Nominated for discussion during an async working group meeting. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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.

Note when opaque future from async fn is involved in a type mismatch is unclear
5 participants