fixup! 2229: Capture box completely in move closures

fixup! 2229: Capture box completely in move closures

Author: arora-aman

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,19 +1626,16 @@ 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 = restrict_preicision_for_box(&place, self.capture_clause);
-
         let place_with_id = PlaceWithHirId { place, ..*place_with_id };
-        debug!(
-            "borrow after restrictions:(place_with_id={:?}, diag_expr_id={:?}, bk={:?})",
-            place_with_id, diag_expr_id, bk
-        );
 
         if !self.capture_information.contains_key(&place_with_id.place) {
             self.init_capture_info_for_place(&place_with_id, diag_expr_id);
@@ -1661,39 +1659,46 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> {
     }
 }
 
-// In case of move closures we don't want to capture derefs on a box.
-// This is motivated by:
-//   1. We only want to capture data that is on the stack
-//   2. One motivatiton 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_preicision_for_box(place: &Place<'tcx>, capture_by: hir::CaptureBy) -> Place<'tcx> {
-    let mut rv = place.clone();
-    match capture_by {
-        hir::CaptureBy::Ref => rv,
+/// 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: Vec::new(), ..rv }
+                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 => rv,
+                    None => {}
                     Some(idx) => {
-                        Place { projections: rv.projections.drain(0..=idx).collect(), ..rv }
+                        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;
@@ -1728,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

@@ -141,7 +141,7 @@ fn truncate_box_derefs() {
     //~^ ERROR: First Pass analysis includes:
     //~| ERROR: Min Capture analysis includes:
         println!("{}", b.0);
-        //~^ NOTE: Capturing b[] -> ByValue
+        //~^ NOTE: Capturing b[Deref,(0, 0)] -> ByValue
         //~| NOTE: Min Capture b[] -> ByValue
     };
 
@@ -158,7 +158,7 @@ fn truncate_box_derefs() {
     //~^ ERROR: First Pass analysis includes:
     //~| ERROR: Min Capture analysis includes:
         println!("{}", t.1.0);
-        //~^ NOTE: Capturing t[(1, 0)] -> ByValue
+        //~^ NOTE: Capturing t[(1, 0),Deref,(0, 0)] -> ByValue
         //~| NOTE: Min Capture t[(1, 0)] -> ByValue
     };
 }

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

@@ -317,7 +317,7 @@ LL | |
 LL | |     };
    | |_____^
    |
-note: Capturing b[] -> ByValue
+note: Capturing b[Deref,(0, 0)] -> ByValue
   --> $DIR/move_closure.rs:143:24
    |
 LL |         println!("{}", b.0);
@@ -353,7 +353,7 @@ LL | |
 LL | |     };
    | |_____^
    |
-note: Capturing t[(1, 0)] -> ByValue
+note: Capturing t[(1, 0),Deref,(0, 0)] -> ByValue
   --> $DIR/move_closure.rs:160:24
    |
 LL |         println!("{}", t.1.0);