macros: improve diagnostics on type mismatch

[taiki-e]

By adjusting the span of some tokens, improve diagnostics on type mismatch

Unfortunately, with this approach, only the nightly compiler can benefit from complete error messages until feature(proc_macro_span) is stabilized (on the stable compiler, some error messages will probably only point to the first token of the statement that has the problem), and it is not very robust, as it can be affected if the span pointed by the error message is significantly changed. Also, some of the help messages still point to the wrong span. That said, these issues are not uncommon in diagnostics of proc-macro, and I think this would be enough improvement in the common case.

This replaces #3039. Thanks @Aaron1011 for finding this issue and being the first to work on the fix.

---

Code:

#[tokio::main]
async fn missing_semicolon_or_return_type() {
    Ok(())
}

Before (current master):

error[E0308]: mismatched types
 --> src/lib.rs:1:1
  |
1 | #[tokio::main]
  | ^^^^^^^^^^^^^^ expected `()`, found enum `Result`
  |
  = note: expected unit type `()`
                  found enum `Result<(), _>`
  = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

After (this PR):

error[E0308]: mismatched types
 --> $DIR/macro_type_mismatch.rs:5:5
  |
5 |     Ok(())
  |     ^^^^^^ expected `()`, found enum `Result`
  |
  = note: expected unit type `()`
                  found enum `Result<(), _>`
help: consider using a semicolon here
  |
5 |     Ok(());
  |           ^
help: try adding a return type
  |
4 | async fn missing_semicolon_or_return_type() -> Result<(), _> {
  |                                             ^^^^^^^^^^^^^^^^

Both help messages are correct.

---

Code:

#[tokio::main]
async fn missing_return_type() {
    return Ok(());
}

Before (current master):

error[E0308]: mismatched types
 --> src/lib.rs:1:1
  |
1 | #[tokio::main]
  | ^^^^^^^^^^^^^^ expected `()`, found enum `Result`
  |
  = note: expected unit type `()`
                  found enum `Result<(), _>`
  = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)

After (this PR):

error[E0308]: mismatched types
  --> $DIR/macro_type_mismatch.rs:16:5
   |
16 |     return Ok(());
   |     ^^^^^^^^^^^^^^ expected `()`, found enum `Result`
   |
   = note: expected unit type `()`
                   found enum `Result<(), _>`
help: consider using a semicolon here
   |
16 |     return Ok(());;
   |                   ^
help: try adding a return type
   |
9  | async fn missing_return_type() -> Result<(), _> {
   | 

The first help message still wrong.
The second help message is correct.

---

Code:

#[tokio::main]
async fn extra_semicolon() -> Result<(), ()> {
    Ok(());
}

Before (current master):

error[E0308]: mismatched types
 --> src/lib.rs:1:1
  |
1 | #[tokio::main]
  | ^^^^^^^^^^^^^^ expected enum `Result`, found `()`
2 | async fn extra_semicolon() -> Result<(), ()> {
  |                               -------------- expected `Result<(), ()>` because of return type
  |
  = note:   expected enum `Result<(), ()>`
          found unit type `()`
  = note: this error originates in an attribute macro (in Nightly builds, run with -Z macro-backtrace for more info)
help: try using a variant of the expected enum
  |
1 | Ok(#[tokio::main])
  |
1 | Err(#[tokio::main])
  |

After (this PR):

error[E0308]: mismatched types
  --> $DIR/macro_type_mismatch.rs:29:5
   |
20 | async fn extra_semicolon() -> Result<(), ()> {
   |                               -------------- expected `Result<(), ()>` because of return type
...
29 |     Ok(());
   |     ^^^^^^^ expected enum `Result`, found `()`
   |
   = note:   expected enum `Result<(), ()>`
           found unit type `()`
help: try using a variant of the expected enum
   |
29 |     Ok(Ok(());)
   |
29 |     Err(Ok(());)
   |

The help message still wrong.

---

Closes #3039
Fixes rust-lang/rust#71968

[taiki-e]

Also, it seems dead_code warnings now work properly by this patch.

error: function is never used: `unused_braces_main`
  --> tokio/tests/macros_test.rs:24:10
   |
24 | async fn unused_braces_main() { println!("hello") }
   |          ^^^^^^^^^^^^^^^^^^
   |
   = note: `-D dead-code` implied by `-D warnings`

https://github.com/tokio-rs/tokio/runs/2535370441

[Darksonn]

I understand that #3039 had some MSRV hazards. What about this PR?

[taiki-e]

@Darksonn I'm not sure which issue you are talking about. However, unlike #3039, this only changes the span of the generated code, not the contents of the generated code.

[Darksonn]

Ah, I was thinking of #3583.

[Darksonn]

This seems reasonable to me.

One question though: Why not define two actual functions and have one call the other?

[taiki-e]

@Darksonn

Ah, I was thinking of #3583.

As far as I know, the change to macro itself by #3583 doesn't actually cause any problems about MSRV. The problem about MSRV in #3583 is that some of the tests #3583 adds cannot compile on older compilers. However, those tests cannot compile on older compilers with or without #3583's changes.

One question though: Why not define two actual functions and have one call the other?

Do you mean I change the test as follows? If so, I'm not sure what the benefits of doing it are...

#[tokio::main]
async fn missing_semicolon_or_return_type() {
    missing_return_type()
}
#[tokio::main]
async fn missing_return_type() -> Result<(), ()> {
    missing_semicolon_or_return_type()
}

---

only the nightly compiler can benefit from complete error messages until feature(proc_macro_span) is stabilized (on the stable compiler, some error messages will probably only point to the first token of the statement that has the problem)

I fixed this limitation in e1442a4. Both nightly and stable compilers now output error messages of almost the same quality.

[Darksonn]

No I didn't mean change the test. I meant change the output to something like this

async fn real_main() -> ... {
    ...
}
fn main() -> ... {
   Runtime::new().block_on(real_main())
}

[taiki-e]

@Darksonn

No I didn't mean change the test. I meant change the output to something like this

Ah, thanks for clarifying. As said in #3039 (review), that approach causes a breaking change:

Unfortunately, this approach (split an async fn to two async or normal fns) causes a breaking change because #[tokio::main] accepts arguments. The self argument can be handled in the same way as async-trait, but we know that some regressions occur. (see dtolnay/async-trait#122 (comment))

For example, the current tokio::main can be used in the trait method.
It would not be possible to implement the approach you mentioned without causing breakage in such code:

• First, we cannot adds new methods to the same scope, so the generated code will be:

  fn f(...) -> ... {
     async fn real_f(...) -> ... { ... }
     Runtime::new().block_on(real_f(...))
  }

  instead of:

  async fn real_f(...) -> ... { ... }
  fn f(...) -> ... {
     Runtime::new().block_on(real_f(...))
  }

• Next, we need to handle self in async fn real_f(...), but, we cannot get the actual type of Self from the function signature.

If we remove support for the trait method, we may be able to implement that approach, but it is a breaking change anyway.

UPDATE: added test case for trait method to prevent accidental breakage (63ee506)

[Darksonn]

Alright. I have approved it. Feel free to merge it whenever you feel it is ready.