-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Prereq1 for async drop - drop
& async_fut
fields in Drop terminator
#129734
base: master
Are you sure you want to change the base?
Prereq1 for async drop - drop
& async_fut
fields in Drop terminator
#129734
Conversation
Some changes occurred to MIR optimizations cc @rust-lang/wg-mir-opt Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri This PR changes MIR cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras |
/// StateTransform pass. In `expand_async_drops` async drops are expanded | ||
/// into one or two yield points with poll ready/pending switch. | ||
/// When a coroutine has any internal async drop, the coroutine drop function will be async | ||
/// (generated by `create_coroutine_drop_shim_async`, not `create_coroutine_drop_shim`). |
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.
It might be worth also discussing async drop in the MirPhase docs. And if any new MIR phase restrictions are added, please make sure to update the MIR validator.
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.
Comment about async drops added in MirPhase:
/// - Async drops: after drop elaboration some drops may become async (`drop`, `async_fut` fields).
/// StateTransform pass will expand those async drops or reset to sync.
Could you, pls, be more specific about "update the MIR validator"? Async drop terminators live from ElaborateDrops
to StateTransform
pass in transition to MirPhase::Runtime(..)
. What MIR validator changes do you recommend?
1cffd92
to
482c2c3
Compare
b45015f
to
f954f0c
Compare
☔ The latest upstream changes (presumably #130724) made this pull request unmergeable. Please resolve the merge conflicts. |
r? @nikomatsakis is going to look into this |
f954f0c
to
d5844bf
Compare
This comment has been minimized.
This comment has been minimized.
d5844bf
to
e8f28c1
Compare
☔ The latest upstream changes (presumably #136751) made this pull request unmergeable. Please resolve the merge conflicts. |
e8f28c1
to
7097c0c
Compare
☔ The latest upstream changes (presumably #137030) made this pull request unmergeable. Please resolve the merge conflicts. |
7097c0c
to
e448c13
Compare
This comment has been minimized.
This comment has been minimized.
e448c13
to
6109ee3
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
This is subpart 1 PR of #123948, just for review purposes.
Added Option
drop
andasync_fut
fields in Drop terminator for use when Drop terminator is async drop.drop
field has the same meaning asdrop
field in Yield terminator. We need it because async drop is a hidden Yield, expanded into yield-point in StateTransform (later commits).async_fut
contains local for async drop future, generated byasync_fut = async_drop_in_place(droppee)
in drop elaboration (later commits).