diff options
11 files changed, 195 insertions, 21 deletions
diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index e1d79248171..2d1231d819d 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -430,6 +430,9 @@ pub struct TypeckResults<'tcx> { /// see `MinCaptureInformationMap` for more details. pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>, + /// [FIXME] RFC2229 Change to use HashSet instead of Vec + pub closure_fake_reads: FxHashMap<DefId, Vec<HirPlace<'tcx>>>, + /// Stores the type, expression, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). pub generator_interior_types: ty::Binder<Vec<GeneratorInteriorTypeCause<'tcx>>>, @@ -464,6 +467,7 @@ impl<'tcx> TypeckResults<'tcx> { concrete_opaque_types: Default::default(), closure_captures: Default::default(), closure_min_captures: Default::default(), + closure_fake_reads: Default::default(), generator_interior_types: ty::Binder::dummy(Default::default()), treat_byte_string_as_slice: Default::default(), } @@ -715,6 +719,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> { ref concrete_opaque_types, ref closure_captures, ref closure_min_captures, + ref closure_fake_reads, ref generator_interior_types, ref treat_byte_string_as_slice, } = *self; @@ -750,6 +755,7 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> { concrete_opaque_types.hash_stable(hcx, hasher); closure_captures.hash_stable(hcx, hasher); closure_min_captures.hash_stable(hcx, hasher); + closure_fake_reads.hash_stable(hcx, hasher); generator_interior_types.hash_stable(hcx, hasher); treat_byte_string_as_slice.hash_stable(hcx, hasher); }) diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index 532c725c823..109a6521128 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -17,7 +17,7 @@ use rustc_target::abi::VariantIdx; use rustc_index::vec::Idx; /// The "outermost" place that holds this value. -#[derive(Copy, Clone)] +#[derive(Copy, Clone, Debug)] crate enum PlaceBase { /// Denotes the start of a `Place`. Local(Local), @@ -67,7 +67,7 @@ crate enum PlaceBase { /// /// This is used internally when building a place for an expression like `a.b.c`. The fields `b` /// and `c` can be progressively pushed onto the place builder that is created when converting `a`. -#[derive(Clone)] +#[derive(Clone, Debug)] crate struct PlaceBuilder<'tcx> { base: PlaceBase, projection: Vec<PlaceElem<'tcx>>, @@ -199,7 +199,7 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>( from_builder: PlaceBuilder<'tcx>, tcx: TyCtxt<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>, -) -> Result<PlaceBuilder<'tcx>, HirId> { +) -> Result<PlaceBuilder<'tcx>, PlaceBuilder<'tcx>> { match from_builder.base { PlaceBase::Local(_) => Ok(from_builder), PlaceBase::Upvar { var_hir_id, closure_def_id, closure_kind } => { @@ -236,7 +236,7 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>( var_hir_id, from_builder.projection, ); } - return Err(var_hir_id); + return Err(upvar_resolved_place_builder); }; let closure_ty = typeck_results @@ -288,7 +288,7 @@ impl<'tcx> PlaceBuilder<'tcx> { if let PlaceBase::Local(local) = self.base { Place { local, projection: tcx.intern_place_elems(&self.projection) } } else { - self.expect_upvars_resolved(tcx, typeck_results).into_place(tcx, typeck_results) + self.try_upvars_resolved(tcx, typeck_results).into_place(tcx, typeck_results) } } @@ -300,6 +300,17 @@ impl<'tcx> PlaceBuilder<'tcx> { to_upvars_resolved_place_builder(self, tcx, typeck_results).unwrap() } + fn try_upvars_resolved<'a>( + self, + tcx: TyCtxt<'tcx>, + typeck_results: &'a ty::TypeckResults<'tcx>, + ) -> PlaceBuilder<'tcx> { + match to_upvars_resolved_place_builder(self, tcx, typeck_results) { + Ok(upvars_resolved) => upvars_resolved, + Err(upvars_unresolved) => upvars_unresolved, + } + } + crate fn base(&self) -> PlaceBase { self.base } diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index d73e5eef70c..3a8665777b7 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -8,6 +8,7 @@ use crate::build::{BlockAnd, BlockAndExtension, Builder}; use crate::thir::*; use rustc_middle::middle::region; use rustc_middle::mir::AssertKind; +use rustc_middle::mir::Place; use rustc_middle::mir::*; use rustc_middle::ty::{self, Ty, UpvarSubsts}; use rustc_span::Span; @@ -164,7 +165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields)) } - ExprKind::Closure { closure_id, substs, upvars, movability } => { + ExprKind::Closure { closure_id, substs, upvars, movability, fake_reads } => { // see (*) above let operands: Vec<_> = upvars .into_iter() @@ -203,6 +204,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } }) .collect(); + + if let Some(fake_reads) = fake_reads { + for thir_place in fake_reads.into_iter() { + // = this.hir.mirror(thir_place); + let mir_place = unpack!(block = this.as_place(block, thir_place)); + // [FIXME] RFC2229 FakeReadCause can be ForLet or ForMatch, need to use the correct one + this.cfg.push_fake_read( + block, + source_info, + FakeReadCause::ForMatchedPlace, + mir_place, + ); + } + } + let result = match substs { UpvarSubsts::Generator(substs) => { // We implicitly set the discriminant to 0. See diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 47f75825fb6..b2e8b2de1bc 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -420,7 +420,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::PlaceTypeAscription { .. } | ExprKind::ValueTypeAscription { .. } => { debug_assert!(Category::of(&expr.kind) == Some(Category::Place)); - let place = unpack!(block = this.as_place(block, expr)); let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place)); this.cfg.push_assign(block, source_info, destination, rvalue); @@ -437,7 +436,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } debug_assert!(Category::of(&expr.kind) == Some(Category::Place)); - let place = unpack!(block = this.as_place(block, expr)); let rvalue = Rvalue::Use(this.consume_by_copy_or_move(place)); this.cfg.push_assign(block, source_info, destination, rvalue); diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index dfcb52c83c0..25e08efb2e3 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -5,6 +5,7 @@ use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_hir as hir; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; use rustc_index::vec::Idx; +use rustc_middle::hir::place::Place as HirPlace; use rustc_middle::hir::place::PlaceBase as HirPlaceBase; use rustc_middle::hir::place::ProjectionKind as HirProjectionKind; use rustc_middle::mir::interpret::Scalar; @@ -452,7 +453,39 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { .zip(substs.upvar_tys()) .map(|(captured_place, ty)| self.capture_upvar(expr, captured_place, ty)), ); - ExprKind::Closure { closure_id: def_id, substs, upvars, movability } + + let fake_reads = match self.typeck_results().closure_fake_reads.get(&def_id) { + Some(vals) => Some(self.arena.alloc_from_iter(vals + .iter() + .filter(|val| match val.base { + HirPlaceBase::Upvar(_) => true, + _ => false, + }) + .map(|val| { + let var_hir_id = match val.base { + HirPlaceBase::Upvar(upvar_id) => { + debug!("upvar"); + upvar_id.var_path.hir_id + } + _ => { + bug!( + "Do not know how to get HirId out of Rvalue and StaticItem" + ); + } + }; + self.fake_read_capture_upvar(expr, val.clone(), var_hir_id) + }) + )), + None => None, + }; + + ExprKind::Closure { + closure_id: def_id, + substs, + upvars, + movability, + fake_reads: fake_reads, + } } hir::ExprKind::Path(ref qpath) => { @@ -1012,6 +1045,49 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { ExprKind::Deref { arg: ref_expr } } + fn fake_read_capture_upvar( + &mut self, + closure_expr: &'tcx hir::Expr<'tcx>, + place: HirPlace<'tcx>, + hir_id: hir::HirId, + ) -> Expr<'thir, 'tcx> { + let temp_lifetime = self.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id); + let var_ty = place.base_ty; + + let mut captured_place_expr = Expr { + temp_lifetime, + ty: var_ty, + span: closure_expr.span, + kind: self.convert_var(hir_id), + }; + // [FIXME] RFC2229 Maybe we should introduce an immutable borrow of the fake capture so that we don't + // end up moving this place + for proj in place.projections.iter() { + let kind = match proj.kind { + HirProjectionKind::Deref => { + ExprKind::Deref { arg: self.arena.alloc(captured_place_expr) } + } + HirProjectionKind::Field(field, ..) => { + // Variant index will always be 0, because for multi-variant + // enums, we capture the enum entirely. + ExprKind::Field { + lhs: self.arena.alloc(captured_place_expr), + name: Field::new(field as usize), + } + } + HirProjectionKind::Index | HirProjectionKind::Subslice => { + // We don't capture these projections, so we can ignore them here + continue; + } + }; + + captured_place_expr = + Expr { temp_lifetime, ty: proj.ty, span: closure_expr.span, kind }; + } + + captured_place_expr + } + fn capture_upvar( &mut self, closure_expr: &'tcx hir::Expr<'tcx>, diff --git a/compiler/rustc_mir_build/src/thir/mod.rs b/compiler/rustc_mir_build/src/thir/mod.rs index 0c9df32c188..730c0f4a3df 100644 --- a/compiler/rustc_mir_build/src/thir/mod.rs +++ b/compiler/rustc_mir_build/src/thir/mod.rs @@ -281,6 +281,7 @@ pub enum ExprKind<'thir, 'tcx> { substs: UpvarSubsts<'tcx>, upvars: &'thir [Expr<'thir, 'tcx>], movability: Option<hir::Movability>, + fake_reads: Option<&'thir mut [Expr<'thir, 'tcx>]>, }, Literal { literal: &'tcx Const<'tcx>, diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 254f12b9563..74e2ca51039 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -34,6 +34,7 @@ use super::writeback::Resolver; use super::FnCtxt; use crate::expr_use_visitor as euv; +use rustc_data_structures::fx::FxHashSet; use rustc_data_structures::fx::FxIndexMap; use rustc_hir as hir; use rustc_hir::def_id::DefId; @@ -145,6 +146,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { current_closure_kind: ty::ClosureKind::LATTICE_BOTTOM, current_origin: None, capture_information: Default::default(), + fake_reads: Default::default(), }; euv::ExprUseVisitor::new( &mut delegate, @@ -246,6 +248,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let final_tupled_upvars_type = self.tcx.mk_tup(final_upvar_tys.iter()); self.demand_suptype(span, substs.tupled_upvars_ty(), final_tupled_upvars_type); + let fake_reads = delegate.fake_reads.into_iter().map(|fake_read| fake_read).collect(); + self.typeck_results.borrow_mut().closure_fake_reads.insert(closure_def_id, fake_reads); + // If we are also inferred the closure kind here, // process any deferred resolutions. let deferred_call_resolutions = self.remove_deferred_call_resolutions(closure_def_id); @@ -1148,6 +1153,8 @@ struct InferBorrowKind<'a, 'tcx> { /// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow } /// ``` capture_information: InferredCaptureInformation<'tcx>, + // [FIXME] RFC2229 Change Vec to FxHashSet + fake_reads: FxHashSet<Place<'tcx>>, // these need to be fake read. } impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { @@ -1409,6 +1416,12 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { } impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { + fn fake_read(&mut self, place: PlaceWithHirId<'tcx>) { + if let PlaceBase::Upvar(_) = place.place.base { + self.fake_reads.insert(place.place); + } + } + fn consume( &mut self, place_with_id: &PlaceWithHirId<'tcx>, diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index af82a3bb4f5..9cccda7768c 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -4,11 +4,14 @@ use crate::check::FnCtxt; +use rustc_data_structures::stable_map::FxHashMap; use rustc_errors::ErrorReported; use rustc_hir as hir; +use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_infer::infer::error_reporting::TypeAnnotationNeeded::E0282; use rustc_infer::infer::InferCtxt; +use rustc_middle::hir::place::Place as HirPlace; use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCast}; use rustc_middle::ty::fold::{TypeFoldable, TypeFolder}; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -56,6 +59,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } wbcx.visit_body(body); wbcx.visit_min_capture_map(); + wbcx.visit_fake_reads_map(); wbcx.visit_upvar_capture_map(); wbcx.visit_closures(); wbcx.visit_liberated_fn_sigs(); @@ -363,6 +367,25 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { self.typeck_results.closure_min_captures = min_captures_wb; } + fn visit_fake_reads_map(&mut self) { + let mut resolved_closure_fake_reads: FxHashMap<DefId, Vec<HirPlace<'tcx>>> = + Default::default(); + for (closure_def_id, fake_reads) in + self.fcx.typeck_results.borrow().closure_fake_reads.iter() + { + let mut resolved_fake_reads = Vec::<HirPlace<'tcx>>::new(); + for fake_read in fake_reads.iter() { + let locatable = + self.tcx().hir().local_def_id_to_hir_id(closure_def_id.expect_local()); + + let resolved_fake_read = self.resolve(fake_read.clone(), &locatable); + resolved_fake_reads.push(resolved_fake_read); + } + resolved_closure_fake_reads.insert(*closure_def_id, resolved_fake_reads); + } + self.typeck_results.closure_fake_reads = resolved_closure_fake_reads; + } + fn visit_upvar_capture_map(&mut self) { for (upvar_id, upvar_capture) in self.fcx.typeck_results.borrow().upvar_capture_map.iter() { let new_upvar_capture = match *upvar_capture { diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 52110af4792..45ecf30cd50 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -10,6 +10,7 @@ pub use rustc_middle::hir::place::{PlaceBase, PlaceWithHirId, Projection}; use rustc_hir as hir; use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; +// use rustc_hir::Pat; use rustc_hir::PatKind; use rustc_index::vec::Idx; use rustc_infer::infer::InferCtxt; @@ -51,6 +52,9 @@ pub trait Delegate<'tcx> { // The path at `assignee_place` is being assigned to. // `diag_expr_id` is the id used for diagnostics (see `consume` for more details). fn mutate(&mut self, assignee_place: &PlaceWithHirId<'tcx>, diag_expr_id: hir::HirId); + + // [FIXME] RFC2229 This should also affect clippy ref: https://github.com/sexxi-goose/rust/pull/27 + fn fake_read(&mut self, place: PlaceWithHirId<'tcx>); } #[derive(Copy, Clone, PartialEq, Debug)] @@ -229,7 +233,24 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { hir::ExprKind::Match(ref discr, arms, _) => { let discr_place = return_if_err!(self.mc.cat_expr(&discr)); - self.borrow_expr(&discr, ty::ImmBorrow); + + // We only want to borrow discr if the pattern contain something other + // than wildcards + let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self; + let mut res = false; + for arm in arms.iter() { + return_if_err!(mc.cat_pattern(discr_place.clone(), &arm.pat, |_place, pat| { + if let PatKind::Binding(_, _, _, opt_sub_pat) = pat.kind { + if let None = opt_sub_pat { + res = true; + } + } + })); + } + + if res { + self.borrow_expr(&discr, ty::ImmBorrow); + } // treatment of the discriminant is handled while walking the arms. for arm in arms { @@ -537,6 +558,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { fn walk_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) { debug!("walk_pat(discr_place={:?}, pat={:?})", discr_place, pat); + self.delegate.fake_read(discr_place.clone()); + let tcx = self.tcx(); let ExprUseVisitor { ref mc, body_owner: _, ref mut delegate } = *self; return_if_err!(mc.cat_pattern(discr_place.clone(), pat, |place, pat| { @@ -599,6 +622,10 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) { debug!("walk_captures({:?})", closure_expr); + // Over here we walk a closure that is nested inside the current body + // If the current body is a closure, then we also want to report back any fake reads, + // starting off of variables that are captured by our parent as well. + let closure_def_id = self.tcx().hir().local_def_id(closure_expr.hir_id).to_def_id(); let upvars = self.tcx().upvars_mentioned(self.body_owner); @@ -611,6 +638,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { if let Some(min_captures) = self.mc.typeck_results.closure_min_captures.get(&closure_def_id) { for (var_hir_id, min_list) in min_captures.iter() { + // Use this as a reference for if we should promote the fake read if upvars.map_or(body_owner_is_closure, |upvars| !upvars.contains_key(var_hir_id)) { // The nested closure might be capturing the current (enclosing) closure's local variables. // We check if the root variable is ever mentioned within the enclosing closure, if not @@ -636,6 +664,12 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { place.projections.clone(), ); + // [FIXME] RFC2229 We want to created another loop that iterates mc.typeck_results.fake_reads() + // [FIXME] RFC2229 Add tests for nested closures + if body_owner_is_closure { + self.delegate.fake_read(place_with_id.clone()); + } + match capture_info.capture_kind { ty::UpvarCapture::ByValue(_) => { let mode = copy_or_move(&self.mc, &place_with_id); diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.rs b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.rs index 6107a082237..2ed0149b9db 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.rs +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.rs @@ -13,12 +13,8 @@ fn main() { let mut point = SingleVariant::Point(10, -10); let c = || { - // FIXME(project-rfc-2229#24): Change this to be a destructure pattern - // once this is fixed, to remove the warning. - if let SingleVariant::Point(ref mut x, _) = point { - //~^ WARNING: irrefutable `if let` pattern - *x += 1; - } + let SingleVariant::Point(ref mut x, _) = point; + *x += 1; }; let b = c; diff --git a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr index 8586dfd9186..4863e6e2976 100644 --- a/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr +++ b/src/test/ui/closures/2229_closure_analysis/diagnostics/closure-origin-single-variant-diagnostics.stderr @@ -21,7 +21,7 @@ LL | | } = help: consider replacing the `if let` with a `let` error[E0382]: use of moved value: `c` - --> $DIR/closure-origin-single-variant-diagnostics.rs:25:13 + --> $DIR/closure-origin-single-variant-diagnostics.rs:21:13 | LL | let b = c; | - value moved here @@ -29,11 +29,11 @@ LL | let a = c; | ^ value used here after move | note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `point.0` out of its environment - --> $DIR/closure-origin-single-variant-diagnostics.rs:18:53 + --> $DIR/closure-origin-single-variant-diagnostics.rs:16:50 | -LL | if let SingleVariant::Point(ref mut x, _) = point { - | ^^^^^ +LL | let SingleVariant::Point(ref mut x, _) = point; + | ^^^^^ -error: aborting due to previous error; 2 warnings emitted +error: aborting due to previous error; 1 warning emitted For more information about this error, try `rustc --explain E0382`. |