Auto merge of #86445 - sexxi-goose:box_fix, r=nikomatsakis

2229: Capture box completely in move closures

Even if the content from box is used in a sharef-ref context,
we capture the box entirerly.

This is motivated by:
1) We only capture data that is on the stack.
2) Capturing data from within the box might end up moving more data than
the user anticipated.

Closes rust-lang/project-rfc-2229#50

r? `@nikomatsakis`

compiler/rustc_typeck/src/check/upvar.rs

@@ -167,7 +167,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         );
         self.log_capture_analysis_first_pass(closure_def_id, &delegate.capture_information, span);
 
-        self.compute_min_captures(closure_def_id, delegate.capture_information);
+        self.compute_min_captures(closure_def_id, capture_clause, delegate.capture_information);
 
         let closure_hir_id = self.tcx.hir().local_def_id_to_hir_id(local_def_id);
 
@@ -200,7 +200,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
 
             // This will update the min captures based on this new fake information.
-            self.compute_min_captures(closure_def_id, capture_information);
+            self.compute_min_captures(closure_def_id, capture_clause, capture_information);
         }
 
         if let Some(closure_substs) = infer_kind {
@@ -213,7 +213,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // If we have an origin, store it.
             if let Some(origin) = delegate.current_origin.clone() {
                 let origin = if enable_precise_capture(self.tcx, span) {
-                    (origin.0, restrict_capture_precision(origin.1))
+                    (origin.0, restrict_capture_precision(capture_clause, origin.1))
                 } else {
                     (origin.0, Place { projections: vec![], ..origin.1 })
                 };
@@ -368,6 +368,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     fn compute_min_captures(
         &self,
         closure_def_id: DefId,
+        capture_clause: hir::CaptureBy,
         capture_information: InferredCaptureInformation<'tcx>,
     ) {
         if capture_information.is_empty() {
@@ -385,7 +386,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 base => bug!("Expected upvar, found={:?}", base),
             };
 
-            let place = restrict_capture_precision(place);
+            let place = restrict_capture_precision(capture_clause, place);
 
             let min_cap_list = match root_var_min_capture_list.get_mut(&var_hir_id) {
                 None => {
@@ -1590,7 +1591,7 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
         if let PlaceBase::Upvar(_) = place.base {
             // We need to restrict Fake Read precision to avoid fake reading unsafe code,
             // such as deref of a raw pointer.
-            let place = restrict_capture_precision(place);
+            let place = restrict_capture_precision(self.capture_clause, place);
             let place =
                 restrict_repr_packed_field_ref_capture(self.fcx.tcx, self.fcx.param_env, &place);
             self.fake_reads.push((place, cause, diag_expr_id));
@@ -1625,11 +1626,15 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
             place_with_id, diag_expr_id, bk
         );
 
+        // We only want repr packed restriction to be applied to reading references into a packed
+        // struct, and not when the data is being moved. There for we call this method here instead
+        // of in `restrict_capture_precision`.
         let place = restrict_repr_packed_field_ref_capture(
             self.fcx.tcx,
             self.fcx.param_env,
             &place_with_id.place,
         );
+
         let place_with_id = PlaceWithHirId { place, ..*place_with_id };
 
         if !self.capture_information.contains_key(&place_with_id.place) {
@@ -1654,11 +1659,46 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
     }
 }
 
+/// Deref of a box isn't captured in move clousres. This is motivated by:
+///   1. We only want to capture data that is on the stack
+///   2. One motivation for the user to use a box might be to reduce the amount of data that gets
+///      moved (if size of pointer < size of data). We want to make sure that this optimization that
+///      the user made is respected.
+fn restrict_precision_for_box<'tcx>(
+    capture_clause: hir::CaptureBy,
+    mut place: Place<'tcx>,
+) -> Place<'tcx> {
+    match capture_clause {
+        hir::CaptureBy::Ref => {}
+        hir::CaptureBy::Value => {
+            if ty::TyS::is_box(place.base_ty) {
+                place.projections.truncate(0);
+            } else {
+                // Either the box is the last access or there is a deref applied on the box
+                // In either case we want to stop at the box.
+                let pos = place.projections.iter().position(|proj| ty::TyS::is_box(proj.ty));
+                match pos {
+                    None => {}
+                    Some(idx) => {
+                        place.projections.truncate(idx + 1);
+                    }
+                }
+            }
+        }
+    }
+
+    place
+}
+
 /// Truncate projections so that following rules are obeyed by the captured `place`:
 /// - No projections are applied to raw pointers, since these require unsafe blocks. We capture
 ///   them completely.
 /// - No Index projections are captured, since arrays are captured completely.
-fn restrict_capture_precision<'tcx>(mut place: Place<'tcx>) -> Place<'tcx> {
+/// - Deref of a box isn't captured in move clousres.
+fn restrict_capture_precision<'tcx>(
+    capture_clause: hir::CaptureBy,
+    mut place: Place<'tcx>,
+) -> Place<'tcx> {
     if place.projections.is_empty() {
         // Nothing to do here
         return place;
@@ -1693,7 +1733,8 @@ fn restrict_capture_precision<'tcx>(mut place: Place<'tcx>) -> Place<'tcx> {
 
     place.projections.truncate(length);
 
