Make inline attributes apply to the generated poll in async fn#149245
Make inline attributes apply to the generated poll in async fn#149245xacrimon wants to merge 1 commit into
poll in async fn#149245Conversation
|
rustbot has assigned @JonathanBrouwer. Use |
This comment has been minimized.
This comment has been minimized.
46d6c3e to
d89022a
Compare
This comment has been minimized.
This comment has been minimized.
9478bfd to
fa35f7b
Compare
This comment has been minimized.
This comment has been minimized.
|
@rustbot author |
|
Reminder, once the PR becomes ready for a review, use |
|
☔ The latest upstream changes (presumably #149853) made this pull request unmergeable. Please resolve the merge conflicts. |
|
☔ The latest upstream changes made this pull request unmergeable. Please resolve the merge conflicts. |
|
Triage: Are you still working on this @xacrimon? |
… instead of the desugared creator function.
fa35f7b to
7054408
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
☔ The latest upstream changes (presumably #156473) made this pull request unmergeable. Please resolve the merge conflicts. |
| let filtered_attrs = self.arena.alloc_from_iter(filtered_iter); | ||
|
|
||
| if filtered_attrs.is_empty() { | ||
| self.attrs.remove(&outer_hir_id.local_id); |
There was a problem hiding this comment.
Is this code reachable with the early-return above?
| if filtered_attrs.is_empty() { | ||
| self.attrs.remove(&outer_hir_id.local_id); | ||
| } else { | ||
| self.attrs.insert(outer_hir_id.local_id, filtered_attrs); |
There was a problem hiding this comment.
Why are we overriding the attrs from outer_hir_id here?
| ); | ||
|
|
||
| this.maybe_forward_track_caller(body.span, closure_hir_id, expr.hir_id); | ||
| this.forward_inline(body.span, closure_hir_id, expr.hir_id); |
There was a problem hiding this comment.
Could you add some tests for this feature?
There was a problem hiding this comment.
From the lang team decision:
We talked about this in the lang call today. We want attributes like this to apply to "where the code is", i.e. to the poll function in this case. The wrapper function, on the other hand, should always be inlined.
Make sure your tests cover this.
I suspect the wrapper function is already marked as inline somewhere, if it is not could you or someone else open a separate PR that does this?
|
Sorry for taking so long to review this PR! This PR fell of my radar, thanks @Dylan-DPC for fixing up the label so I saw it again |
This PR adds lowering code (similar to how track_caller forwarding works) to
async fnitems andasync ||closures such that any inline attributes put on them are inherited by the generated coroutine (corresponding to thepoll()implementation) and are not applied to the outer function. This behavior matches the accepted FCP in the first linked issue.Code like the following now correctly codegen 3 functions
async_fn_test::consumerasync_fn_test::hi::{closure#0}core::ptr::drop_in_place::<async_fn_test::hi::{closure#0}>Fixes #129347 and fixes #106765.
This has already been FCP'ed here: #129347