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

Note when opaque future from async fn is involved in a type mismatch is unclear #80658

Closed
SNCPlay42 opened this issue Jan 3, 2021 · 24 comments · Fixed by #107902
Closed

Note when opaque future from async fn is involved in a type mismatch is unclear #80658

SNCPlay42 opened this issue Jan 3, 2021 · 24 comments · Fixed by #107902
Assignees
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@SNCPlay42
Copy link
Contributor

SNCPlay42 commented Jan 3, 2021

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

emits

error[E0308]: mismatched types
 --> src/lib.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- the `Output` of this `async fn`'s found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note:     expected type `u8`
          found opaque type `impl Future`

The note "the Output of this async fn's found opaque type" is kind of hard to understand. If the return type of async_fn isn't explicitly given, it doesn't even point at a type:

2 |     async fn async_fn() {}
  |                         - the `Output` of this `async fn`'s found opaque type

Perhaps the note should instead look like this:

2 |     async fn async_fn() {}
  |     ----- the found opaque type is the `impl Future` returned by this `async fn`

@rustbot label A-async-await A-diagnostics D-confusing T-compiler

@rustbot rustbot added A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 3, 2021
@JohnTitor JohnTitor added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jan 5, 2021
@nellshamrell nellshamrell self-assigned this Jan 7, 2021
@tmandry tmandry added the AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. label Jan 7, 2021
@nellshamrell
Copy link
Contributor

Looks like #66463 touched areas of code that are relevant to this issue (not to say that it caused the issue, only that it provides a potential pointer to the parts of the code related to generating this error message)

@tmandry
Copy link
Member

tmandry commented Jan 28, 2021

@nellshamrell Are you still planning to work on this? If not no worries, feel free to release your assignment.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 26, 2021
…tebank

Reword labels on E0308 involving async fn return type

Fix for rust-lang#80658.

When someone writes code like this:

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

    async_fn()
}
```

And they try to compile it, they will see an error that looks like this:

```bash
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of this `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`
```
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 26, 2021
…tebank

Reword labels on E0308 involving async fn return type

Fix for rust-lang#80658.

When someone writes code like this:

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

    async_fn()
}
```

And they try to compile it, they will see an error that looks like this:

```bash
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of this `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`
```
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Feb 26, 2021
…tebank

Reword labels on E0308 involving async fn return type

Fix for rust-lang#80658.

When someone writes code like this:

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

    async_fn()
}
```

And they try to compile it, they will see an error that looks like this:

```bash
error[E0308]: mismatched types
 --> test.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of this `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`
