Rollup merge of rust-lang#138517 - compiler-errors:better-child-capture, r=oli-obk

Improve upvar analysis for deref of child capture

Two fixes to the heuristic I implemented in rust-lang#123660. As I noted in the code:

> Luckily, if this function is not correct, then the program is not unsound, since we still borrowck and validate the choices made from this function -- the only side-effect is that the user may receive unnecessary borrowck errors.

This indeed fixes unnecessary borrowck errors.

r? oli-obk

---

The heuristic is only valid if we deref a `&T`, not a `&mut T` or `Box<T>`, so make sure to check the type. This fixes:

```rust
struct Foo { precise: i32 }

fn mut_ref_inside_mut(f: &mut Foo) {
    let x: impl AsyncFn() = async move || {
        let y = &f.precise;
    };
}
```

Since the capture from `f` to `&f.precise` needs to be treated as a lending borrow from the parent coroutine-closure to the child coroutine.

---

The heuristic is also valid if *any* deref projection in the child capture's projections is a `&T`, but we were only looking at the last one. This ensures that this function is considered not to be lending:

```rust
struct Foo { precise: i32 }

fn ref_inside_mut(f: &mut &Foo) {
    let x: impl Fn() -> _ = async move || {
        let y = &f.precise;
    };
}
```

(Specifically, checking that `impl Fn() -> _` is satisfied is exercising that the coroutine is not considered to be lending.)

Author: jhpratt

compiler/rustc_hir_typeck/src/upvar.rs

@@ -1862,8 +1862,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 ///
 /// (1.) Are we borrowing data owned by the parent closure? We can determine if
 /// that is the case by checking if the parent capture is by move, EXCEPT if we
-/// apply a deref projection, which means we're reborrowing a reference that we
-/// captured by move.
+/// apply a deref projection of an immutable reference, reborrows of immutable
+/// references which aren't restricted to the LUB of the lifetimes of the deref
+/// chain. This is why `&'short mut &'long T` can be reborrowed as `&'long T`.
 ///
 /// ```rust
 /// let x = &1i32; // Let's call this lifetime `'1`.
@@ -1902,10 +1903,22 @@ fn should_reborrow_from_env_of_parent_coroutine_closure<'tcx>(
 ) -> bool {
     // (1.)
     (!parent_capture.is_by_ref()
-        && !matches!(
-            child_capture.place.projections.get(parent_capture.place.projections.len()),
-            Some(Projection { kind: ProjectionKind::Deref, .. })
-        ))
+        // This is just inlined `place.deref_tys()` but truncated to just
+        // the child projections. Namely, look for a `&T` deref, since we
+        // can always extend `&'short mut &'long T` to `&'long T`.
+        && !child_capture
+            .place
+            .projections
+            .iter()
+            .enumerate()
+            .skip(parent_capture.place.projections.len())
+            .any(|(idx, proj)| {
+                matches!(proj.kind, ProjectionKind::Deref)
+                    && matches!(
+                        child_capture.place.ty_before_projection(idx).kind(),
+                        ty::Ref(.., ty::Mutability::Not)
+                    )
+            }))
         // (2.)
         || matches!(child_capture.info.capture_kind, UpvarCapture::ByRef(ty::BorrowKind::Mutable))
 }

tests/ui/async-await/async-closures/imm-deref-lending.rs

@@ -0,0 +1,46 @@
+//@ edition: 2021
+//@ check-pass
+
+#![feature(impl_trait_in_bindings)]
+
+struct FooS {
+    precise: i32,
+}
+
+fn ref_inside_mut(f: &mut &FooS) {
+    let x: impl AsyncFn() = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn mut_inside_ref(f: &&mut FooS) {
+    let x: impl AsyncFn() = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn mut_ref_inside_mut(f: &mut &mut FooS) {
+    let x: impl AsyncFn() = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn ref_inside_box(f: Box<&FooS>) {
+    let x: impl AsyncFn() = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn box_inside_ref(f: &Box<FooS>) {
+    let x: impl AsyncFn() = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn box_inside_box(f: Box<Box<FooS>>) {
+    let x: impl AsyncFn() = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn main() {}

tests/ui/async-await/async-closures/imm-deref-not-lending.rs

@@ -0,0 +1,49 @@
+//@ edition: 2021
+
+#![feature(impl_trait_in_bindings)]
+
+struct FooS {
+    precise: i32,
+}
+
+fn ref_inside_mut(f: &mut &FooS) {
+    let x: impl Fn() -> _ = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn mut_inside_ref(f: &&mut FooS) {
+    let x: impl Fn() -> _ = async move || {
+        let y = &f.precise;
+    };
+}
+
+// Expected to fail, no immutable reference here.
+fn mut_ref_inside_mut(f: &mut &mut FooS) {
+    let x: impl Fn() -> _ = async move || {
+        //~^ ERROR async closure does not implement `Fn`
+        let y = &f.precise;
+    };
+}
+
+fn ref_inside_box(f: Box<&FooS>) {
+    let x: impl Fn() -> _ = async move || {
+        let y = &f.precise;
+    };
+}
+
+fn box_inside_ref(f: &Box<FooS>) {
+    let x: impl Fn() -> _ = async move || {
+        let y = &f.precise;
+    };
+}
+
+// Expected to fail, no immutable reference here.
+fn box_inside_box(f: Box<Box<FooS>>) {
+    let x: impl Fn() -> _ = async move || {
+        //~^ ERROR async closure does not implement `Fn`
+        let y = &f.precise;
+    };
+}
+
+fn main() {}

tests/ui/async-await/async-closures/imm-deref-not-lending.stderr

@@ -0,0 +1,14 @@
+error: async closure does not implement `Fn` because it captures state from its environment
+  --> $DIR/imm-deref-not-lending.rs:23:29
+   |
+LL |     let x: impl Fn() -> _ = async move || {
+   |                             ^^^^^^^^^^^^^
+
+error: async closure does not implement `Fn` because it captures state from its environment
+  --> $DIR/imm-deref-not-lending.rs:43:29
+   |
+LL |     let x: impl Fn() -> _ = async move || {
+   |                             ^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+