Skip to content

Remove fake borrows of refs that are converted into non-refs in MakeByMoveBody #138514

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

Merged
merged 1 commit into from
Mar 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions compiler/rustc_mir_transform/src/coroutine/by_move_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,17 +178,18 @@ pub(crate) fn coroutine_by_move_body_def_id<'tcx>(
),
};

(
Some((
FieldIdx::from_usize(child_field_idx + num_args),
(
FieldIdx::from_usize(parent_field_idx + num_args),
parent_capture_ty,
peel_deref,
child_precise_captures,
),
)
))
},
)
.flatten()
.collect();

if coroutine_kind == ty::ClosureKind::FnOnce {
Expand Down Expand Up @@ -312,10 +313,46 @@ impl<'tcx> MutVisitor<'tcx> for MakeByMoveBody<'tcx> {
self.super_place(place, context, location);
}

fn visit_statement(&mut self, statement: &mut mir::Statement<'tcx>, location: mir::Location) {
// Remove fake borrows of closure captures if that capture has been
// replaced with a by-move version of that capture.
//
// For example, imagine we capture `Foo` in the parent and `&Foo`
// in the child. We will emit two fake borrows like:
//
// ```
// _2 = &fake shallow (*(_1.0: &Foo));
// _3 = &fake shallow (_1.0: &Foo);
// ```
//
// However, since this transform is responsible for replacing
// `_1.0: &Foo` with `_1.0: Foo`, that makes the second fake borrow
// obsolete, and we should replace it with a nop.
//
// As a side-note, we don't actually even care about fake borrows
// here at all since they're fully a MIR borrowck artifact, and we
// don't need to borrowck by-move MIR bodies. But it's best to preserve
// as much as we can between these two bodies :)
if let mir::StatementKind::Assign(box (_, rvalue)) = &statement.kind
&& let mir::Rvalue::Ref(_, mir::BorrowKind::Fake(mir::FakeBorrowKind::Shallow), place) =
rvalue
&& let mir::PlaceRef {
local: ty::CAPTURE_STRUCT_LOCAL,
projection: [mir::ProjectionElem::Field(idx, _)],
} = place.as_ref()
&& let Some(&(_, _, true, _)) = self.field_remapping.get(&idx)
{
statement.kind = mir::StatementKind::Nop;
}

self.super_statement(statement, location);
}

fn visit_local_decl(&mut self, local: mir::Local, local_decl: &mut mir::LocalDecl<'tcx>) {
// Replace the type of the self arg.
if local == ty::CAPTURE_STRUCT_LOCAL {
local_decl.ty = self.by_move_coroutine_ty;
}
self.super_local_decl(local, local_decl);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// MIR for `foo::{closure#0}::{closure#0}` after built

fn foo::{closure#0}::{closure#0}(_1: {async closure body@$DIR/async_closure_fake_read_for_by_move.rs:12:27: 15:6}, _2: ResumeTy) -> ()
yields ()
{
debug _task_context => _2;
debug f => (*(_1.0: &&Foo));
let mut _0: ();
let mut _3: &Foo;
let mut _4: &&Foo;
let mut _5: &&&Foo;
let mut _6: isize;
let mut _7: bool;

bb0: {
PlaceMention((*(_1.0: &&Foo)));
_6 = discriminant((*(*(_1.0: &&Foo))));
switchInt(move _6) -> [0: bb2, otherwise: bb1];
}

bb1: {
_0 = const ();
goto -> bb10;
}

bb2: {
falseEdge -> [real: bb5, imaginary: bb1];
}

bb3: {
goto -> bb1;
}

bb4: {
FakeRead(ForMatchedPlace(None), (*(_1.0: &&Foo)));
unreachable;
}

bb5: {
_3 = &fake shallow (*(*(_1.0: &&Foo)));
_4 = &fake shallow (*(_1.0: &&Foo));
_5 = &fake shallow (_1.0: &&Foo);
StorageLive(_7);
_7 = const true;
switchInt(move _7) -> [0: bb8, otherwise: bb7];
}

bb6: {
falseEdge -> [real: bb3, imaginary: bb1];
}

bb7: {
StorageDead(_7);
FakeRead(ForMatchGuard, _3);
FakeRead(ForMatchGuard, _4);
FakeRead(ForMatchGuard, _5);
_0 = const ();
goto -> bb10;
}

bb8: {
goto -> bb9;
}

bb9: {
StorageDead(_7);
goto -> bb6;
}

bb10: {
drop(_1) -> [return: bb11, unwind: bb12];
}

bb11: {
return;
}

bb12 (cleanup): {
resume;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// MIR for `foo::{closure#0}::{closure#1}` after built

fn foo::{closure#0}::{closure#1}(_1: {async closure body@$DIR/async_closure_fake_read_for_by_move.rs:12:27: 15:6}, _2: ResumeTy) -> ()
yields ()
{
debug _task_context => _2;
debug f => (_1.0: &Foo);
let mut _0: ();
let mut _3: &Foo;
let mut _4: &&Foo;
let mut _5: &&&Foo;
let mut _6: isize;
let mut _7: bool;

bb0: {
PlaceMention((_1.0: &Foo));
_6 = discriminant((*(_1.0: &Foo)));
switchInt(move _6) -> [0: bb2, otherwise: bb1];
}

bb1: {
_0 = const ();
goto -> bb6;
}

bb2: {
falseEdge -> [real: bb3, imaginary: bb1];
}

bb3: {
_3 = &fake shallow (*(_1.0: &Foo));
_4 = &fake shallow (_1.0: &Foo);
nop;
StorageLive(_7);
_7 = const true;
switchInt(move _7) -> [0: bb5, otherwise: bb4];
}

bb4: {
StorageDead(_7);
FakeRead(ForMatchGuard, _3);
FakeRead(ForMatchGuard, _4);
FakeRead(ForMatchGuard, _5);
_0 = const ();
goto -> bb6;
}

bb5: {
StorageDead(_7);
falseEdge -> [real: bb1, imaginary: bb1];
}

bb6: {
drop(_1) -> [return: bb7, unwind: bb8];
}

bb7: {
return;
}

bb8 (cleanup): {
resume;
}
}
18 changes: 18 additions & 0 deletions tests/mir-opt/async_closure_fake_read_for_by_move.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//@ edition:2021
// skip-filecheck

enum Foo {
Bar,
Baz,
}

// EMIT_MIR async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir
// EMIT_MIR async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#1}.built.after.mir
fn foo(f: &Foo) {
let x = async move || match f {
Foo::Bar if true => {}
_ => {}
};
}

fn main() {}
Loading