```
@henryboisdequin
Copy link
Contributor

Can't this issue be closed (#82165) or is there still more work to be done?

@nellshamrell nellshamrell removed their assignment Mar 18, 2021
@camelid
Copy link
Member

camelid commented Apr 2, 2021

The note seems clearer now:

error[E0308]: mismatched types
 --> src/lib.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> () {}
  |                            -- checked the `Output` of this `async fn`, found opaque type
3 |     
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of the `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`

But it would be good to get confirmation that this is fixed before closing.

@SNCPlay42
Copy link
Contributor Author

"checked the Output of this async fn, found opaque type" still seems confusing to me; that makes it sound like the mismatch is in the Output of the Future returned by the function, which isn't the case. The mismatch is that it is a Future<Output = T>, regardless of what T is, instead of just a u8.

(Oops, did I confuse the issue by giving foo and async_fn different nominal return types? It would still be a mismatch if async_fn was declared as async fn async_fn() -> u8.)

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Apr 13, 2021

This is going to be a very common scenario.

I'd prefer to see a more radically improved error message. What I posted on #84166 was something like

error[E0308]: missing await on future
 --> src/main.rs:8:12
  |
3 | async fn return_number() -> usize {
  | ----- calling an async function returns a future
...
8 |     return return_number();
  |            ^^^^^^^^^^^^^^^ expected `usize`, found future
  |
help: consider `await`ing on the `Future`
  |
8 |     return return_number().await;
  |                           ^^^^^^

error: aborting due to previous error

@tmandry
Copy link
Member

tmandry commented Apr 16, 2021

@rustbot assign @nellshamrell

@nikomatsakis
Copy link
Contributor

So I was looking at this in order to write mentoring instructions. I note that the example in the OP contains a kind of type error (the async function is returning (), but the outer function is returning u8) -- @SNCPlay42 was that intentional?

If we rewrite to this example:

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

we presently get this error:

error[E0308]: mismatched types
 --> src/lib.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> u8 {  22 }
  |                            -- the `Output` of this `async fn`'s found opaque type
3 |     
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note:     expected type `u8`
          found opaque type `impl Future`
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ^^^^^^

@nikomatsakis
Copy link
Contributor

I would like to give this error;

error[E0308]: missing await on future
 --> src/main.rs:8:12
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> u8 {  22 }
  |     ----- calling an async function returns a future
3 |     
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found future
  |
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ^^^^^^

error: aborting due to previous error

which isn't that far apart.

@nikomatsakis
Copy link
Contributor

I believe the current "help" is generated by this function:

/// A possible error is to forget to add `.await` when using futures:
///
/// ```
/// async fn make_u32() -> u32 {
/// 22
/// }
///
/// fn take_u32(x: u32) {}
///
/// async fn foo() {
/// let x = make_u32();
/// take_u32(x);
/// }
/// ```
///
/// This routine checks if the found type `T` implements `Future<Output=U>` where `U` is the
/// expected type. If this is the case, and we are inside of an async body, it suggests adding
/// `.await` to the tail of the expression.
fn suggest_await_on_expect_found(
&self,
cause: &ObligationCause<'tcx>,
exp_span: Span,
exp_found: &ty::error::ExpectedFound<Ty<'tcx>>,
diag: &mut DiagnosticBuilder<'tcx>,
) {

although there are some other places that generate similar messages (e.g., here).

@nikomatsakis
Copy link
Contributor

This in turn is invoked from

pub fn note_type_err(
&self,
diag: &mut DiagnosticBuilder<'tcx>,
cause: &ObligationCause<'tcx>,
secondary_span: Option<(Span, String)>,
mut values: Option<ValuePairs<'tcx>>,
terr: &TypeError<'tcx>,
) {

@nikomatsakis
Copy link
Contributor

This could be a bit difficult because the setup is currently very oriented at adding helps and notes to an existing error (diag). To generate the error the way I wanted it would require "resetting' diag to a whole different error pattern, probably by detecting the situation here instead:

pub fn report_and_explain_type_error(
&self,
trace: TypeTrace<'tcx>,
terr: &TypeError<'tcx>,
) -> DiagnosticBuilder<'tcx> {
debug!("report_and_explain_type_error(trace={:?}, terr={:?})", trace, terr);

although note_type_err is also called from other places, like here.

@nikomatsakis
Copy link
Contributor

You'll have to dig a bit through the code to see just how hard it is to decide when the await message applies, although you can see from the OP that even if the types aren't exactly equal, there's probably a better error to be given when one type is a future and the other is not.

I think if I were going to do this I might keep the logic in note_type_err, but also have report_and_explain_type_error discover the situation and report a different sort of error, although it'd be good to make some examples where note_type_err' logic still triggers.

@nikomatsakis
Copy link
Contributor

Not sure @nellshamrell if that helps at all =)

@nellshamrell
Copy link
Contributor

FINALLY have some time to devote to this. Thank you so much for the pointers, @nikomatsakis, that helps a lot!

@nellshamrell
Copy link
Contributor

nellshamrell commented May 31, 2021

Repro steps

Adding some repro steps for my own reference (and for anyone else!)

$ cargo new troubleshooting_example
$ cd troubleshooting_example

Head into src/lib.rs and change to:

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

Actual behavior

$ cargo build

error[E0308]: mismatched types
 --> src/lib.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> u8 {  22 }
  |                            -- the `Output` of this `async fn`'s found opaque type
3 |     
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note:     expected type `u8`
          found opaque type `impl std::future::Future`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
error: could not compile `troubleshooting_example`.

To learn more, run the command again with --verbose.

Desired behavior

$ cargo build

error[E0308]: missing await on future
 --> src/main.rs:8:12
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> u8 {  22 }
  |     ----- calling an async function returns a future
3 |     
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found future
  |
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ^^^^^^

error: aborting due to previous error

@nellshamrell
Copy link
Contributor

Notes

Relevant files/functions

compiler/rustc_infer/src/infer/error_reporting/mod.rs

add_labels_for_types

This function adds some wording async errors (this is where I focused in #82165)

            fn add_labels_for_types(
                &self,
                err: &mut DiagnosticBuilder<'_>,
                target: &str,
                types: &FxHashMap<TyCategory, FxHashSet<Span>>,
            ) {
                for (key, values) in types.iter() {
                    let count = values.len();
                    let kind = key.descr();
                    let mut returned_async_output_error = false;
                    for sp in values {
                        err.span_label(
                            *sp,
                            format!(
                                "{}{}{} {}{}",
                                if sp.is_desugaring(DesugaringKind::Async)
                                    && !returned_async_output_error
                                {
                                    "checked the `Output` of this `async fn`, "
                                } else if count == 1 {
                                    "the "
                                } else {
                                    ""
                                },
                                if count > 1 { "one of the " } else { "" },
                                target,
                                kind,
                                pluralize!(count),
                            ),
                        );
                        if sp.is_desugaring(DesugaringKind::Async)
                            && returned_async_output_error == false
                        {
                            err.note("while checking the return type of the `async fn`");
                            returned_async_output_error = true;
                        }
                    }
                }
            }
        }

suggest_await_on_expect_found

This appears to generate the help text in the relevant async error from the example, particularly this section:

            (_, Some(ty)) if ty::TyS::same_type(exp_found.expected, ty) => {
                let span = match cause.code {
                    // scrutinee's span
                    ObligationCauseCode::Pattern { span: Some(span), .. } => span,
                    _ => exp_span,
                };
                diag.span_suggestion_verbose(
                    span.shrink_to_hi(),
                    "consider `await`ing on the `Future`",
                    ".await".to_string(),
                    Applicability::MaybeIncorrect,
                );
            }

note_type_err

This function calls suggest_await_on_expect_found

        if let Some(exp_found) = exp_found {
            self.suggest_as_ref_where_appropriate(span, &exp_found, diag);
            self.suggest_accessing_field_where_appropriate(cause, &exp_found, diag);
            self.suggest_await_on_expect_found(cause, span, &exp_found, diag);
        }

@nikomatsakis
Copy link
Contributor

@nellshamrell sounds good-- let me know if I can help

@nellshamrell
Copy link
Contributor

nellshamrell commented Jun 7, 2021

I've completed a partial fix for this at this time.

When running the above example, I now get this output:

nell@DESKTOP-VBH9MU6:~/rust/troubleshooting_example$ rustc +rust-stage-1 src/lib.rs --edition 2018
error[E0308]: mismatched types
 --> src/lib.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
2 |     async fn async_fn() -> u8 {  22 }
  |                            -- calling an async function returns a future
3 |
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
  = note: while checking the return type of the `async fn`
  = note:     expected type `u8`
          found opaque type `impl Future`
help: consider `await`ing on the `Future`
  |
4 |     async_fn().await
  |               ^^^^^^

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0308, E0601.
For more information about an error, try `rustc --explain E0308`.

Seeing if I can further improve it.

@nellshamrell
Copy link
Contributor

I'm trying to figure out where in the code "^^^^^^^^^^ expected u8, found opaque type" is generated.

@nikomatsakis
Copy link
Contributor

@nellshamrell there is some generic machinery to manage type mismatches, I guess it's there somewhere

@nellshamrell
Copy link
Contributor

I've opened a PR with at least a partial fix for this issue: #86705

@eholk
Copy link
Contributor

eholk commented Nov 21, 2022

Is this still an issue?

The current output for the test case (playground) is:

Compiling playground v0.0.1 (/playground)
error[[E0308]](https://doc.rust-lang.org/stable/error-index.html#E0308): mismatched types
 --> src/lib.rs:4:5
  |
1 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
...
4 |     async_fn()
  |     ^^^^^^^^^^ expected `u8`, found opaque type
  |
note: while checking the return type of the `async fn`
 --> src/lib.rs:2:28
  |
2 |     async fn async_fn() -> u8 {  22 }
  |                            ^^ checked the `Output` of this `async fn`, 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
  |               ++++++

For more information about this error, try `rustc --explain E0308`.
error: could not compile `playground` due to previous error

which looks pretty close to the desired output listed above.

@vincenzopalazzo
Copy link
Member

@rustbot claim

I will take a look as discussed in one of the async team meeting

vincenzopalazzo added a commit to vincenzopalazzo/rust that referenced this issue Feb 13, 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 <[email protected]>
@bors bors closed this as completed in 73b022b Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-await Area: Async & Await A-diagnostics Area: Messages for errors, warnings, and lints AsyncAwait-Triaged Async-await issues that have been triaged during a working group meeting. C-enhancement Category: An issue proposing an enhancement or a PR with one. D-confusing Diagnostics: Confusing error or lint that should be reworked. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants