Rollup merge of #138514 - compiler-errors:fake-borrow-ref-to-value, r=oli-obk

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

Remove fake borrows of closure captures if that capture has been replaced with a by-move version of that capture.

For example, given an async closure that looks like:

```
let f: Foo;
let c = async move || {
    match f { ... }
};
```

... in this pair of coroutine-closure + coroutine, 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 the by-move-body transform is responsible for replacing `_1.0: &Foo` with `_1.0: Foo` (since the `AsyncFnOnce` coroutine will own `Foo` by value), that makes the second fake borrow obsolete since we never have an upvar of type `&Foo`, 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 :)

Fixes #138501

r? oli-obk

Author: matthiaskrgr

Diff for: compiler/rustc_mir_transform/src/coroutine/by_move_body.rs

@@ -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 {
@@ -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);
     }
 }

Diff for: tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#0}.built.after.mir

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

Diff for: tests/mir-opt/async_closure_fake_read_for_by_move.foo-{closure#0}-{closure#1}.built.after.mir

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

Diff for: tests/mir-opt/async_closure_fake_read_for_by_move.rs

@@ -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() {}