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

Async drop codegen #123948

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Async drop codegen #123948

wants to merge 12 commits into from

Conversation

azhogin
Copy link
Contributor

@azhogin azhogin commented Apr 15, 2024

Async drop implementation using templated coroutine for async drop glue generation.
Implementation details:
https://github.com/azhogin/posts/blob/main/async-drop-impl.md

Implementation is splitted into 12 parts for review purposes:
1). New fields in Drop terminator (drop & async_fut). Processing in codegen/miri must validate that those fields are empty (in full version async Drop terminator will be expanded at StateTransform pass or reverted to sync version). Changes in terminator visiting to consider possible new successor (drop field).
#129734
2). ResumedAfterDrop messages for panic when coroutine is resumed after it is started to be async drop'ed.
#129736
3). Lang item for generated coroutine for async function async_drop_in_place. async fn async_drop_in_place<T>()::{{closure0}}.
#129737
4). is_async_drop_raw and small needs_async_drop fixes and previous async drop glue implementation cleanup.
#129739
5). Instances and resolving. AsyncDropGlueCtorShim for async_drop_in_place() function returning drop coroutine. AsyncDropGlue for async drop coroutine. FutureDropPoll for async drop of coroutine.
#129740
6). Templated coroutine support. async fn async_drop_in_place<T>()::{{closure0}} (async drop coroutine) has completely different layout for different dropped types T, so it cannot be represented by generic coroutine, we need to implement templated coroutine with code generation after all GenericArgs substituted.
#129741
7). Library changes. async fn async_drop_in_place<T>() declaration etc.
#129742
8). Scopes processing for generate async drop preparations. Async drop is a hidden Yield, so potentially async drops require the same dropline preparation as for Yield terminators.
#129744
9). Drop elaboration.
#129745
10). Processing in StateTransform. Async drops are expanded into yield-point. Generation of async drop of coroutine itself added.
#129746
11). Shim generation. Shims for AsyncDropGlueCtorShim, AsyncDropGlue and FutureDropPoll.
#129747
12). Tests.

#[lang = "async_drop"]
pub trait AsyncDrop {
    #[allow(async_fn_in_trait)]
    async fn drop(self: Pin<&mut Self>);
}

impl Drop for Foo {
    fn drop(&mut self) {
        println!("Foo::drop({})", self.my_resource_handle);
    }
}

impl AsyncDrop for Foo {
    async fn drop(self: Pin<&mut Self>) {
        println!("Foo::async drop({})", self.my_resource_handle);
    }
}

First async drop glue implementation re-worked to use the same drop elaboration code as for sync drop.
async_drop_in_place changed to be async fn. So both async_drop_in_place ctor and produced coroutine have their lang items (AsyncDropInPlace/AsyncDropInPlacePoll) and shim instances (AsyncDropGlueCtorShim/AsyncDropGlue).

pub async unsafe fn async_drop_in_place<T: ?Sized>(_to_drop: *mut T) {
}

AsyncDropGlue shim generation uses elaborate_drops::elaborate_drop to produce drop ladder (in the similar way as for sync drop glue) and then coroutine::StateTransform to convert function into coroutine poll.

AsyncDropGlue coroutine's layout can't be calculated for generic T, it requires known final dropee type to be generated (in StateTransform). So, templated coroutine was introduced here (templated_coroutine_layout(...) etc).

Such approach overrides the first implementation using mixing language-level futures in #121801.

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2024

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot
Copy link
Collaborator

rustbot commented Apr 15, 2024

rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead.

cc @rust-lang/rust-analyzer

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

This PR changes Stable MIR

cc @oli-obk, @celinval, @ouz-a

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 15, 2024
@slanterns
Copy link
Contributor

How does this relate to #121801?

@azhogin
Copy link
Contributor Author

azhogin commented Apr 15, 2024

How does this relate to #121801?

This is a proof-of-concept of async drop implementation with only final drop function (no glue for child objects drop).
Async drop glue generation implemented in #121801. We will intergate this two PRs.

@@ -170,7 +170,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
}

