From b8405a97d725923cb060b305bb2221871689e102 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 16 Jul 2024 15:30:32 -0400 Subject: [PATCH 1/3] Rewrite closure-of-async to async-closure --- compiler/rustc_ast/src/ast.rs | 30 +++++++++- compiler/rustc_ast_lowering/src/expr.rs | 59 +++++++++++++++---- .../async-closures/wrong-fn-kind.stderr | 26 +------- 3 files changed, 76 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 411fc8311a08f..a5f663e3ad61f 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1214,6 +1214,30 @@ impl Expr { } } + pub fn peel_uwu(&self) -> &Expr { + let mut expr = self; + loop { + match &expr.kind { + ExprKind::Block(blk, None) => { + if blk.stmts.len() == 1 + && let StmtKind::Expr(blk) = &blk.stmts[0].kind + { + expr = blk; + } else { + break; + } + } + ExprKind::Paren(paren) => { + expr = paren; + } + _ => { + break; + } + } + } + expr + } + pub fn peel_parens(&self) -> &Expr { let mut expr = self; while let ExprKind::Paren(inner) = &expr.kind { @@ -1614,15 +1638,15 @@ pub struct QSelf { } /// A capture clause used in closures and `async` blocks. -#[derive(Clone, Copy, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)] +#[derive(Clone, Copy, PartialEq, Encodable, Decodable, Debug, HashStable_Generic, Ord, PartialOrd)] pub enum CaptureBy { + /// `move` keyword was not specified. + Ref, /// `move |x| y + x`. Value { /// The span of the `move` keyword. move_kw: Span, }, - /// `move` keyword was not specified. - Ref, } /// Closure lifetime binder, `for<'a, 'b>` in `for<'a, 'b> |_: &'a (), _: &'b ()|`. diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index d870f9fe0aef2..c430d23258ce8 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -218,18 +218,53 @@ impl<'hir> LoweringContext<'_, 'hir> { *fn_decl_span, *fn_arg_span, ), - None => self.lower_expr_closure( - binder, - *capture_clause, - e.id, - hir_id, - *constness, - *movability, - fn_decl, - body, - *fn_decl_span, - *fn_arg_span, - ), + None => { + let peeled = body.peel_uwu(); + if let ast::ExprKind::Gen( + gen_capture_clause, + block, + gen_kind @ ast::GenBlockKind::Async, + span, + ) = &peeled.kind + { + let coroutine_kind = match gen_kind { + GenBlockKind::Async => CoroutineKind::Async { span: *span, closure_id: peeled.node_id(), return_impl_trait_id: self.next_node_id() }, + GenBlockKind::Gen => CoroutineKind::Gen { span: *span, closure_id: peeled.node_id(), return_impl_trait_id: self.next_node_id() }, + GenBlockKind::AsyncGen => CoroutineKind::AsyncGen { span: *span, closure_id: peeled.node_id(), return_impl_trait_id: self.next_node_id() }, + }; + let id = self.next_node_id(); + self.lower_expr_coroutine_closure( + binder, + capture_clause.max(*gen_capture_clause), + e.id, + hir_id, + coroutine_kind, + fn_decl, + &ast::Expr { + id, + span: *span, + kind: ExprKind::Block(block.clone(), None), + attrs: thin_vec![], + tokens: None, + }, + *fn_decl_span, + *fn_arg_span, + ) + } else { + self.lower_expr_closure( + binder, + *capture_clause, + e.id, + hir_id, + *constness, + *movability, + fn_decl, + body, + *fn_decl_span, + *fn_arg_span, + ) + } + } }, ExprKind::Gen(capture_clause, block, genblock_kind, decl_span) => { let desugaring_kind = match genblock_kind { diff --git a/tests/ui/async-await/async-closures/wrong-fn-kind.stderr b/tests/ui/async-await/async-closures/wrong-fn-kind.stderr index 4b626c1bed6c3..a5a67bf887c5c 100644 --- a/tests/ui/async-await/async-closures/wrong-fn-kind.stderr +++ b/tests/ui/async-await/async-closures/wrong-fn-kind.stderr @@ -1,24 +1,3 @@ -error[E0525]: expected a closure that implements the `async Fn` trait, but this closure only implements `async FnOnce` - --> $DIR/wrong-fn-kind.rs:17:20 - | -LL | needs_async_fn(move || async move { - | -------------- -^^^^^^ - | | | - | _____|______________this closure implements `async FnOnce`, not `async Fn` - | | | - | | required by a bound introduced by this call -LL | | -LL | | println!("{x}"); - | | - closure is `async FnOnce` because it moves the variable `x` out of its environment -LL | | }); - | |_____- the requirement to implement `async Fn` derives from here - | -note: required by a bound in `needs_async_fn` - --> $DIR/wrong-fn-kind.rs:5:27 - | -LL | fn needs_async_fn(_: impl async Fn()) {} - | ^^^^^^^^^^ required by this bound in `needs_async_fn` - error[E0596]: cannot borrow `x` as mutable, as it is a captured variable in a `Fn` closure --> $DIR/wrong-fn-kind.rs:9:20 | @@ -35,7 +14,6 @@ LL | LL | x += 1; | - mutable borrow occurs due to use of `x` in closure -error: aborting due to 2 previous errors +error: aborting due to 1 previous error -Some errors have detailed explanations: E0525, E0596. -For more information about an error, try `rustc --explain E0525`. +For more information about this error, try `rustc --explain E0596`. From 2d8c21505573e96d04ab947e294a762817008e2d Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 16 Jul 2024 15:42:50 -0400 Subject: [PATCH 2/3] sins --- compiler/rustc_ast/src/ast.rs | 3 ++- compiler/rustc_ast_lowering/src/expr.rs | 20 ++++++++++++--- .../issue-69446-fnmut-capture.stderr | 25 ++++++++----------- .../issue-70935-complex-spans.stderr | 2 ++ .../opaque-cast-field-access-in-future.stderr | 11 ++++++-- 5 files changed, 40 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index a5f663e3ad61f..c5e1d17918cfd 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1638,7 +1638,8 @@ pub struct QSelf { } /// A capture clause used in closures and `async` blocks. -#[derive(Clone, Copy, PartialEq, Encodable, Decodable, Debug, HashStable_Generic, Ord, PartialOrd)] +#[derive(Clone, Copy, PartialEq, Ord, Eq, PartialOrd, Debug)] +#[derive(Encodable, Decodable, HashStable_Generic)] pub enum CaptureBy { /// `move` keyword was not specified. Ref, diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index c430d23258ce8..2d4be3212c4cc 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -228,14 +228,26 @@ impl<'hir> LoweringContext<'_, 'hir> { ) = &peeled.kind { let coroutine_kind = match gen_kind { - GenBlockKind::Async => CoroutineKind::Async { span: *span, closure_id: peeled.node_id(), return_impl_trait_id: self.next_node_id() }, - GenBlockKind::Gen => CoroutineKind::Gen { span: *span, closure_id: peeled.node_id(), return_impl_trait_id: self.next_node_id() }, - GenBlockKind::AsyncGen => CoroutineKind::AsyncGen { span: *span, closure_id: peeled.node_id(), return_impl_trait_id: self.next_node_id() }, + GenBlockKind::Async => CoroutineKind::Async { + span: *span, + closure_id: peeled.node_id(), + return_impl_trait_id: self.next_node_id(), + }, + GenBlockKind::Gen => CoroutineKind::Gen { + span: *span, + closure_id: peeled.node_id(), + return_impl_trait_id: self.next_node_id(), + }, + GenBlockKind::AsyncGen => CoroutineKind::AsyncGen { + span: *span, + closure_id: peeled.node_id(), + return_impl_trait_id: self.next_node_id(), + }, }; let id = self.next_node_id(); self.lower_expr_coroutine_closure( binder, - capture_clause.max(*gen_capture_clause), + (*capture_clause).max(*gen_capture_clause), e.id, hir_id, coroutine_kind, diff --git a/tests/ui/async-await/issue-69446-fnmut-capture.stderr b/tests/ui/async-await/issue-69446-fnmut-capture.stderr index 0366c2f44c0e7..8a94209e09617 100644 --- a/tests/ui/async-await/issue-69446-fnmut-capture.stderr +++ b/tests/ui/async-await/issue-69446-fnmut-capture.stderr @@ -1,19 +1,16 @@ -error: captured variable cannot escape `FnMut` closure body - --> $DIR/issue-69446-fnmut-capture.rs:19:17 +error: async closure does not implement `FnMut` because it captures state from its environment + --> $DIR/issue-69446-fnmut-capture.rs:19:9 | -LL | let mut x = Foo; - | ----- variable defined here -LL | bar(move || async { - | _______________-_^ - | | | - | | inferred to be a `FnMut` closure -LL | | x.foo(); - | | - variable captured here -LL | | }); - | |_____^ returns an `async` block that contains a reference to a captured variable, which then escapes the closure body +LL | bar(move || async { + | --- ^^^^^^^ + | | + | required by a bound introduced by this call | - = note: `FnMut` closures only have access to their captured variables while they are executing... - = note: ...therefore, they cannot allow references to captured variables to escape +note: required by a bound in `bar` + --> $DIR/issue-69446-fnmut-capture.rs:12:25 + | +LL | async fn bar(_: impl FnMut() -> T) + | ^^^^^^^^^^^^ required by this bound in `bar` error: aborting due to 1 previous error diff --git a/tests/ui/async-await/issue-70935-complex-spans.stderr b/tests/ui/async-await/issue-70935-complex-spans.stderr index 1ca0b339c16ad..080c3af6be6a2 100644 --- a/tests/ui/async-await/issue-70935-complex-spans.stderr +++ b/tests/ui/async-await/issue-70935-complex-spans.stderr @@ -13,6 +13,7 @@ note: required because it appears within the type `NotSync` LL | struct NotSync(PhantomData<*mut ()>); | ^^^^^^^ = note: required for `&NotSync` to implement `Send` + = note: required because it appears within the type `(&NotSync,)` note: required because it's used within this closure --> $DIR/issue-70935-complex-spans.rs:19:13 | @@ -46,6 +47,7 @@ note: required because it appears within the type `NotSync` LL | struct NotSync(PhantomData<*mut ()>); | ^^^^^^^ = note: required for `&NotSync` to implement `Send` + = note: required because it appears within the type `(&NotSync,)` note: required because it's used within this closure --> $DIR/issue-70935-complex-spans.rs:19:13 | diff --git a/tests/ui/impl-trait/opaque-cast-field-access-in-future.stderr b/tests/ui/impl-trait/opaque-cast-field-access-in-future.stderr index 5ade6a69d4b0d..df375ccd773a2 100644 --- a/tests/ui/impl-trait/opaque-cast-field-access-in-future.stderr +++ b/tests/ui/impl-trait/opaque-cast-field-access-in-future.stderr @@ -1,3 +1,9 @@ +error[E0282]: type annotations needed + --> $DIR/opaque-cast-field-access-in-future.rs:7:14 + | +LL | &mut foo.bar; + | ^^^ cannot infer type + error[E0283]: type annotations needed --> $DIR/opaque-cast-field-access-in-future.rs:22:17 | @@ -6,6 +12,7 @@ LL | fn run() -> Foo> { | = note: cannot satisfy `_: Future` -error: aborting due to 1 previous error +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0283`. +Some errors have detailed explanations: E0282, E0283. +For more information about an error, try `rustc --explain E0282`. From 3851fe0f53269c9af374af5ad3b92f2c0f04f4c7 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 25 Jul 2024 14:02:33 -0400 Subject: [PATCH 3/3] Make coroutine-closures possible to be cloned --- compiler/rustc_mir_transform/src/shim.rs | 3 +++ .../src/solve/assembly/structural_traits.rs | 5 +++- .../src/traits/select/mod.rs | 17 +++++++++++-- .../async-closures/clone-closure.rs | 24 +++++++++++++++++++ .../async-closures/clone-closure.run.stdout | 2 ++ .../move-consuming-capture.stderr | 9 +++++++ ...thout-precise-captures-we-are-powerless.rs | 2 +- ...t-precise-captures-we-are-powerless.stderr | 23 +++++------------- 8 files changed, 64 insertions(+), 21 deletions(-) create mode 100644 tests/ui/async-await/async-closures/clone-closure.rs create mode 100644 tests/ui/async-await/async-closures/clone-closure.run.stdout diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index 6835a39cf3624..50fe33b5e4fa1 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -434,6 +434,9 @@ fn build_clone_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, self_ty: Ty<'tcx>) - match self_ty.kind() { ty::FnDef(..) | ty::FnPtr(_) => builder.copy_shim(), ty::Closure(_, args) => builder.tuple_like_shim(dest, src, args.as_closure().upvar_tys()), + ty::CoroutineClosure(_, args) => { + builder.tuple_like_shim(dest, src, args.as_coroutine_closure().upvar_tys()) + } ty::Tuple(..) => builder.tuple_like_shim(dest, src, self_ty.tuple_fields()), ty::Coroutine(coroutine_def_id, args) => { assert_eq!(tcx.coroutine_movability(*coroutine_def_id), hir::Movability::Movable); diff --git a/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs b/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs index 7df14e81ab5d4..bd4a754881c3f 100644 --- a/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs +++ b/compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs @@ -216,7 +216,10 @@ where // impl Copy/Clone for Closure where Self::TupledUpvars: Copy/Clone ty::Closure(_, args) => Ok(vec![ty::Binder::dummy(args.as_closure().tupled_upvars_ty())]), - ty::CoroutineClosure(..) => Err(NoSolution), + // impl Copy/Clone for CoroutineClosure where Self::TupledUpvars: Copy/Clone + ty::CoroutineClosure(_, args) => { + Ok(vec![ty::Binder::dummy(args.as_coroutine_closure().tupled_upvars_ty())]) + } // only when `coroutine_clone` is enabled and the coroutine is movable // impl Copy/Clone for Coroutine where T: Copy/Clone forall T in (upvars, witnesses) diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index c007cd5314a86..699c05466bd5a 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2262,8 +2262,21 @@ impl<'tcx> SelectionContext<'_, 'tcx> { } } - // FIXME(async_closures): These are never clone, for now. - ty::CoroutineClosure(_, _) => None, + ty::CoroutineClosure(_, args) => { + // (*) binder moved here + let ty = self.infcx.shallow_resolve(args.as_coroutine_closure().tupled_upvars_ty()); + if let ty::Infer(ty::TyVar(_)) = ty.kind() { + // Not yet resolved. + Ambiguous + } else { + Where( + obligation + .predicate + .rebind(args.as_coroutine_closure().upvar_tys().to_vec()), + ) + } + } + // `Copy` and `Clone` are automatically implemented for an anonymous adt // if all of its fields are `Copy` and `Clone` ty::Adt(adt, args) if adt.is_anonymous() => { diff --git a/tests/ui/async-await/async-closures/clone-closure.rs b/tests/ui/async-await/async-closures/clone-closure.rs new file mode 100644 index 0000000000000..807897e3e0314 --- /dev/null +++ b/tests/ui/async-await/async-closures/clone-closure.rs @@ -0,0 +1,24 @@ +//@ aux-build:block-on.rs +//@ edition:2021 +//@ run-pass +//@ check-run-results + +#![feature(async_closure)] + +extern crate block_on; + +async fn for_each(f: impl async FnOnce(&str) + Clone) { + f.clone()("world").await; + f.clone()("world2").await; +} + +fn main() { + block_on::block_on(async_main()); +} + +async fn async_main() { + let x = String::from("Hello,"); + for_each(async move |s| { + println!("{x} {s}"); + }).await; +} diff --git a/tests/ui/async-await/async-closures/clone-closure.run.stdout b/tests/ui/async-await/async-closures/clone-closure.run.stdout new file mode 100644 index 0000000000000..0cfcf1923da88 --- /dev/null +++ b/tests/ui/async-await/async-closures/clone-closure.run.stdout @@ -0,0 +1,2 @@ +Hello, world +Hello, world2 diff --git a/tests/ui/async-await/async-closures/move-consuming-capture.stderr b/tests/ui/async-await/async-closures/move-consuming-capture.stderr index 45c1eac8f8f56..4ce71ec49d612 100644 --- a/tests/ui/async-await/async-closures/move-consuming-capture.stderr +++ b/tests/ui/async-await/async-closures/move-consuming-capture.stderr @@ -11,6 +11,15 @@ LL | x().await; | note: `async_call_once` takes ownership of the receiver `self`, which moves `x` --> $SRC_DIR/core/src/ops/async_function.rs:LL:COL +help: you could `clone` the value and consume it, if the `NoCopy: Clone` trait bound could be satisfied + | +LL | x.clone()().await; + | ++++++++ +help: consider annotating `NoCopy` with `#[derive(Clone)]` + | +LL + #[derive(Clone)] +LL | struct NoCopy; + | error: aborting due to 1 previous error diff --git a/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.rs b/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.rs index 17681161e20ed..18f16ca4b2d30 100644 --- a/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.rs +++ b/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.rs @@ -19,7 +19,7 @@ fn simple<'a>(x: &'a i32) { let c = async move || { println!("{}", *x); }; outlives::<'a>(c()); //~ ERROR `c` does not live long enough - outlives::<'a>(call_once(c)); //~ ERROR cannot move out of `c` + outlives::<'a>(call_once(c)); } struct S<'a>(&'a i32); diff --git a/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.stderr b/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.stderr index 569028934cbac..ed32a53e80728 100644 --- a/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.stderr +++ b/tests/ui/async-await/async-closures/without-precise-captures-we-are-powerless.stderr @@ -29,22 +29,6 @@ LL | outlives::<'a>(call_once(c)); LL | } | - `c` dropped here while still borrowed -error[E0505]: cannot move out of `c` because it is borrowed - --> $DIR/without-precise-captures-we-are-powerless.rs:22:30 - | -LL | fn simple<'a>(x: &'a i32) { - | -- lifetime `'a` defined here -... -LL | let c = async move || { println!("{}", *x); }; - | - binding `c` declared here -LL | outlives::<'a>(c()); - | --- - | | - | borrow of `c` occurs here - | argument requires that `c` is borrowed for `'a` -LL | outlives::<'a>(call_once(c)); - | ^ move out of `c` occurs here - error[E0597]: `x` does not live long enough --> $DIR/without-precise-captures-we-are-powerless.rs:28:13 | @@ -72,6 +56,11 @@ LL | outlives::<'a>(call_once(c)); LL | LL | let c = async move || { println!("{}", *x.0); }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ move out of `x` occurs here + | +help: consider cloning the value if the performance cost is acceptable + | +LL | let c = async || { println!("{}", *x.0); }.clone(); + | ++++++++ error[E0597]: `c` does not live long enough --> $DIR/without-precise-captures-we-are-powerless.rs:33:20 @@ -146,7 +135,7 @@ LL | // outlives::<'a>(call_once(c)); // FIXME(async_closures): Figure out w LL | } | - `c` dropped here while still borrowed -error: aborting due to 10 previous errors +error: aborting due to 9 previous errors Some errors have detailed explanations: E0505, E0597, E0621. For more information about an error, try `rustc --explain E0505`.