-    place
+    // Dont't capture projections on top of a box in move closures.
+    restrict_precision_for_box(capture_clause, place)
 }
 
 /// Truncates a place so that the resultant capture doesn't move data out of a reference

src/test/ui/closures/2229_closure_analysis/move_closure.rs

@@ -114,8 +114,9 @@ fn struct_contains_ref_to_another_struct_3() {
 fn truncate_box_derefs() {
     struct S(i32);
 
-    let b = Box::new(S(10));
 
+    // Content within the box is moved within the closure
+    let b = Box::new(S(10));
     let c = #[rustc_capture_analysis]
     //~^ ERROR: attributes on expressions are experimental
     //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
@@ -129,6 +130,37 @@ fn truncate_box_derefs() {
     };
 
     c();
+
+    // Content within the box is used by a shared ref and the box is the root variable
+    let b = Box::new(S(10));
+
+    let c = #[rustc_capture_analysis]
+    //~^ ERROR: attributes on expressions are experimental
+    //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
+    move || {
+    //~^ ERROR: First Pass analysis includes:
+    //~| ERROR: Min Capture analysis includes:
+        println!("{}", b.0);
+        //~^ NOTE: Capturing b[Deref,(0, 0)] -> ByValue
+        //~| NOTE: Min Capture b[] -> ByValue
+    };
+
+    c();
+
+    // Content within the box is used by a shared ref and the box is not the root variable
+    let b = Box::new(S(10));
+    let t = (0, b);
+
+    let c = #[rustc_capture_analysis]
+    //~^ ERROR: attributes on expressions are experimental
+    //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
+    move || {
+    //~^ ERROR: First Pass analysis includes:
+    //~| ERROR: Min Capture analysis includes:
+        println!("{}", t.1.0);
+        //~^ NOTE: Capturing t[(1, 0),Deref,(0, 0)] -> ByValue
+        //~| NOTE: Min Capture t[(1, 0)] -> ByValue
+    };
 }
 
 fn main() {

src/test/ui/closures/2229_closure_analysis/move_closure.stderr

@@ -44,7 +44,25 @@ LL |     let mut c = #[rustc_capture_analysis]
    = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
 
 error[E0658]: attributes on expressions are experimental
-  --> $DIR/move_closure.rs:119:13
+  --> $DIR/move_closure.rs:120:13
+   |
+LL |     let c = #[rustc_capture_analysis]
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
+   = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
+
+error[E0658]: attributes on expressions are experimental
+  --> $DIR/move_closure.rs:137:13
+   |
+LL |     let c = #[rustc_capture_analysis]
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: see issue #15701 <https://github.com/rust-lang/rust/issues/15701> for more information
+   = help: add `#![feature(stmt_expr_attributes)]` to the crate attributes to enable
+
+error[E0658]: attributes on expressions are experimental
+  --> $DIR/move_closure.rs:154:13
    |
 LL |     let c = #[rustc_capture_analysis]
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -247,7 +265,7 @@ LL |         let _t = t.0.0;
    |                  ^^^^^
 
 error: First Pass analysis includes:
-  --> $DIR/move_closure.rs:122:5
+  --> $DIR/move_closure.rs:123:5
    |
 LL | /     move || {
 LL | |
@@ -259,18 +277,18 @@ LL | |     };
    | |_____^
    |
 note: Capturing b[Deref,(0, 0)] -> ByValue
-  --> $DIR/move_closure.rs:125:18
+  --> $DIR/move_closure.rs:126:18
    |
 LL |         let _t = b.0;
    |                  ^^^
 note: Capturing b[] -> ByValue
-  --> $DIR/move_closure.rs:125:18
+  --> $DIR/move_closure.rs:126:18
    |
 LL |         let _t = b.0;
    |                  ^^^
 
 error: Min Capture analysis includes:
-  --> $DIR/move_closure.rs:122:5
+  --> $DIR/move_closure.rs:123:5
    |
 LL | /     move || {
 LL | |
@@ -282,11 +300,83 @@ LL | |     };
    | |_____^
    |
 note: Min Capture b[] -> ByValue
-  --> $DIR/move_closure.rs:125:18
+  --> $DIR/move_closure.rs:126:18
    |
 LL |         let _t = b.0;
    |                  ^^^
 
-error: aborting due to 18 previous errors; 1 warning emitted
+error: First Pass analysis includes:
+  --> $DIR/move_closure.rs:140:5
+   |
+LL | /     move || {
+LL | |
+LL | |
+LL | |         println!("{}", b.0);
+LL | |
+LL | |
+LL | |     };
+   | |_____^
+   |
+note: Capturing b[Deref,(0, 0)] -> ByValue
+  --> $DIR/move_closure.rs:143:24
+   |
+LL |         println!("{}", b.0);
+   |                        ^^^
+
+error: Min Capture analysis includes:
+  --> $DIR/move_closure.rs:140:5
+   |
+LL | /     move || {
+LL | |
+LL | |
+LL | |         println!("{}", b.0);
+LL | |
+LL | |
+LL | |     };
+   | |_____^
+   |
+note: Min Capture b[] -> ByValue
+  --> $DIR/move_closure.rs:143:24
+   |
+LL |         println!("{}", b.0);
+   |                        ^^^
+
+error: First Pass analysis includes:
+  --> $DIR/move_closure.rs:157:5
+   |
+LL | /     move || {
+LL | |
+LL | |
+LL | |         println!("{}", t.1.0);
+LL | |
+LL | |
+LL | |     };
+   | |_____^
+   |
+note: Capturing t[(1, 0),Deref,(0, 0)] -> ByValue
+  --> $DIR/move_closure.rs:160:24
+   |
+LL |         println!("{}", t.1.0);
+   |                        ^^^^^
+
+error: Min Capture analysis includes:
+  --> $DIR/move_closure.rs:157:5
+   |
+LL | /     move || {
+LL | |
+LL | |
+LL | |         println!("{}", t.1.0);
+LL | |
+LL | |
+LL | |     };
+   | |_____^
+   |
+note: Min Capture t[(1, 0)] -> ByValue
+  --> $DIR/move_closure.rs:160:24
+   |
+LL |         println!("{}", t.1.0);
+   |                        ^^^^^
+
+error: aborting due to 24 previous errors; 1 warning emitted
 
 For more information about this error, try `rustc --explain E0658`.