Skip to content

Commit

Permalink
Unrolled build for rust-lang#128201
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#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 rust-lang#128201 (comment).

r? oli-obk
  • Loading branch information
rust-timer authored Jul 27, 2024
2 parents 7c2012d + 5a9959f commit 1e91163
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 21 deletions.
5 changes: 5 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_mir_transform/src/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 15 additions & 2 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() => {
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/async-await/async-closures/clone-closure.rs
Original file line number Diff line number Diff line change
@@ -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;
}
2 changes: 2 additions & 0 deletions tests/ui/async-await/async-closures/clone-closure.run.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Hello, world
Hello, world2
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
36 changes: 36 additions & 0 deletions tests/ui/async-await/async-closures/not-clone-closure.rs
Original file line number Diff line number Diff line change
@@ -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() {}
20 changes: 20 additions & 0 deletions tests/ui/async-await/async-closures/not-clone-closure.stderr
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand Down Expand Up @@ -146,7 +130,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`.

0 comments on commit 1e91163

Please sign in to comment.