Add a lint for an async block/closure that yields a type that is itself awaitable. #5909
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @flip1995 (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
|
r? @yaahc bc you like async stuff iirc |
|
☔ The latest upstream changes (presumably #5894) made this pull request unmergeable. Please resolve the merge conflicts. |
khuey
force-pushed
the
async_yields_async
branch
2 times, most recently
from
9d687ee
to
3f1d5ee
Compare
khuey
force-pushed
the
async_yields_async
branch
from
1a27ec1
to
b060058
Compare
| let _n = async || custom_future_type_ctor(); | ||
| let _o = async || f(); |
There was a problem hiding this comment.
why aren't these two triggering the lint here? I can understand that their definitions shouldn't trigger the lint but these usages seem like they should.
There was a problem hiding this comment.
This is an example of the limitations of having to match a zillion AST types to deduce return_expr_span :(
There's no case for these so they don't warn.
There was a problem hiding this comment.
oh, do you mean the XXXhuey bit, as in, this doesn't resolve to either a Block or a Path?
There was a problem hiding this comment.
yea it feels like there has to be a way to handle this...
|
☔ The latest upstream changes (presumably #5952) made this pull request unmergeable. Please resolve the merge conflicts. |
…lf awaitable.
This catches bugs of the form
tokio::spawn(async move {
let f = some_async_thing();
f // Oh no I forgot to await f so that work will never complete.
});
khuey
force-pushed
the
async_yields_async
branch
from
b060058
to
04912ca
Compare
|
Given that nobody knows a better way forward with |
|
Seems fair. @yaahc ? |
|
sounds good to me @bors r+ |
|
📌 Commit 04912ca has been approved by |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
more improvementthis code should still be avoid: tokio::spawn(async move {
some_async_thing().await
});if tokio::spawn(some_async_thing());correct me if I'm wrong |
This catches bugs of the form
tokio::spawn(async move {
let f = some_async_thing();
f // Oh no I forgot to await f so that work will never complete.
});
See the two XXXkhuey comments and the unfixed
_lstructure for things that need more thought.Please keep the line below
changelog: none