Rollup merge of #128201 - compiler-errors:closure-clone, r=oli-obk

Implement `Copy`/`Clone` for async closures

We can do so in the same cases that regular closures do.

For the purposes of cloning, coroutine-closures are actually precisely the same as regular closures, specifically in the aspect that `Clone` impls care about which is the upvars. The only difference b/w coroutine-closures and regular closures is the type that they *return*, but this type has not been *created* yet, so we don't really have a problem.

IDK why I didn't add this impl initially -- I went back and forth a bit on the internal representation for coroutine-closures before settling on a design which largely models regular closures. Previous (not published) iterations of coroutine-closures used to be represented as a special (read: cursed) kind of coroutine, which would probably suffer from the pitfalls that coroutines have that oli mentioned below in #128201 (comment).

r? oli-obk

Author: tgross35

compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs

@@ -1442,9 +1442,14 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, '_, 'infcx, 'tcx> {
             // See `tests/ui/moves/needs-clone-through-deref.rs`
             return false;
         }
+        // We don't want to suggest `.clone()` in a move closure, since the value has already been captured.
         if self.in_move_closure(expr) {
             return false;
         }
+        // We also don't want to suggest cloning a closure itself, since the value has already been captured.
+        if let hir::ExprKind::Closure(_) = expr.kind {
+            return false;
+        }
         // Try to find predicates on *generic params* that would allow copying `ty`
         let mut suggestion =
             if let Some(symbol) = tcx.hir().maybe_get_struct_pattern_shorthand_field(expr) {

compiler/rustc_mir_transform/src/shim.rs

@@ -435,6 +435,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);

compiler/rustc_next_trait_solver/src/solve/assembly/structural_traits.rs

@@ -217,7 +217,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)

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() => {

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;
+}

tests/ui/async-await/async-closures/clone-closure.run.stdout

@@ -0,0 +1,2 @@
+Hello, world
+Hello, world2

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
 

tests/ui/async-await/async-closures/not-clone-closure.rs

@@ -0,0 +1,36 @@
+//@ edition: 2021
+
+#![feature(async_closure)]
+
+struct NotClonableArg;
+#[derive(Default)]
+struct NotClonableReturnType;
+
+// Verify that the only components that we care about are the upvars, not the signature.
+fn we_are_okay_with_not_clonable_signature() {
+    let x = async |x: NotClonableArg| -> NotClonableReturnType { Default::default() };
+    x.clone(); // Okay
+}
+
+#[derive(Debug)]
+struct NotClonableUpvar;
+
+fn we_only_care_about_clonable_upvars() {
+    let x = NotClonableUpvar;
+    // Notably, this is clone because we capture `&x`.
+    let yes_clone = async || {
+        println!("{x:?}");
+    };
+    yes_clone.clone(); // Okay
+
+    let z = NotClonableUpvar;
+    // However, this is not because the closure captures `z` by move.
+    // (Even though the future that is lent out captures `z by ref!)
+    let not_clone = async move || {
+        println!("{z:?}");
+    };
+    not_clone.clone();
+    //~^ ERROR the trait bound `NotClonableUpvar: Clone` is not satisfied
+}
+
+fn main() {}

tests/ui/async-await/async-closures/not-clone-closure.stderr

@@ -0,0 +1,20 @@
+error[E0277]: the trait bound `NotClonableUpvar: Clone` is not satisfied in `{async closure@$DIR/not-clone-closure.rs:29:21: 29:34}`
+  --> $DIR/not-clone-closure.rs:32:15
+   |
+LL |     not_clone.clone();
+   |               ^^^^^ within `{async closure@$DIR/not-clone-closure.rs:29:21: 29:34}`, the trait `Clone` is not implemented for `NotClonableUpvar`, which is required by `{async closure@$DIR/not-clone-closure.rs:29:21: 29:34}: Clone`
+   |
+note: required because it's used within this closure
+  --> $DIR/not-clone-closure.rs:29:21
+   |
+LL |     let not_clone = async move || {
+   |                     ^^^^^^^^^^^^^
+help: consider annotating `NotClonableUpvar` with `#[derive(Clone)]`
+   |
+LL + #[derive(Clone)]
+LL | struct NotClonableUpvar;
+   |
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.

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);