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: better spans in #[tokio::main] output #5619

Closed
wants to merge 9 commits into from

Conversation

evertedsphere
Copy link

@evertedsphere evertedsphere commented Apr 12, 2023

Fix #5518.

Motivation

The #[tokio::main] macro in its current state produces output that degrades IDE functionality — code generated by the proc macro is annotated with spans that point back to the end of the (user-provided) contents of async fn main, resulting in IDE plugins (correctly) treating the last binding in main as a call to a Tokio function.

As an example, if one adds a typo to the generated code, the error produced on master points to user code:

error[E0599]: no method named `enable_al` found for struct `tokio::runtime::Builder` in the current scope
 --> src/main.rs:5:13
  |
5 |     variable;
  |             ^ help: there is a method with a similar name: `enable_all`

Solution

We replace the span attached to the Tokio-generated code with one that points to the invocation site of the #[tokio::main] macro itself:

error[E0599]: no method named `enable_al` found for struct `tokio::runtime::Builder` in the current scope
 --> src/main.rs:1:1
  |
1 | #[tokio::main]
  | ^^^^^^^^^^^^^^ help: there is a method with a similar name: `enable_all`

@evertedsphere
Copy link
Author

There are a few other quote! invocations in parse_knobs that also point back to user code; perhaps it's a good idea to also make the same change for those?

@evertedsphere
Copy link
Author

evertedsphere commented Apr 13, 2023

Looks like my patch breaks errors which should point to user code, e.g. when the body of async fn main is an expression that returns the wrong type, the following

error[E0308]: mismatched types
  --> $DIR/macros_type_mismatch.rs:32:5
   |
30 | async fn issue_4635() {
   |                       - help: try adding a return type: `-> i32`
31 |     return 1;
32 |     ;
   |     ^ expected `()`, found integer

becomes


error[E0308]: mismatched types
  --> tests/fail/macros_type_mismatch.rs:29:1
   |
29 | #[tokio::main]
   | ^^^^^^^^^^^^^^ expected `()`, found integer
30 | async fn issue_4635() {
   |                       - help: try adding a return type: `-> i32`

This can be fixed by asserting the right type for the body earlier within a span that points back to user code.


One thing I've tried is changing the quote! here to a quote_spanned!:

quote! {
let body = async #body;
}

and annotating the type there via let body: ??? = async #body, so that typechecking fails there instead of inside the block_on.

However, I can't figure out what type to put there: rustc complains about my variations on &mut dyn Future<..> not being Unpin. If I use the code in the is_test branch with the Unpin, it works just fine and produces decent spans (although there's some noise because the type mismatch occurs at a pinned future type instantiated at the output type, rather than at the output type itself):

error[E0308]: mismatched types
  --> $DIR/macros_type_mismatch.rs:32:5
   |
30 | async fn issue_4635() {
   |                       - help: try adding a return type: `-> i32`
31 |     return 1;
32 |     ;
   |     ^ expected `()`, found integer

is what you get on master, which becomes

error[E0271]: expected `[async block@$DIR/tests/fail/macros_type_mismatch.rs:30:23: 33:2]` to be a future that resolves to `()`, but it resolves to `{integer}`
  --> tests/fail/macros_type_mismatch.rs:32:5
   |
32 |     ;
   |     ^ expected `()`, found integer
   |
   = note: required for the cast from `[async block@$DIR/tests/fail/macros_type_mismatch.rs:30:23: 33:2]` to the object type `dyn Future<Output = ()>`

@evertedsphere
Copy link
Author

evertedsphere commented Apr 13, 2023

I figured it out: doing

let body = async { let x: #output_type = body; x };

solves the problem — although it does complain about unreachable code when body is, say, return Ok(());.

@evertedsphere
Copy link
Author

I think this is ready for review.

Remaining issues:

  1. I'll need some help with this test that calls #[tokio::test] inside an MBE macro:
error[E0425]: cannot find value `body` in this scope
  --> tokio/tests/macros_test.rs:79:13
   |
79 |               #[::tokio::test]
   |               ^^^^^^^^^^^^^^^^ not found in this scope
...
85 | /     mac!(
86 | |         async fn foo() {}
87 | |     );
   | |_____- in this macro invocation
   |

I'm clearly doing something wrong around identifiers/quoting here, but I can't spot the issue.

  1. Not sure what to do for
error[E0658]: the `!` type is experimental
  --> tokio/tests/macros_test.rs:38:37
   |
38 | pub async fn issue_4175_main_1() -> ! {
   |                                     ^
   |

since the type is already there in the test's source. (I suppose annotating a value as x: ! isn't supported?)

  1. This abomination:
  --> tests/fail/macros_type_mismatch.rs:10:12
   |
10 |     return Ok(());
   |            ^^^^^^
help: try wrapping the expression in a variant of `Result`
   |
10 |     return Ok(())Ok(;)
   |                  +++ +
10 |     return Ok(())Err(;)
   |                  ++++ +

@evertedsphere
Copy link
Author

I'm unable to run tests etc on the MSRV (1.56.0 currently) because I get:

$ TRYBUILD=overwrite cargo +1.56.0 test -p tests-build  --all-features -j4
    Updating crates.io index
error: failed to select a version for the requirement `predicates = "=2.1.5"`
candidate versions found which didn't match: 2.1.1, 2.1.0, 2.0.3, ...
location searched: crates.io index
required by package `mockall v0.11.4`
    ... which satisfies dependency `mockall = "=0.11.4"` of package `tokio v1.27.0 (/home/k/c/tokio/tokio)`

@taiki-e
Copy link
Member

taiki-e commented Apr 15, 2023

Thanks for the PR.

  1. I'll need some help with this test that calls #[tokio::test] inside an MBE macro:

See #5244, which fixed the same issue in the past.

error[E0658]: the ! type is experimental

If the return type contains an unstable type such as impl trait or never type, the type annotation needs to be omitted.
See also #3039 (comment).

I'm unable to run tests etc on the MSRV (1.56.0 currently) because I get:

This is due to rust-lang/cargo#10623 (predicates uses weak and namespaced features).

@Darksonn Darksonn added the A-tokio-macros Area: The tokio-macros crate label Apr 15, 2023
@Darksonn
Copy link
Contributor

I just merged #5621, which conflicts with your PR. Sorry about that.

@Darksonn
Copy link
Contributor

Darksonn commented May 8, 2023

Would you like to continue this PR?

@Darksonn
Copy link
Contributor

Darksonn commented Jun 4, 2023

Superseded by #5762.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-macros Area: The tokio-macros crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#[tokio::main] proc macro attaches questionable spans to user code
3 participants