-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
[wg-async-await] Drop async fn
arguments in async block
#59135
Conversation
I'm not completely happy with the way this works, but I couldn't work out anything better. Happy to make any changes or completely rethink the approach if preferred. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There's a summary of the current state of this PR on Zulip. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
)); | ||
let _ = fut.as_mut().poll(&waker); | ||
assert_eq!(*af.borrow(), &[ | ||
Function, Val("_y"), Val("_c"), Val("a"), Val("x"), Val("_"), Val("_"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should name these _1
etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to have the names here match the function arguments as I found that was easier to reason about when there was a failure. So, _1
could be used for the argument names, but I'd still need to use something non-numeric for the bindings that don't start with _
which would be inconsistent.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This tests that async functions drop parameters in the same order as regular functions.
This will be used to keep track of the origin of a local in the AST. In particular, it will be used by `async fn` lowering for the locals in `let <pat>: <ty> = __arg0;` statements.
This commit adds an `AsyncArgument` struct to the AST that contains the generated argument and statement that will be used in HIR lowering, name resolution and def collection.
This commit takes advantage of `AsyncArgument` type that was added in a previous commit to replace the arguments of the `async fn` in the HIR and add statements to move the bindings from the new arguments to the pattern from the old argument. For example, the async function `foo` below: async fn foo((x, _y): (T, V)) { async move { } } becomes: async fn foo(__arg0: (T, V)) { async move { let (x, _y) = __arg0; } }
This commit extends the previous commit to apply to trait methods as well as free functions.
This avoids issues with `impl_trait_in_bindings` as the type from the argument is normally used as the let binding, but `impl Trait` is unstable in binding position.
This commit introduces an `ArgSource` enum that is lowered into the HIR so that diagnostics can correctly refer to the argument pattern's original name rather than the generated pattern.
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Ping from triage, @davidtwco, you have some test failures. |
Thanks @Centril, I noticed, this PR is currently waiting on some feedback from wg-async-await about the approach and some pointers on fixing the current test failures. |
Closing this until there is bandwidth to review. |
Re-opening as #59823 after wg-async-await weekly meeting - apparently you can't re-open PRs that you've force-pushed too. |
Fixes #54716.
This PR modifies the HIR lowering (and some other places to make this work) so that unused arguments to a async function are always dropped inside the
async move
block and not at the end of the function body.However, the exact ordering of drops is not the same as a regular function, as visible in this playground example - I believe this to be an unrelated issue. There is a Zulip topic for this.
r? @cramertj
cc @nikomatsakis