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

Improve diagnostics for tokio::main and tokio::test attributes #6882

Closed
wants to merge 1 commit into from

Conversation

Veykril
Copy link
Contributor

@Veykril Veykril commented Oct 3, 2024

Motivation

Motivation is to fix #5518, by no longer attaching questionable spans to tokens for improved diagnostics.

Solution

Turns out we can further improve diagnostics while not having to attach tail expression spans to tokens, bringing the diagnostics closer to what rustc reports for non tokio attributed async functions by just re-emitting a local version of the annotated function instead of using an async block. Compare the test output with the output of the non attributed versions (playground link for those)

They are now equal.

Closes #5518

@Veykril Veykril marked this pull request as draft October 3, 2024 13:27
@Veykril
Copy link
Contributor Author

Veykril commented Oct 3, 2024

Ah I wasn't aware this worked on associated functions

@Veykril
Copy link
Contributor Author

Veykril commented Oct 3, 2024

I also now see this was tried before in #3039 (review) reading up on that now

Edit: I see, this runs risk of changing drop order in theory. An async blocks drop order is dependent on the order it references its captures by, where as for async function it is the parameter order. So for parameterless (and unary) functions this change is fine, but for more parameters it can potentially change the drop order, ... how annoying. Interestingly enough that also means that #[tokio::main] functions have a differing drop order from vanilla async functions.

@Veykril
Copy link
Contributor Author

Veykril commented Oct 3, 2024

I'll close this PR then, unfortunate but I don't think there is a way to get around that. Might be good to note this as a potential change for a future 2.0 release if that ever is going to happen though, as the difference in drop order can be surprising I reckon.

@Veykril Veykril closed this Oct 3, 2024
@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

As for drop order, doing the similar thing as async-trait did (dtolnay/async-trait#143) should fix it without reducing the number of use cases supported as this PR does.

@Veykril
Copy link
Contributor Author

Veykril commented Oct 3, 2024

But that is going from an async fn in the expansion to an async block, fixing drop order there is simple as you can just emit usages in order at the start of the block. Going from a block to an async fn as is done here doesn't have such a solution (as you can't tell the order of captures of an async block syntactically).

Unless I am misunderstanding that PR (its quite a massive diff)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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