Skip to content

Commit

Permalink
Force move async-closures that are FnOnce to make their inner cor…
Browse files Browse the repository at this point in the history
…outines also `move`
  • Loading branch information
compiler-errors committed Apr 3, 2024
1 parent 4c09456 commit c6f8ef6
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 73 deletions.
25 changes: 24 additions & 1 deletion compiler/rustc_hir_typeck/src/upvar.rs
Expand Up @@ -166,7 +166,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
span: Span,
body_id: hir::BodyId,
body: &'tcx hir::Body<'tcx>,
capture_clause: hir::CaptureBy,
mut capture_clause: hir::CaptureBy,
) {
// Extract the type of the closure.
let ty = self.node_ty(closure_hir_id);
Expand Down Expand Up @@ -259,6 +259,29 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
)
.consume_body(body);

// If a coroutine is comes from a coroutine-closure that is `move`, but
// the coroutine-closure was inferred to be `FnOnce` during signature
// inference, then it's still possible that we try to borrow upvars from
// the coroutine-closure because they are not used by the coroutine body
// in a way that forces a move.
//
// This would lead to an impossible to satisfy situation, since `AsyncFnOnce`
// coroutine bodies can't borrow from their parent closure. To fix this,
// we force the inner coroutine to also be `move`. This only matters for
// coroutine-closures that are `move` since otherwise they themselves will
// be borrowing from the outer environment, so there's no self-borrows occuring.
if let UpvarArgs::Coroutine(..) = args
&& let hir::CoroutineKind::Desugared(_, hir::CoroutineSource::Closure) =
self.tcx.coroutine_kind(closure_def_id).expect("coroutine should have kind")
&& let parent_hir_id =
self.tcx.local_def_id_to_hir_id(self.tcx.local_parent(closure_def_id))
&& let parent_ty = self.node_ty(parent_hir_id)
&& parent_ty.is_coroutine_closure()
&& let Some(ty::ClosureKind::FnOnce) = self.closure_kind(parent_ty)
{
capture_clause = self.tcx.hir_node(parent_hir_id).expect_closure().capture_clause;
}

debug!(
"For closure={:?}, capture_information={:#?}",
closure_def_id, delegate.capture_information
Expand Down
20 changes: 14 additions & 6 deletions compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
Expand Up @@ -91,15 +91,17 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
return;
}

let ty::Coroutine(_, coroutine_args) = *coroutine_ty.kind() else { bug!("{body:#?}") };
// We don't need to generate a by-move coroutine if the kind of the coroutine is
// already `FnOnce` -- that means that any upvars that the closure consumes have
// already been taken by-value.
let coroutine_kind = coroutine_args.as_coroutine().kind_ty().to_opt_closure_kind().unwrap();
if coroutine_kind == ty::ClosureKind::FnOnce {
// We don't need to generate a by-move coroutine if the coroutine body was
// produced by the `CoroutineKindShim`, since it's already by-move.
if matches!(body.source.instance, ty::InstanceDef::CoroutineKindShim { .. }) {
return;
}

let ty::Coroutine(_, args) = *coroutine_ty.kind() else { bug!("{body:#?}") };
let args = args.as_coroutine();

let coroutine_kind = args.kind_ty().to_opt_closure_kind().unwrap();

let parent_def_id = tcx.local_parent(coroutine_def_id);
let ty::CoroutineClosure(_, parent_args) =
*tcx.type_of(parent_def_id).instantiate_identity().kind()
Expand Down Expand Up @@ -128,6 +130,12 @@ impl<'tcx> MirPass<'tcx> for ByMoveBody {
// the outer closure body -- we need to change the coroutine to take the
// upvar by value.
if coroutine_capture.is_by_ref() && !parent_capture.is_by_ref() {
assert_ne!(
coroutine_kind,
ty::ClosureKind::FnOnce,
"`FnOnce` coroutine-closures return coroutines that capture from \
their body; it will always result in a borrowck error!"
);
by_ref_fields.insert(FieldIdx::from_usize(num_args + idx));
}

Expand Down
31 changes: 0 additions & 31 deletions src/tools/miri/tests/pass/async-closure-captures.stderr

This file was deleted.

14 changes: 14 additions & 0 deletions src/tools/miri/tests/pass/async-closure-captures.stdout
@@ -0,0 +1,14 @@
Hello(0)
Hello(0)
Hello(1)
Hello(1)
Hello(2)
Hello(3)
Hello(3)
Hello(4)
Hello(4)
Hello(5)
Hello(6)
Hello(7)
Hello(8)
Hello(9)
6 changes: 2 additions & 4 deletions tests/ui/async-await/async-closures/captures.rs
@@ -1,7 +1,7 @@
//@ aux-build:block-on.rs
//@ edition:2021


//@ run-pass
//@ check-run-results

// Same as miri's `tests/pass/async-closure-captures.rs`, keep in sync

Expand Down Expand Up @@ -104,14 +104,12 @@ async fn async_main() {
let x = Hello(8);
let c = force_fnonce(async || {
println!("{x:?}");
//~^ ERROR `x` does not live long enough
});
call_once(c).await;

let x = &Hello(9);
let c = force_fnonce(async || {
println!("{x:?}");
//~^ ERROR `x` does not live long enough
});
call_once(c).await;
}
Expand Down
4 changes: 4 additions & 0 deletions tests/ui/async-await/async-closures/captures.run.stdout
Expand Up @@ -8,3 +8,7 @@ Hello(3)
Hello(4)
Hello(4)
Hello(5)
Hello(6)
Hello(7)
Hello(8)
Hello(9)
31 changes: 0 additions & 31 deletions tests/ui/async-await/async-closures/captures.stderr

This file was deleted.

0 comments on commit c6f8ef6

Please sign in to comment.