From de2052af9cf7ac83da6a825701e32bd0e4aa9a85 Mon Sep 17 00:00:00 2001 From: Aman Arora Date: Fri, 18 Jun 2021 16:35:10 -0400 Subject: [PATCH] 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. --- compiler/rustc_typeck/src/check/upvar.rs | 35 ++++++ .../2229_closure_analysis/move_closure.rs | 34 +++++- .../2229_closure_analysis/move_closure.stderr | 104 ++++++++++++++++-- 3 files changed, 165 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 4c9c3954624..00531f25861 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -1630,7 +1630,14 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, '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); @@ -1654,6 +1661,34 @@ 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, + hir::CaptureBy::Value => { + if ty::TyS::is_box(place.base_ty) { + Place { projections: Vec::new(), ..rv } + } 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, + Some(idx) => { + Place { projections: rv.projections.drain(0..=idx).collect(), ..rv } + } + } + } + } + } +} + /// 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. 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 1c574da5f48..3b5171d7da6 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.rs +++ b/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 @@ -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 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + println!("{}", b.0); + //~^ NOTE: Capturing b[] -> 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 + move || { + //~^ ERROR: First Pass analysis includes: + //~| ERROR: Min Capture analysis includes: + println!("{}", t.1.0); + //~^ NOTE: Capturing t[(1, 0)] -> ByValue + //~| NOTE: Min Capture t[(1, 0)] -> ByValue + }; } fn main() { 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 b91ef4dd85c..5ec9bbfe2b0 100644 --- a/src/test/ui/closures/2229_closure_analysis/move_closure.stderr +++ b/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 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 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[] -> 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)] -> 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`.