From d37a07ffbefd6772a68c25f18d35a7fe5b82942e Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Tue, 22 Jun 2021 01:58:17 -0400 Subject: [PATCH] fixup! 2229: Capture box completely in move closures fixup! 2229: Capture box completely in move closures --- compiler/rustc_typeck/src/check/upvar.rs | 58 ++++++++++--------- .../2229_closure_analysis/move_closure.rs | 4 +- .../2229_closure_analysis/move_closure.stderr | 4 +- 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 00531f25861..581aa087be9 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/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 diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/move_closure.rs index 3b5171d7da6..f96370eb203 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.rs +++ b/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 }; } diff --git a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/move_closure.stderr index 5ec9bbfe2b0..82aa7ab8912 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr +++ b/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);