Nellshamrell/improve async error

[nellshamrell]

This is at least a partial fix for #80658

Prior to this fix, when someone attempted to compile this code:

fn main() {

}

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

    async_fn()
}

The would receive this output:

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`

If this change is merged, someone compiling the same code will then get this output instead:

error[E0308]: mismatched types
 --> src/lib.rs:8:5
  |
5 | fn foo() -> u8 {
  |             -- expected `u8` because of return type
6 |     async fn async_fn() -> u8 {  22 }
  |                            -- calling an async function returns a future
7 |
8 |     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`
  |
8 |     async_fn().await
  |               ^^^^^^

error: aborting due to previous error

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

[rust-highfive]

r? @varkor

(rust-highfive has picked a reviewer for you, use r? to override)

[varkor]

Changes look good to me. Just have a small tweak for the diagnostics.

[Aaron1011]

This looks like the diagnostic is saying that Test is a future. Could we instead say something like 'an async function returns a future that outputs the written return type'?

[varkor]

Maybe we could highlight the entire function signature and say something like:

this async function will return an `impl Future<Output = Test>`

[Aaron1011]

To better support macros, I think we should keep pointing to the return type, but mention the function name in the error message. This will produce a nicer message when the return type tokens are constructed separately (e.g. a type passed into a macro_rules! macro).

[nellshamrell]

Thank you for the suggestion - any pointers on how to surface the function name in the error message?

[varkor]

It doesn't look conveniently accessible at the moment. Maybe @estebank has a suggestion on the cleanest way to get the name of the desugared async function?

[estebank]

In /~https://github.com/rust-lang/rust/pull/86705/files#diff-845d26194de711d60e1317dbe90812d5d7635f4f758222fd7e14bf550ffe895dL1522 we have the DefId of the opaque type. With it I think you can get the def id of the parent fn, and call def_path on that, but I don't have the codebase at hand now to be more specific. I can help you nell either tomorrow or on thursday.

[nellshamrell]

Thanks for the review! Finally turning my attention back to this today :)

[nellshamrell]

@varkor - I've made a small change to the error message to fit with your suggestion. I'm also intrigued by the idea of surfacing the function name in the error message, but could use some help understanding how I might implement that (a pointer to similar areas of the code would be most welcome!).

[estebank]

Love the new output! It is certainly better.

I wonder if we should (maybe in the future?) give output similar to "async function make_u32 returns an impl Future<Output = u32>". I know I've pushed for that in the past and not everyone is happy with that, but I fear that we are not taking advantage of teaching people about what "futures" are at the type level. Having said that, we could reserve a more "accurate" phrasing for specific cases (like when we suggest appending .await to a function call), and maybe make it part of a note.

[estebank]

I'd forgotten I had #87673 open that touches the same general place, mainly moving the span_label to a span_note using a MultiSpan to have more text. I wonder if instead of landing my PR you could incorporate some of the changes I made?

I just spent a few minutes trying to accomplish what you mentioned (use the name of the method) and this is what I landed at, which I feel looks "pretty neat", but I want us to go for "easy to understand by newcomers":

error[E0308]: mismatched types
  --> src/test/ui/async-await/dont-suggest-missing-await.rs:14:18
   |
14 |         take_u32(x)
   |                  ^ expected `u32`, found opaque type
   |
note: while checking the return type of the async function `make_u32`
  --> src/test/ui/async-await/dont-suggest-missing-await.rs:7:24
   |
7  | async fn make_u32() -> u32 {
   |          --------      ^^^ the desugared version of `make_u32` returns `impl Future<Output = u32>`
   |          |
   |          async functions return futures
   = note:     expected type `u32`
           found opaque type `impl Future`
help: consider `await`ing on the `Future`
   |
14 |         take_u32(x.await)
   |                   ^^^^^^

I'll push the code to #87673 in a minute and you can tell me if you can pick from it so you can run with it and make it your own if there are more things we want to do here :) (because I'm not satisfied with the wording I threw in there, but the code to get the spans and names is still valid).

[estebank]

Oh! While I was getting this working, the PR got approved and selected in a rollup (otherwise I'd kill it). We'll have to rebase this PR after that lands, sorry about that :-/

The follow up code the above screenshot came from is at bfcaf8b. Would you have the time/desire to take that and make it the best output it can be?

[nellshamrell]

Sure! I likely won't have time until next weekend, but I would be happy to! I'll likely open a new PR with that change.

[nellshamrell]

And ty so much for the help!

[bors]

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