Drop { place, target, unwind, replace: _ } => {
Drop { place, target, unwind, replace: _, drop: _, async_fut: _ } => {
Copy link
Member

@RalfJung RalfJung Apr 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something strange going on that's not explained in the comments in syntax.rs -- why are these things entirely ignored by the interpreter and codegen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, both drop and async_fut fields are only used in compiler/rustc_mir_transform/src/coroutine.rs, StateTransform pass. In expand_async_drops async drops are expanded into one or two yield points with poll ready/pending switch.

Drop terminator comments updated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case codegen and the interpreter should bug! when those fields are present.

Though it seems to me it would probably be better to make this a different terminator. They have very different operational behavior, after all.

target: BasicBlock,
unwind: UnwindAction,
replace: bool,
/// Cleanup to be done if the coroutine is dropped at this suspend point (for async drop).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does it mean for the coroutine to be "dropped at this suspend point"? I don't understand under which conditions the code will jump to the drop block.

Does it really make sense to make the existing Drop terminator do both regular drop and async drop? Either way, the doc comment for the terminator needs to be a self-contained explanation of what exactly happens when the terminator is executed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Async drop in StateTransform pass will be expanded into yield point for poll-loop of internal drop coroutine. So, we need the same drop target block prepared for async Drop, as for Yield terminator. It means - "where the main coroutine should go, if it is dropped at this yield point (drop pool-loop)".
StateTransform pass generates "coroutine_resume" function for poll and "coroutine_drop_async" for drop of coroutine itself. Both functions are switches: for resume function state target blocks are taken from target field of Yield (or async Drop) and for drop function state target blocks are taken from drop field of Yield (or async Drop).

Yes, I think keeping Drop terminator for both sync/async is good atm because async Drop is optionally expanded if conditions are met. For sync context (just inside normal function) and for unwind - we can't use AsyncDrop atm, so sync drop must be as a fallback.

Drop terminator comments updated.

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 17, 2024
…-obk

Add simple async drop glue generation

This is a prototype of the async drop glue generation for some simple types. Async drop glue is intended to behave very similar to the regular drop glue except for being asynchronous. Currently it does not execute synchronous drops but only calls user implementations of `AsyncDrop::async_drop` associative function and awaits the returned future. It is not complete as it only recurses into arrays, slices, tuples, and structs and does not have same sensible restrictions as the old `Drop` trait implementation like having the same bounds as the type definition, while code assumes their existence (requires a future work).

This current design uses a workaround as it does not create any custom async destructor state machine types for ADTs, but instead uses types defined in the std library called future combinators (deferred_async_drop, chain, ready_unit).

Also I recommend reading my [explainer](https://zetanumbers.github.io/book/async-drop-design.html).

This is a part of the [MCP: Low level components for async drop](rust-lang/compiler-team#727) work.

Feature completeness:

 - [x] `AsyncDrop` trait
 - [ ] `async_drop_in_place_raw`/async drop glue generation support for
   - [x] Trivially destructible types (integers, bools, floats, string slices, pointers, references, etc.)
   - [x] Arrays and slices (array pointer is unsized into slice pointer)
   - [x] ADTs (enums, structs, unions)
   - [x] tuple-like types (tuples, closures)
   - [ ] Dynamic types (`dyn Trait`, see explainer's [proposed design](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#async-drop-glue-for-dyn-trait))
   - [ ] coroutines (rust-lang#123948)
 - [x] Async drop glue includes sync drop glue code
 - [x] Cleanup branch generation for `async_drop_in_place_raw`
 - [ ] Union rejects non-trivially async destructible fields
 - [ ] `AsyncDrop` implementation requires same bounds as type definition
 - [ ] Skip trivially destructible fields (optimization)
 - [ ] New [`TyKind::AdtAsyncDestructor`](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#adt-async-destructor-types) and get rid of combinators
 - [ ] [Synchronously undroppable types](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#exclusively-async-drop)
 - [ ] Automatic async drop at the end of the scope in async context
Comment on lines 60 to 86
fn block_on<F>(fut_unpin: F) -> F::Output
where
F: Future,
{
let mut fut_pin = pin!(ManuallyDrop::new(fut_unpin));
let mut fut: Pin<&mut F> = unsafe {
Pin::map_unchecked_mut(fut_pin.as_mut(), |x| &mut **x)
};
let (waker, rx) = simple_waker();
let mut context = Context::from_waker(&waker);
let rv = loop {
match fut.as_mut().poll(&mut context) {
Poll::Ready(out) => break out,
// expect wake in polls
Poll::Pending => rx.try_recv().unwrap(),
}
};
loop {
match future_drop_poll(fut.as_mut(), &mut context) {
Poll::Ready(()) => break,
// expect wake in polls
Poll::Pending => rx.try_recv().unwrap(),
}
}
rv
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say this is a bit overblown and a single poll would have been enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shows an example of correct block_on function for async drop coroutine. I don't see a reason to optimize test here.

@zetanumbers
Copy link
Contributor

To clarify: if I understood everyone correctly this PR implements async drop with some features, which are considered to be undesirable by a number of Async WG members (regular drop after async drop was suspended, etc.) This is fine as long as we would be capable of disabling those (by restricting synchronous drop for such coroutines).

@azhogin azhogin force-pushed the azhogin/async-drop branch 2 times, most recently from e12d2c6 to cdf91ab Compare April 21, 2024 22:39
replace: _,
drop: _,
async_fut: _,
} => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should match on TerminatorKind::Drop { async_fut: Some(_), drop: Some(_), .. } in "shouldn't exist at codegen" branch, and put None in these fields in old drop case.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 23, 2024
Add simple async drop glue generation

This is a prototype of the async drop glue generation for some simple types. Async drop glue is intended to behave very similar to the regular drop glue except for being asynchronous. Currently it does not execute synchronous drops but only calls user implementations of `AsyncDrop::async_drop` associative function and awaits the returned future. It is not complete as it only recurses into arrays, slices, tuples, and structs and does not have same sensible restrictions as the old `Drop` trait implementation like having the same bounds as the type definition, while code assumes their existence (requires a future work).

This current design uses a workaround as it does not create any custom async destructor state machine types for ADTs, but instead uses types defined in the std library called future combinators (deferred_async_drop, chain, ready_unit).

Also I recommend reading my [explainer](https://zetanumbers.github.io/book/async-drop-design.html).

This is a part of the [MCP: Low level components for async drop](rust-lang/compiler-team#727) work.

Feature completeness:

 - [x] `AsyncDrop` trait
 - [ ] `async_drop_in_place_raw`/async drop glue generation support for
   - [x] Trivially destructible types (integers, bools, floats, string slices, pointers, references, etc.)
   - [x] Arrays and slices (array pointer is unsized into slice pointer)
   - [x] ADTs (enums, structs, unions)
   - [x] tuple-like types (tuples, closures)
   - [ ] Dynamic types (`dyn Trait`, see explainer's [proposed design](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#async-drop-glue-for-dyn-trait))
   - [ ] coroutines (rust-lang#123948)
 - [x] Async drop glue includes sync drop glue code
 - [x] Cleanup branch generation for `async_drop_in_place_raw`
 - [ ] Union rejects non-trivially async destructible fields
 - [ ] `AsyncDrop` implementation requires same bounds as type definition
 - [ ] Skip trivially destructible fields (optimization)
 - [ ] New [`TyKind::AdtAsyncDestructor`](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#adt-async-destructor-types) and get rid of combinators
 - [ ] [Synchronously undroppable types](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#exclusively-async-drop)
 - [ ] Automatic async drop at the end of the scope in async context
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Apr 23, 2024
Add simple async drop glue generation

This is a prototype of the async drop glue generation for some simple types. Async drop glue is intended to behave very similar to the regular drop glue except for being asynchronous. Currently it does not execute synchronous drops but only calls user implementations of `AsyncDrop::async_drop` associative function and awaits the returned future. It is not complete as it only recurses into arrays, slices, tuples, and structs and does not have same sensible restrictions as the old `Drop` trait implementation like having the same bounds as the type definition, while code assumes their existence (requires a future work).

This current design uses a workaround as it does not create any custom async destructor state machine types for ADTs, but instead uses types defined in the std library called future combinators (deferred_async_drop, chain, ready_unit).

Also I recommend reading my [explainer](https://zetanumbers.github.io/book/async-drop-design.html).

This is a part of the [MCP: Low level components for async drop](rust-lang/compiler-team#727) work.

Feature completeness:

 - [x] `AsyncDrop` trait
 - [ ] `async_drop_in_place_raw`/async drop glue generation support for
   - [x] Trivially destructible types (integers, bools, floats, string slices, pointers, references, etc.)
   - [x] Arrays and slices (array pointer is unsized into slice pointer)
   - [x] ADTs (enums, structs, unions)
   - [x] tuple-like types (tuples, closures)
   - [ ] Dynamic types (`dyn Trait`, see explainer's [proposed design](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#async-drop-glue-for-dyn-trait))
   - [ ] coroutines (rust-lang/rust#123948)
 - [x] Async drop glue includes sync drop glue code
 - [x] Cleanup branch generation for `async_drop_in_place_raw`
 - [ ] Union rejects non-trivially async destructible fields
 - [ ] `AsyncDrop` implementation requires same bounds as type definition
 - [ ] Skip trivially destructible fields (optimization)
 - [ ] New [`TyKind::AdtAsyncDestructor`](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#adt-async-destructor-types) and get rid of combinators
 - [ ] [Synchronously undroppable types](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#exclusively-async-drop)
 - [ ] Automatic async drop at the end of the scope in async context
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
Add simple async drop glue generation

This is a prototype of the async drop glue generation for some simple types. Async drop glue is intended to behave very similar to the regular drop glue except for being asynchronous. Currently it does not execute synchronous drops but only calls user implementations of `AsyncDrop::async_drop` associative function and awaits the returned future. It is not complete as it only recurses into arrays, slices, tuples, and structs and does not have same sensible restrictions as the old `Drop` trait implementation like having the same bounds as the type definition, while code assumes their existence (requires a future work).

This current design uses a workaround as it does not create any custom async destructor state machine types for ADTs, but instead uses types defined in the std library called future combinators (deferred_async_drop, chain, ready_unit).

Also I recommend reading my [explainer](https://zetanumbers.github.io/book/async-drop-design.html).

This is a part of the [MCP: Low level components for async drop](rust-lang/compiler-team#727) work.

Feature completeness:

 - [x] `AsyncDrop` trait
 - [ ] `async_drop_in_place_raw`/async drop glue generation support for
   - [x] Trivially destructible types (integers, bools, floats, string slices, pointers, references, etc.)
   - [x] Arrays and slices (array pointer is unsized into slice pointer)
   - [x] ADTs (enums, structs, unions)
   - [x] tuple-like types (tuples, closures)
   - [ ] Dynamic types (`dyn Trait`, see explainer's [proposed design](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#async-drop-glue-for-dyn-trait))
   - [ ] coroutines (rust-lang/rust#123948)
 - [x] Async drop glue includes sync drop glue code
 - [x] Cleanup branch generation for `async_drop_in_place_raw`
 - [ ] Union rejects non-trivially async destructible fields
 - [ ] `AsyncDrop` implementation requires same bounds as type definition
 - [ ] Skip trivially destructible fields (optimization)
 - [ ] New [`TyKind::AdtAsyncDestructor`](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#adt-async-destructor-types) and get rid of combinators
 - [ ] [Synchronously undroppable types](https://github.com/zetanumbers/posts/blob/main/async-drop-design.md#exclusively-async-drop)
 - [ ] Automatic async drop at the end of the scope in async context
ty::InstanceDef::DropGlue(_, None)
| ty::InstanceDef::AsyncDropGlueCtorShim(_, None)
| ty::InstanceDef::AsyncDropGlue(_, None),
) = def {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this change to rustc_codegen_cranelift too at

if let ty::InstanceDef::DropGlue(_, None) | ty::InstanceDef::AsyncDropGlueCtorShim(_, None) =
and
InstanceDef::DropGlue(_, None) | ty::InstanceDef::AsyncDropGlueCtorShim(_, None) => {
?

bors added a commit to rust-lang-ci/rust that referenced this pull request May 31, 2024
Implement `needs_async_drop` in rustc and optimize async drop glue

This PR expands on rust-lang#121801 and implements `Ty::needs_async_drop` which works almost exactly the same as `Ty::needs_drop`, which is needed for rust-lang#123948.

Also made compiler's async drop code to look more like compiler's regular drop code, which enabled me to write an optimization where types which do not use `AsyncDrop` can simply forward async drop glue to `drop_in_place`. This made size of the async block from the [async_drop test](https://github.com/zetanumbers/rust/blob/67980dd6fb11917d23d01a19c2cf4cfc3978aac8/tests/ui/async-await/async-drop.rs) to decrease by 12%.
@petrochenkov petrochenkov added the F-async_drop Async drop label Jun 17, 2024
@oli-obk oli-obk added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 28, 2024
@azhogin
Copy link
Contributor Author

azhogin commented Sep 8, 2024

It is splitted into small PRs. @rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 8, 2024
@bors
Copy link
Contributor

bors commented Sep 9, 2024

☔ The latest upstream changes (presumably #130165) made this pull request unmergeable. Please resolve the merge conflicts.

@davidtwco
Copy link
Member

r? @nikomatsakis is going to look into this

@rustbot rustbot assigned nikomatsakis and unassigned davidtwco Oct 1, 2024
@nikomatsakis
Copy link
Contributor

r? @nikomatsakis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-async_drop Async drop S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.