From ec10b71d42ace3c3d57c3d44bc1007badcd58ee8 Mon Sep 17 00:00:00 2001 From: Roxane Date: Tue, 2 Feb 2021 21:07:52 -0500 Subject: [PATCH 1/8] Introduce new fake reads --- compiler/rustc_middle/src/ty/context.rs | 6 ++ .../src/build/expr/as_place.rs | 21 +++-- .../src/build/expr/as_rvalue.rs | 18 ++++- .../rustc_mir_build/src/build/expr/into.rs | 2 - compiler/rustc_mir_build/src/thir/cx/expr.rs | 78 ++++++++++++++++++- compiler/rustc_mir_build/src/thir/mod.rs | 1 + compiler/rustc_typeck/src/check/upvar.rs | 13 ++++ compiler/rustc_typeck/src/check/writeback.rs | 23 ++++++ compiler/rustc_typeck/src/expr_use_visitor.rs | 36 ++++++++- ...osure-origin-single-variant-diagnostics.rs | 8 +- ...e-origin-single-variant-diagnostics.stderr | 10 +-- 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 e1d79248171a8..2d1231d819d38 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>>, + /// 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>>, @@ -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> 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> 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 532c725c823ef..109a652112883 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>, @@ -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, HirId> { +) -> Result, 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 d73e5eef70ca6..3a8665777b79e 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 47f75825fb6af..b2e8b2de1bc6a 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 dfcb52c83c0d7..25e08efb2e353 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 0c9df32c18803..730c0f4a3df2a 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, + 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 254f12b956304..74e2ca51039d3 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>, // 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 af82a3bb4f59a..9cccda7768cc3 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>> = + Default::default(); + for (closure_def_id, fake_reads) in + self.fcx.typeck_results.borrow().closure_fake_reads.iter() + { + let mut resolved_fake_reads = Vec::>::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 52110af47929e..45ecf30cd50e3 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 6107a082237c6..2ed0149b9db7d 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 8586dfd91863c..4863e6e2976d9 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`. From d4f8729c892882b16d7ce66f287818b6a66fe200 Mon Sep 17 00:00:00 2001 From: Roxane Date: Thu, 18 Feb 2021 19:02:07 -0500 Subject: [PATCH 2/8] Delay use of Place in favor of PlaceBuilder --- compiler/rustc_mir_build/src/build/block.rs | 10 +++ .../src/build/expr/as_place.rs | 35 +++++--- .../rustc_mir_build/src/build/expr/into.rs | 1 + .../rustc_mir_build/src/build/expr/mod.rs | 2 +- .../rustc_mir_build/src/build/matches/mod.rs | 84 ++++++++++++------- .../src/build/matches/simplify.rs | 28 ++++--- .../rustc_mir_build/src/build/matches/test.rs | 15 ++-- .../rustc_mir_build/src/build/matches/util.rs | 34 ++++---- compiler/rustc_mir_build/src/build/mod.rs | 6 +- 9 files changed, 139 insertions(+), 76 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index 808c6e3ff644b..ae4d6b56152c5 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -94,6 +94,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) ); } + // ROX: + // + // Where the handling of destructure patterns start + // + // let (a, b, c, _ ) = something + // + // (a, b, c, _) is the pattern + // something is the initializer StmtKind::Let { remainder_scope, init_scope, pattern, initializer, lint_level } => { let ignores_expr_result = matches!(*pattern.kind, PatKind::Wild); this.block_context.push(BlockFrame::Statement { ignores_expr_result }); @@ -125,6 +133,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ArmHasGuard(false), Some((None, initializer_span)), ); + // This is where we get into pattern handling of the let + // statement this.expr_into_pattern(block, pattern.clone(), init) }) } 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 109a652112883..f819a638c7aa0 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -10,6 +10,7 @@ use rustc_middle::hir::place::ProjectionKind as HirProjectionKind; use rustc_middle::middle::region; use rustc_middle::mir::AssertKind::BoundsCheck; use rustc_middle::mir::*; +use rustc_middle::ty::AdtDef; use rustc_middle::ty::{self, CanonicalUserTypeAnnotation, Ty, TyCtxt, Variance}; use rustc_span::Span; use rustc_target::abi::VariantIdx; @@ -17,7 +18,7 @@ use rustc_target::abi::VariantIdx; use rustc_index::vec::Idx; /// The "outermost" place that holds this value. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, PartialEq)] crate enum PlaceBase { /// Denotes the start of a `Place`. Local(Local), @@ -67,7 +68,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, Debug)] +#[derive(Clone, Debug, PartialEq)] crate struct PlaceBuilder<'tcx> { base: PlaceBase, projection: Vec>, @@ -292,6 +293,7 @@ impl<'tcx> PlaceBuilder<'tcx> { } } + /// ROX: Function that will be called when we really do need a place fn expect_upvars_resolved<'a>( self, tcx: TyCtxt<'tcx>, @@ -319,15 +321,22 @@ impl<'tcx> PlaceBuilder<'tcx> { self.project(PlaceElem::Field(f, ty)) } - fn deref(self) -> Self { + crate fn deref(self) -> Self { self.project(PlaceElem::Deref) } + crate fn downcast(self, adt_def: &'tcx AdtDef, variant_index: VariantIdx) -> Self { + self.project(PlaceElem::Downcast( + Some(adt_def.variants[variant_index].ident.name), + variant_index, + )) + } + fn index(self, index: Local) -> Self { self.project(PlaceElem::Index(index)) } - fn project(mut self, elem: PlaceElem<'tcx>) -> Self { + crate fn project(mut self, elem: PlaceElem<'tcx>) -> Self { self.projection.push(elem); self } @@ -367,6 +376,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(place_builder.into_place(self.tcx, self.typeck_results)) } + // ROX: As place builder /// This is used when constructing a compound `Place`, so that we can avoid creating /// intermediate `Place` values until we know the full set of projections. crate fn as_place_builder( @@ -613,13 +623,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // The "retagging" transformation (for Stacked Borrows) relies on this. let idx = unpack!(block = self.as_temp(block, temp_lifetime, index, Mutability::Not,)); - block = self.bounds_check( - block, - base_place.clone().into_place(self.tcx, self.typeck_results), - idx, - expr_span, - source_info, - ); + block = self.bounds_check(block, base_place.clone(), idx, expr_span, source_info); if is_outermost_index { self.read_fake_borrows(block, fake_borrow_temps, source_info) @@ -640,7 +644,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn bounds_check( &mut self, block: BasicBlock, - slice: Place<'tcx>, + slice: PlaceBuilder<'tcx>, index: Local, expr_span: Span, source_info: SourceInfo, @@ -652,7 +656,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let lt = self.temp(bool_ty, expr_span); // len = len(slice) - self.cfg.push_assign(block, source_info, len, Rvalue::Len(slice)); + self.cfg.push_assign( + block, + source_info, + len, + Rvalue::Len(slice.clone().into_place(self.tcx, self.typeck_results)), + ); // lt = idx < len self.cfg.push_assign( block, diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index b2e8b2de1bc6a..e0ad34e08efd4 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -280,6 +280,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let field_names: Vec<_> = (0..adt_def.variants[variant_index].fields.len()).map(Field::new).collect(); + // ROX: This is probably here the function record/struct update pattern is done. let fields: Vec<_> = if let Some(FruInfo { base, field_types }) = base { let place_builder = unpack!(block = this.as_place_builder(block, base)); diff --git a/compiler/rustc_mir_build/src/build/expr/mod.rs b/compiler/rustc_mir_build/src/build/expr/mod.rs index 07338928eb8ef..539de80cab71c 100644 --- a/compiler/rustc_mir_build/src/build/expr/mod.rs +++ b/compiler/rustc_mir_build/src/build/expr/mod.rs @@ -62,7 +62,7 @@ mod as_constant; mod as_operand; -mod as_place; +pub mod as_place; mod as_rvalue; mod as_temp; mod category; diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 6c31528be73f7..7285cf756234d 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1,3 +1,6 @@ +// ROX: this folder contains all code for handling patterns, including exhaustiveness checking etc. +// We want to be careful ^^' + //! Code related to match expressions. These are sufficiently complex to //! warrant their own module and submodules. :) This main module includes the //! high-level algorithm, the submodules contain the details. @@ -5,6 +8,7 @@ //! This also includes code for pattern bindings in `let` statements and //! function parameters. +use crate::build::expr::as_place::PlaceBuilder; use crate::build::scope::DropKind; use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard}; use crate::build::{BlockAnd, BlockAndExtension, Builder}; @@ -96,7 +100,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let scrutinee_place = unpack!(block = self.lower_scrutinee(block, scrutinee, scrutinee_span,)); - let mut arm_candidates = self.create_match_candidates(scrutinee_place, &arms); + let mut arm_candidates = self.create_match_candidates(scrutinee_place.clone(), &arms); let match_has_guard = arms.iter().any(|arm| arm.guard.is_some()); let mut candidates = @@ -121,8 +125,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { mut block: BasicBlock, scrutinee: &Expr<'_, 'tcx>, scrutinee_span: Span, - ) -> BlockAnd> { - let scrutinee_place = unpack!(block = self.as_place(block, scrutinee)); + ) -> BlockAnd> { + let scrutinee_place_builder = unpack!(block = self.as_place_builder(block, scrutinee)); + let scrutinee_place = + scrutinee_place_builder.clone().into_place(self.tcx, self.typeck_results); // Matching on a `scrutinee_place` with an uninhabited type doesn't // generate any memory reads by itself, and so if the place "expression" // contains unsafe operations like raw pointer dereferences or union @@ -142,13 +148,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let source_info = self.source_info(scrutinee_span); self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place); - block.and(scrutinee_place) + block.and(scrutinee_place_builder) } /// Create the initial `Candidate`s for a `match` expression. fn create_match_candidates<'pat>( &mut self, - scrutinee: Place<'tcx>, + scrutinee: PlaceBuilder<'tcx>, arms: &'pat [Arm<'pat, 'tcx>], ) -> Vec<(&'pat Arm<'pat, 'tcx>, Candidate<'pat, 'tcx>)> { // Assemble a list of candidates: there is one candidate per pattern, @@ -156,7 +162,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { arms.iter() .map(|arm| { let arm_has_guard = arm.guard.is_some(); - let arm_candidate = Candidate::new(scrutinee, &arm.pattern, arm_has_guard); + let arm_candidate = Candidate::new(scrutinee.clone(), &arm.pattern, arm_has_guard); (arm, arm_candidate) }) .collect() @@ -222,7 +228,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn lower_match_arms( &mut self, destination: Place<'tcx>, - scrutinee_place: Place<'tcx>, + scrutinee_place: PlaceBuilder<'tcx>, scrutinee_span: Span, arm_candidates: Vec<(&'_ Arm<'_, 'tcx>, Candidate<'_, 'tcx>)>, outer_source_info: SourceInfo, @@ -241,7 +247,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { arm.span, &arm.pattern, ArmHasGuard(arm.guard.is_some()), - Some((Some(&scrutinee_place), scrutinee_span)), + Some(( + Some( + &scrutinee_place.clone().into_place(this.tcx, this.typeck_results), + ), + scrutinee_span, + )), ); let arm_block = this.bind_pattern( @@ -446,8 +457,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } _ => { - let place = unpack!(block = self.as_place(block, initializer)); - self.place_into_pattern(block, irrefutable_pat, place, true) + // Converts the destruct pattern into a place + // + // We don't want to convert to a place right away + // because in case of such pattern inside a closure, the projections matching a + // captured place might have not been applied. + // [FIXME] Need to find where this is happening and make the necessary changes there once + // Candidate is modified + // + // We want to use a place builder; Maybe use `as_place_builder` + let place_builder = unpack!(block = self.as_place_builder(block, initializer)); + self.place_into_pattern(block, irrefutable_pat, place_builder, true) } } } @@ -456,9 +476,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut self, block: BasicBlock, irrefutable_pat: Pat<'tcx>, - initializer: Place<'tcx>, + initializer: PlaceBuilder<'tcx>, set_match_place: bool, ) -> BlockAnd<()> { + let place = initializer.clone().into_place(self.tcx, self.typeck_results); let mut candidate = Candidate::new(initializer, &irrefutable_pat, false); let fake_borrow_temps = @@ -478,7 +499,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. }, )))) = self.local_decls[local].local_info { - *match_place = Some(initializer); + *match_place = Some(place); } else { bug!("Let binding to non-user variable.") } @@ -717,7 +738,7 @@ struct Candidate<'pat, 'tcx> { } impl<'tcx, 'pat> Candidate<'pat, 'tcx> { - fn new(place: Place<'tcx>, pattern: &'pat Pat<'tcx>, has_guard: bool) -> Self { + fn new(place: PlaceBuilder<'tcx>, pattern: &'pat Pat<'tcx>, has_guard: bool) -> Self { Candidate { span: pattern.span, has_guard, @@ -791,7 +812,7 @@ struct Ascription<'tcx> { #[derive(Clone, Debug)] crate struct MatchPair<'pat, 'tcx> { // this place... - place: Place<'tcx>, + place: PlaceBuilder<'tcx>, // ... must match this pattern. pattern: &'pat Pat<'tcx>, @@ -1198,7 +1219,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { &mut otherwise, pats, or_span, - place, + place.clone(), fake_borrows, ); }); @@ -1224,12 +1245,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { otherwise: &mut Option, pats: &'pat [Pat<'tcx>], or_span: Span, - place: Place<'tcx>, + place: PlaceBuilder<'tcx>, fake_borrows: &mut Option>>, ) { debug!("test_or_pattern:\ncandidate={:#?}\npats={:#?}", candidate, pats); - let mut or_candidates: Vec<_> = - pats.iter().map(|pat| Candidate::new(place, pat, candidate.has_guard)).collect(); + let mut or_candidates: Vec<_> = pats + .iter() + .map(|pat| Candidate::new(place.clone(), pat, candidate.has_guard)) + .collect(); let mut or_candidate_refs: Vec<_> = or_candidates.iter_mut().collect(); let otherwise = if candidate.otherwise_block.is_some() { &mut candidate.otherwise_block @@ -1412,7 +1435,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // extract the match-pair from the highest priority candidate let match_pair = &candidates.first().unwrap().match_pairs[0]; let mut test = self.test(match_pair); - let match_place = match_pair.place; + let match_place = match_pair.place.clone(); // most of the time, the test to perform is simply a function // of the main candidate; but for a test like SwitchInt, we @@ -1428,7 +1451,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } TestKind::Switch { adt_def: _, ref mut variants } => { for candidate in candidates.iter() { - if !self.add_variants_to_switch(&match_place, candidate, variants) { + if !self.add_variants_to_switch(&match_place.clone(), candidate, variants) { break; } } @@ -1438,7 +1461,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Insert a Shallow borrow of any places that is switched on. if let Some(fb) = fake_borrows { - fb.insert(match_place); + fb.insert(match_place.clone().into_place(self.tcx, self.typeck_results)); } // perform the test, branching to one of N blocks. For each of @@ -1456,7 +1479,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // encounter a candidate where the test is not relevant; at // that point, we stop sorting. while let Some(candidate) = candidates.first_mut() { - if let Some(idx) = self.sort_candidate(&match_place, &test, candidate) { + if let Some(idx) = self.sort_candidate(&match_place.clone(), &test, candidate) { let (candidate, rest) = candidates.split_first_mut().unwrap(); target_candidates[idx].push(candidate); candidates = rest; @@ -1519,7 +1542,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target_blocks }; - self.perform_test(block, match_place, &test, make_target_blocks); + self.perform_test(block, match_place.clone(), &test, make_target_blocks); } /// Determine the fake borrows that are needed from a set of places that @@ -1753,11 +1776,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } Guard::IfLet(pat, scrutinee) => { let scrutinee_span = scrutinee.span; - let scrutinee_place = - unpack!(block = self.lower_scrutinee(block, scrutinee, scrutinee_span)); - let mut guard_candidate = Candidate::new(scrutinee_place, &pat, false); + let scrutinee_place = unpack!( + block = self.lower_scrutinee(block, scrutinee.clone(), scrutinee_span) + ); + let mut guard_candidate = Candidate::new(scrutinee_place.clone(), &pat, false); let wildcard = Pat::wildcard_from_ty(pat.ty); - let mut otherwise_candidate = Candidate::new(scrutinee_place, &wildcard, false); + let mut otherwise_candidate = + Candidate::new(scrutinee_place.clone(), &wildcard, false); let fake_borrow_temps = self.lower_match_tree( block, pat.span, @@ -1769,7 +1794,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pat.span.to(arm_span.unwrap()), pat, ArmHasGuard(false), - Some((Some(&scrutinee_place), scrutinee.span)), + Some(( + Some(&scrutinee_place.clone().into_place(tcx, self.typeck_results)), + scrutinee.span, + )), ); let post_guard_block = self.bind_pattern( self.source_info(pat.span), diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index 9931cdf3b9e91..d9c1d4abf0700 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -12,11 +12,11 @@ //! sort of test: for example, testing which variant an enum is, or //! testing a value against a constant. +use crate::build::expr::as_place::PlaceBuilder; use crate::build::matches::{Ascription, Binding, Candidate, MatchPair}; use crate::build::Builder; use crate::thir::{self, *}; use rustc_hir::RangeEnd; -use rustc_middle::mir::Place; use rustc_middle::ty; use rustc_middle::ty::layout::IntegerExt; use rustc_target::abi::{Integer, Size}; @@ -68,11 +68,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let match_pairs = mem::take(&mut candidate.match_pairs); if let [MatchPair { pattern: Pat { kind: box PatKind::Or { pats }, .. }, place }] = - *match_pairs + &*match_pairs.clone() { existing_bindings.extend_from_slice(&new_bindings); mem::swap(&mut candidate.bindings, &mut existing_bindings); - candidate.subcandidates = self.create_or_subcandidates(candidate, place, pats); + candidate.subcandidates = + self.create_or_subcandidates(candidate, place.clone(), pats); return true; } @@ -125,12 +126,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn create_or_subcandidates<'pat>( &mut self, candidate: &Candidate<'pat, 'tcx>, - place: Place<'tcx>, + place: PlaceBuilder<'tcx>, pats: &'pat [Pat<'tcx>], ) -> Vec> { pats.iter() .map(|pat| { - let mut candidate = Candidate::new(place, pat, candidate.has_guard); + let mut candidate = Candidate::new(place.clone(), pat, candidate.has_guard); self.simplify_candidate(&mut candidate); candidate }) @@ -148,6 +149,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'pat, 'tcx>, ) -> Result<(), MatchPair<'pat, 'tcx>> { let tcx = self.tcx; + // Generate place to be used in Ascription + // Generate place to be used in Binding + let place = match_pair.place.clone().into_place(tcx, self.typeck_results); match *match_pair.pattern.kind { PatKind::AscribeUserType { ref subpattern, @@ -158,7 +162,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate.ascriptions.push(Ascription { span: user_ty_span, user_ty, - source: match_pair.place, + source: place, variance, }); @@ -177,7 +181,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { name, mutability, span: match_pair.pattern.span, - source: match_pair.place, + source: place, var_id: var, var_ty: ty, binding_mode: mode, @@ -264,8 +268,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { }) && (adt_def.did.is_local() || !adt_def.is_variant_list_non_exhaustive()); if irrefutable { - let place = tcx.mk_place_downcast(match_pair.place, adt_def, variant_index); - candidate.match_pairs.extend(self.field_match_pairs(place, subpatterns)); + let place_builder = match_pair.place.downcast(adt_def, variant_index); + candidate + .match_pairs + .extend(self.field_match_pairs(place_builder, subpatterns)); Ok(()) } else { Err(match_pair) @@ -290,8 +296,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } PatKind::Deref { ref subpattern } => { - let place = tcx.mk_place_deref(match_pair.place); - candidate.match_pairs.push(MatchPair::new(place, subpattern)); + let place_builder = match_pair.place.deref(); + candidate.match_pairs.push(MatchPair::new(place_builder, subpattern)); Ok(()) } diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 48abaa8d35f82..b804cd4574fa7 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -5,6 +5,7 @@ // identify what tests are needed, perform the tests, and then filter // the candidates based on the result. +use crate::build::expr::as_place::PlaceBuilder; use crate::build::matches::{Candidate, MatchPair, Test, TestKind}; use crate::build::Builder; use crate::thir::pattern::compare_const_vals; @@ -81,7 +82,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(super) fn add_cases_to_switch<'pat>( &mut self, - test_place: &Place<'tcx>, + test_place: &PlaceBuilder<'tcx>, candidate: &Candidate<'pat, 'tcx>, switch_ty: Ty<'tcx>, options: &mut FxIndexMap<&'tcx ty::Const<'tcx>, u128>, @@ -123,7 +124,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(super) fn add_variants_to_switch<'pat>( &mut self, - test_place: &Place<'tcx>, + test_place: &PlaceBuilder<'tcx>, candidate: &Candidate<'pat, 'tcx>, variants: &mut BitSet, ) -> bool { @@ -151,10 +152,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(super) fn perform_test( &mut self, block: BasicBlock, - place: Place<'tcx>, + place_builder: PlaceBuilder<'tcx>, test: &Test<'tcx>, make_target_blocks: impl FnOnce(&mut Self) -> Vec, ) { + let place = place_builder.clone().into_place(self.tcx, self.typeck_results); debug!( "perform_test({:?}, {:?}: {:?}, {:?})", block, @@ -481,7 +483,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { /// tighter match code if we do something a bit different. pub(super) fn sort_candidate<'pat>( &mut self, - test_place: &Place<'tcx>, + test_place: &PlaceBuilder<'tcx>, test: &Test<'tcx>, candidate: &mut Candidate<'pat, 'tcx>, ) -> Option { @@ -728,7 +730,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'pat, 'tcx>, ) { let match_pair = candidate.match_pairs.remove(match_pair_index); - let tcx = self.tcx; // So, if we have a match-pattern like `x @ Enum::Variant(P1, P2)`, // we want to create a set of derived match-patterns like @@ -737,10 +738,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { Some(adt_def.variants[variant_index].ident.name), variant_index, ); - let downcast_place = tcx.mk_place_elem(match_pair.place, elem); // `(x as Variant)` + let downcast_place = match_pair.place.project(elem); // `(x as Variant)` let consequent_match_pairs = subpatterns.iter().map(|subpattern| { // e.g., `(x as Variant).0` - let place = tcx.mk_place_field(downcast_place, subpattern.field, subpattern.pattern.ty); + let place = downcast_place.clone().field(subpattern.field, subpattern.pattern.ty); // e.g., `(x as Variant).0 @ P1` MatchPair::new(place, &subpattern.pattern) }); diff --git a/compiler/rustc_mir_build/src/build/matches/util.rs b/compiler/rustc_mir_build/src/build/matches/util.rs index 15aca0203aa01..d49a00a566053 100644 --- a/compiler/rustc_mir_build/src/build/matches/util.rs +++ b/compiler/rustc_mir_build/src/build/matches/util.rs @@ -1,3 +1,4 @@ +use crate::build::expr::as_place::PlaceBuilder; use crate::build::matches::MatchPair; use crate::build::Builder; use crate::thir::*; @@ -9,13 +10,13 @@ use std::convert::TryInto; impl<'a, 'tcx> Builder<'a, 'tcx> { crate fn field_match_pairs<'pat>( &mut self, - place: Place<'tcx>, + place: PlaceBuilder<'tcx>, subpatterns: &'pat [FieldPat<'tcx>], ) -> Vec> { subpatterns .iter() .map(|fieldpat| { - let place = self.tcx.mk_place_field(place, fieldpat.field, fieldpat.pattern.ty); + let place = place.clone().field(fieldpat.field, fieldpat.pattern.ty); MatchPair::new(place, &fieldpat.pattern) }) .collect() @@ -24,13 +25,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { crate fn prefix_slice_suffix<'pat>( &mut self, match_pairs: &mut SmallVec<[MatchPair<'pat, 'tcx>; 1]>, - place: &Place<'tcx>, + place: &PlaceBuilder<'tcx>, prefix: &'pat [Pat<'tcx>], opt_slice: Option<&'pat Pat<'tcx>>, suffix: &'pat [Pat<'tcx>], ) { let tcx = self.tcx; - let (min_length, exact_size) = match place.ty(&self.local_decls, tcx).ty.kind() { + let (min_length, exact_size) = match place + .clone() + .into_place(tcx, self.typeck_results) + .ty(&self.local_decls, tcx) + .ty + .kind() + { ty::Array(_, length) => (length.eval_usize(tcx, self.param_env), true), _ => ((prefix.len() + suffix.len()).try_into().unwrap(), false), }; @@ -38,20 +45,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { match_pairs.extend(prefix.iter().enumerate().map(|(idx, subpattern)| { let elem = ProjectionElem::ConstantIndex { offset: idx as u64, min_length, from_end: false }; - let place = tcx.mk_place_elem(*place, elem); + let place = place.clone().project(elem); MatchPair::new(place, subpattern) })); if let Some(subslice_pat) = opt_slice { let suffix_len = suffix.len() as u64; - let subslice = tcx.mk_place_elem( - *place, - ProjectionElem::Subslice { - from: prefix.len() as u64, - to: if exact_size { min_length - suffix_len } else { suffix_len }, - from_end: !exact_size, - }, - ); + let subslice = place.clone().project(ProjectionElem::Subslice { + from: prefix.len() as u64, + to: if exact_size { min_length - suffix_len } else { suffix_len }, + from_end: !exact_size, + }); match_pairs.push(MatchPair::new(subslice, subslice_pat)); } @@ -62,7 +66,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { min_length, from_end: !exact_size, }; - let place = tcx.mk_place_elem(*place, elem); + let place = place.clone().project(elem); MatchPair::new(place, subpattern) })); } @@ -91,7 +95,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { - crate fn new(place: Place<'tcx>, pattern: &'pat Pat<'tcx>) -> MatchPair<'pat, 'tcx> { + crate fn new(place: PlaceBuilder<'tcx>, pattern: &'pat Pat<'tcx>) -> MatchPair<'pat, 'tcx> { MatchPair { place, pattern } } } diff --git a/compiler/rustc_mir_build/src/build/mod.rs b/compiler/rustc_mir_build/src/build/mod.rs index 9c83c0d09aa8e..4bc497f5ce29c 100644 --- a/compiler/rustc_mir_build/src/build/mod.rs +++ b/compiler/rustc_mir_build/src/build/mod.rs @@ -1,4 +1,5 @@ use crate::build; +use crate::build::expr::as_place::PlaceBuilder; use crate::build::scope::DropKind; use crate::thir::{build_thir, Arena, BindingMode, Expr, LintLevel, Pat, PatKind}; use rustc_attr::{self as attr, UnwindAttr}; @@ -1004,7 +1005,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { matches::ArmHasGuard(false), Some((Some(&place), span)), ); - unpack!(block = self.place_into_pattern(block, pattern, place, false)); + let place_builder = PlaceBuilder::from(local); + unpack!( + block = self.place_into_pattern(block, pattern, place_builder, false) + ); } } self.source_scope = original_source_scope; From b6cf070eb4c228c146ca9971cddeb034084f88de Mon Sep 17 00:00:00 2001 From: Roxane Date: Sun, 21 Feb 2021 10:20:40 -0500 Subject: [PATCH 3/8] Attempt to deal with nested closures properly --- compiler/rustc_typeck/src/check/upvar.rs | 7 +- compiler/rustc_typeck/src/expr_use_visitor.rs | 40 ++++-- ...tructure-pattern-closure-within-closure.rs | 21 ++++ ...ture-pattern-closure-within-closure.stderr | 110 ++++++++++++++++ .../run_pass/destructure_patterns-1.rs | 118 ++++++++++++++++++ .../run_pass/destructure_patterns-1.stderr | 42 +++++++ .../run_pass/destructure_patterns.rs | 52 ++++++++ .../run_pass/destructure_patterns.stderr | 61 +++++++++ .../no_capture_with_wildcard_match.rs | 12 ++ .../no_capture_with_wildcard_match.stderr | 32 +++++ .../run_pass/struct_update_syntax.rs | 23 ++++ .../run_pass/struct_update_syntax.stderr | 57 +++++++++ 12 files changed, 560 insertions(+), 15 deletions(-) create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 74e2ca51039d3..932f07ff1bd1b 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -1153,7 +1153,6 @@ 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>, // these need to be fake read. } @@ -1416,9 +1415,9 @@ 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 fake_read(&mut self, place: Place<'tcx>) { + if let PlaceBase::Upvar(_) = place.base { + self.fake_reads.insert(place); } } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 45ecf30cd50e3..ad39b93c067ee 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -5,7 +5,7 @@ pub use self::ConsumeMode::*; // Export these here so that Clippy can use them. -pub use rustc_middle::hir::place::{PlaceBase, PlaceWithHirId, Projection}; +pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection}; use rustc_hir as hir; use rustc_hir::def::Res; @@ -54,7 +54,7 @@ pub trait Delegate<'tcx> { 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>); + fn fake_read(&mut self, place: Place<'tcx>); } #[derive(Copy, Clone, PartialEq, Debug)] @@ -558,7 +558,7 @@ 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()); + self.delegate.fake_read(discr_place.place.clone()); let tcx = self.tcx(); let ExprUseVisitor { ref mc, body_owner: _, ref mut delegate } = *self; @@ -620,8 +620,6 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// - When reporting the Place back to the Delegate, ensure that the UpvarId uses the enclosing /// closure as the DefId. 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. @@ -635,6 +633,32 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { ty::Closure(..) | ty::Generator(..) ); + // [FIXME] RFC2229 Closures within closures don't work + if let Some(fake_reads) = self.mc.typeck_results.closure_fake_reads.get(&closure_def_id) { + for fake_read in fake_reads.iter() { + // Use this as a reference for if we should promote the fake read + match fake_read.base { + PlaceBase::Upvar(upvar_id) => { + if upvars.map_or(body_owner_is_closure, |upvars| { + !upvars.contains_key(&upvar_id.var_path.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 + // then for the current body (if it's a closure) these aren't captures, we will ignore them. + continue; + } + } + _ => { + bug!( + "Do not know how to get HirId out of Rvalue and StaticItem {:?}", + fake_read.base + ); + } + }; + self.delegate.fake_read(fake_read.clone()); + } + } + 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() { @@ -664,12 +688,6 @@ 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/run_pass/destructure-pattern-closure-within-closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs new file mode 100644 index 0000000000000..db067b6c9bca3 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs @@ -0,0 +1,21 @@ +#![feature(capture_disjoint_fields)] +#![feature(rustc_attrs)] + +fn main() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + let g = (String::from("Mr"), String::from("Goose")); + + let a = #[rustc_capture_analysis] || { + let (_, g2) = g; + println!("{}", g2); + let c = #[rustc_capture_analysis] || { + let (_, t2) = t; + println!("{}", t2); + }; + + c(); + }; + + a(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr new file mode 100644 index 0000000000000..c9a1a32fc579b --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr @@ -0,0 +1,110 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/destructure-pattern-closure-within-closure.rs:9:13 + | +LL | let a = #[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/destructure-pattern-closure-within-closure.rs:12:17 + | +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 + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/destructure-pattern-closure-within-closure.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/destructure-pattern-closure-within-closure.rs:12:43 + | +LL | let c = #[rustc_capture_analysis] || { + | ___________________________________________^ +LL | | let (_, t2) = t; +LL | | println!("{}", t2); +LL | | }; + | |_________^ + | +note: Capturing t[(1, 0)] -> ByValue + --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 + | +LL | let (_, t2) = t; + | ^ + +error: Min Capture analysis includes: + --> $DIR/destructure-pattern-closure-within-closure.rs:12:43 + | +LL | let c = #[rustc_capture_analysis] || { + | ___________________________________________^ +LL | | let (_, t2) = t; +LL | | println!("{}", t2); +LL | | }; + | |_________^ + | +note: Min Capture t[(1, 0)] -> ByValue + --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 + | +LL | let (_, t2) = t; + | ^ + +error: First Pass analysis includes: + --> $DIR/destructure-pattern-closure-within-closure.rs:9:39 + | +LL | let a = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | let (_, g2) = g; +LL | | println!("{}", g2); +LL | | let c = #[rustc_capture_analysis] || { +... | +LL | | c(); +LL | | }; + | |_____^ + | +note: Capturing g[(1, 0)] -> ByValue + --> $DIR/destructure-pattern-closure-within-closure.rs:10:23 + | +LL | let (_, g2) = g; + | ^ +note: Capturing t[(1, 0)] -> ByValue + --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 + | +LL | let (_, t2) = t; + | ^ + +error: Min Capture analysis includes: + --> $DIR/destructure-pattern-closure-within-closure.rs:9:39 + | +LL | let a = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | let (_, g2) = g; +LL | | println!("{}", g2); +LL | | let c = #[rustc_capture_analysis] || { +... | +LL | | c(); +LL | | }; + | |_____^ + | +note: Min Capture g[(1, 0)] -> ByValue + --> $DIR/destructure-pattern-closure-within-closure.rs:10:23 + | +LL | let (_, g2) = g; + | ^ +note: Min Capture t[(1, 0)] -> ByValue + --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 + | +LL | let (_, t2) = t; + | ^ + +error: aborting due to 6 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs new file mode 100644 index 0000000000000..757b506fd6ede --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs @@ -0,0 +1,118 @@ +//check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![warn(unused)] + +struct Point { + x: u32, + y: u32, +} + +fn test1() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (t1, t2) = t; + println!("{} {}", t1, t2); + }; + + c(); +} + +fn test2() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (t1, _) = t; + println!("{}", t1); + }; + + c(); +} + +fn test3() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (_, t2) = t; + println!("{}", t2); + }; + + c(); +} + +fn test4() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + //~^ WARN unused variable: `t` + + let c = || { + let (_, _) = t; + }; + + c(); +} + +fn test5() { + let _z = 9; + let t = (String::new(), String::new()); + let _c = || { + let _a = match t { + (t1, _) => t1, + }; + }; +} + +fn test6() { + let _z = 9; + let t = (String::new(), String::new()); + let _c = || { + let _a = match t { + (_, t2) => t2, + }; + }; +} + +fn test7() { + let x = 0; + //~^ WARN unused variable: `x` + let tup = (1, 2); + //~^ WARN unused variable: `tup` + let p = Point { x: 10, y: 20 }; + + let c = || { + let _ = x; + let Point { x, y } = p; // 1 + //~^ WARN unused variable: `x` + println!("{}", y); + let (_, _) = tup; // 2 + }; + + c(); +} + +fn test8() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (_, t) = t; + println!("{}", t); + }; + + c(); +} + +fn main() { + test1(); + test2(); + test3(); + test4(); + test5(); + test6(); + test7(); + test8(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr new file mode 100644 index 0000000000000..1ae64eb83ef78 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr @@ -0,0 +1,42 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/destructure_patterns-1.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: unused variable: `t` + --> $DIR/destructure_patterns-1.rs:49:9 + | +LL | let t = (String::from("Hello"), String::from("World")); + | ^ help: if this is intentional, prefix it with an underscore: `_t` + | +note: the lint level is defined here + --> $DIR/destructure_patterns-1.rs:4:9 + | +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` + +warning: unused variable: `x` + --> $DIR/destructure_patterns-1.rs:88:21 + | +LL | let Point { x, y } = p; // 1 + | ^ help: try ignoring the field: `x: _` + +warning: unused variable: `x` + --> $DIR/destructure_patterns-1.rs:80:9 + | +LL | let x = 0; + | ^ help: if this is intentional, prefix it with an underscore: `_x` + +warning: unused variable: `tup` + --> $DIR/destructure_patterns-1.rs:82:9 + | +LL | let tup = (1, 2); + | ^^^ help: if this is intentional, prefix it with an underscore: `_tup` + +warning: 5 warnings emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs new file mode 100644 index 0000000000000..6f958bb18c387 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs @@ -0,0 +1,52 @@ +#![feature(capture_disjoint_fields)] +#![feature(rustc_attrs)] + +struct S { + a: String, + b: String, +} + +fn main() { + let t = (String::new(), String::new()); + + let s = S { + a: String::new(), + b: String::new(), + }; + + let c = #[rustc_capture_analysis] || { + let (t1, t2) = t; + }; + + + // MIR Build + // + // Create place for the initalizer in let which is `t` + // + // I'm reading Field 1 from `t`, so apply Field projections; + // + // new place -> t[1] + // + // I'm reading Field 2 from `t`, so apply Field projections; + // + // new place -> t[2] + // + // New + // --------- + // + // I'm building something starting at `t` + // + // I read field 1 from `t` + // + // I need to use `t[1]`, therefore the place must be constructable + // + // Find the capture index for `t[1]` for this closure. + // + // I read field 2 from `t` + // + // I need to use `t[2]`, therefore the place must be constructable + // + // Find the capture index for `t[2]` for this closure. + + c(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr new file mode 100644 index 0000000000000..1ee5d2abf133a --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr @@ -0,0 +1,61 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/destructure_patterns.rs:17: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 + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/destructure_patterns.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/destructure_patterns.rs:17:39 + | +LL | let c = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | let (t1, t2) = t; +LL | | }; + | |_____^ + | +note: Capturing t[(0, 0)] -> ByValue + --> $DIR/destructure_patterns.rs:18:24 + | +LL | let (t1, t2) = t; + | ^ +note: Capturing t[(1, 0)] -> ByValue + --> $DIR/destructure_patterns.rs:18:24 + | +LL | let (t1, t2) = t; + | ^ + +error: Min Capture analysis includes: + --> $DIR/destructure_patterns.rs:17:39 + | +LL | let c = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | let (t1, t2) = t; +LL | | }; + | |_____^ + | +note: Min Capture t[(0, 0)] -> ByValue + --> $DIR/destructure_patterns.rs:18:24 + | +LL | let (t1, t2) = t; + | ^ +note: Min Capture t[(1, 0)] -> ByValue + --> $DIR/destructure_patterns.rs:18:24 + | +LL | let (t1, t2) = t; + | ^ + +error: aborting due to 3 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs new file mode 100644 index 0000000000000..1880351610176 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs @@ -0,0 +1,12 @@ +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![feature(rustc_attrs)] + +fn main() { + let foo = [1, 2, 3]; + let c = #[rustc_capture_analysis] || { + //~^ ERROR: attributes on expressions are experimental + //~| ERROR: First Pass analysis includes: + match foo { _ => () }; + }; +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr new file mode 100644 index 0000000000000..e2e825fe9425e --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr @@ -0,0 +1,32 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/no_capture_with_wildcard_match.rs:7: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 + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/no_capture_with_wildcard_match.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/no_capture_with_wildcard_match.rs:7:39 + | +LL | let c = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | +LL | | +LL | | match foo { _ => () }; +LL | | }; + | |_____^ + +error: aborting due to 2 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs new file mode 100644 index 0000000000000..9757860fb4c7f --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs @@ -0,0 +1,23 @@ +#![feature(capture_disjoint_fields)] +#![feature(rustc_attrs)] + +struct S { + a: String, + b: String, +} + +fn main() { + let s = S { + a: String::new(), + b: String::new(), + }; + + let c = #[rustc_capture_analysis] || { + let s2 = S { + a: format!("New a"), + ..s + }; + }; + + c(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr new file mode 100644 index 0000000000000..15d8040c46c51 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr @@ -0,0 +1,57 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/struct_update_syntax.rs:15: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 + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/struct_update_syntax.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/struct_update_syntax.rs:15:39 + | +LL | let c = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | let s2 = S { +LL | | a: format!("New a"), +LL | | ..s +LL | | }; +LL | | }; + | |_____^ + | +note: Capturing s[(1, 0)] -> ByValue + --> $DIR/struct_update_syntax.rs:18:15 + | +LL | ..s + | ^ + +error: Min Capture analysis includes: + --> $DIR/struct_update_syntax.rs:15:39 + | +LL | let c = #[rustc_capture_analysis] || { + | _______________________________________^ +LL | | let s2 = S { +LL | | a: format!("New a"), +LL | | ..s +LL | | }; +LL | | }; + | |_____^ + | +note: Min Capture s[(1, 0)] -> ByValue + --> $DIR/struct_update_syntax.rs:18:15 + | +LL | ..s + | ^ + +error: aborting due to 3 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. From 685a4c6b6b6016fa0c4b1ae4af745103add0bb02 Mon Sep 17 00:00:00 2001 From: Roxane Date: Tue, 23 Feb 2021 17:55:36 -0500 Subject: [PATCH 4/8] Use the correct FakeReadCause --- compiler/rustc_middle/src/ty/context.rs | 4 +- compiler/rustc_mir_build/src/build/block.rs | 10 -- .../src/build/expr/as_place.rs | 34 ++-- .../src/build/expr/as_rvalue.rs | 33 ++-- .../rustc_mir_build/src/build/expr/into.rs | 1 - .../rustc_mir_build/src/build/matches/mod.rs | 90 ++++++----- .../src/build/matches/simplify.rs | 5 +- .../rustc_mir_build/src/build/matches/test.rs | 9 +- compiler/rustc_mir_build/src/thir/cx/expr.rs | 103 +++++------- compiler/rustc_mir_build/src/thir/mod.rs | 4 +- compiler/rustc_typeck/src/check/upvar.rs | 11 +- compiler/rustc_typeck/src/check/writeback.rs | 15 +- compiler/rustc_typeck/src/expr_use_visitor.rs | 37 +++-- ...e-origin-single-variant-diagnostics.stderr | 13 -- ...tructure-pattern-closure-within-closure.rs | 12 +- ...ture-pattern-closure-within-closure.stderr | 106 ++----------- .../run_pass/destructure_patterns-1.rs | 118 -------------- .../run_pass/destructure_patterns-1.stderr | 42 ----- .../run_pass/destructure_patterns.rs | 148 +++++++++++++----- .../run_pass/destructure_patterns.stderr | 87 +++++----- .../no_capture_with_wildcard_match.rs | 23 ++- .../no_capture_with_wildcard_match.stderr | 25 +-- .../run_pass/struct_update_syntax.rs | 4 +- .../run_pass/struct_update_syntax.stderr | 50 +----- .../borrowck-move-ref-pattern.stderr | 25 +-- 25 files changed, 391 insertions(+), 618 deletions(-) delete mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs delete mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 2d1231d819d38..425e62d6f61a9 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -47,6 +47,7 @@ use rustc_hir::{ }; use rustc_index::vec::{Idx, IndexVec}; use rustc_macros::HashStable; +use rustc_middle::mir::FakeReadCause; use rustc_serialize::opaque::{FileEncodeResult, FileEncoder}; use rustc_session::config::{BorrowckMode, CrateType, OutputFilenames}; use rustc_session::lint::{Level, Lint}; @@ -430,8 +431,7 @@ 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>>, + pub closure_fake_reads: FxHashMap, FakeReadCause)>>, /// Stores the type, expression, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). diff --git a/compiler/rustc_mir_build/src/build/block.rs b/compiler/rustc_mir_build/src/build/block.rs index ae4d6b56152c5..808c6e3ff644b 100644 --- a/compiler/rustc_mir_build/src/build/block.rs +++ b/compiler/rustc_mir_build/src/build/block.rs @@ -94,14 +94,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) ); } - // ROX: - // - // Where the handling of destructure patterns start - // - // let (a, b, c, _ ) = something - // - // (a, b, c, _) is the pattern - // something is the initializer StmtKind::Let { remainder_scope, init_scope, pattern, initializer, lint_level } => { let ignores_expr_result = matches!(*pattern.kind, PatKind::Wild); this.block_context.push(BlockFrame::Statement { ignores_expr_result }); @@ -133,8 +125,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ArmHasGuard(false), Some((None, initializer_span)), ); - // This is where we get into pattern handling of the let - // statement this.expr_into_pattern(block, pattern.clone(), init) }) } 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 f819a638c7aa0..6519d5fae1c7f 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -84,6 +84,7 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>( mir_projections: &[PlaceElem<'tcx>], ) -> Vec { let mut hir_projections = Vec::new(); + let mut variant = None; for mir_projection in mir_projections { let hir_projection = match mir_projection { @@ -91,13 +92,17 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>( ProjectionElem::Field(field, _) => { // We will never encouter this for multivariant enums, // read the comment for `Downcast`. - HirProjectionKind::Field(field.index() as u32, VariantIdx::new(0)) + let variant = variant.unwrap_or(VariantIdx::new(0)); + HirProjectionKind::Field(field.index() as u32, variant) } - ProjectionElem::Downcast(..) => { - // This projections exist only for enums that have - // multiple variants. Since such enums that are captured - // completely, we can stop here. - break; + ProjectionElem::Downcast(.., idx) => { + // This projections exist for enums that have + // single and multiple variants. + // For single variants, enums are not captured completely. + // We keep track of VariantIdx so we can use this information + // if the next ProjectionElem is a Field + variant = Some(*idx); + continue; } ProjectionElem::Index(..) | ProjectionElem::ConstantIndex { .. } @@ -107,7 +112,7 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>( break; } }; - + variant = None; hir_projections.push(hir_projection); } @@ -231,13 +236,12 @@ fn to_upvars_resolved_place_builder<'a, 'tcx>( from_builder.projection ) } else { - // FIXME(project-rfc-2229#24): Handle this case properly debug!( "No associated capture found for {:?}[{:#?}]", var_hir_id, from_builder.projection, ); } - return Err(upvar_resolved_place_builder); + return Err(from_builder); }; let closure_ty = typeck_results @@ -289,11 +293,10 @@ impl<'tcx> PlaceBuilder<'tcx> { if let PlaceBase::Local(local) = self.base { Place { local, projection: tcx.intern_place_elems(&self.projection) } } else { - self.try_upvars_resolved(tcx, typeck_results).into_place(tcx, typeck_results) + self.expect_upvars_resolved(tcx, typeck_results).into_place(tcx, typeck_results) } } - /// ROX: Function that will be called when we really do need a place fn expect_upvars_resolved<'a>( self, tcx: TyCtxt<'tcx>, @@ -302,14 +305,14 @@ impl<'tcx> PlaceBuilder<'tcx> { to_upvars_resolved_place_builder(self, tcx, typeck_results).unwrap() } - fn try_upvars_resolved<'a>( + crate fn try_upvars_resolved<'a>( self, tcx: TyCtxt<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>, - ) -> PlaceBuilder<'tcx> { + ) -> Result, PlaceBuilder<'tcx>> { match to_upvars_resolved_place_builder(self, tcx, typeck_results) { - Ok(upvars_resolved) => upvars_resolved, - Err(upvars_unresolved) => upvars_unresolved, + Ok(upvars_resolved) => Ok(upvars_resolved), + Err(upvars_unresolved) => Err(upvars_unresolved), } } @@ -376,7 +379,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(place_builder.into_place(self.tcx, self.typeck_results)) } - // ROX: As place builder /// This is used when constructing a compound `Place`, so that we can avoid creating /// intermediate `Place` values until we know the full set of projections. crate fn as_place_builder( 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 3a8665777b79e..c7f16fd4e4049 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -165,7 +165,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields)) } - ExprKind::Closure { closure_id, substs, upvars, movability, fake_reads } => { + ExprKind::Closure { + closure_id, + substs, + upvars, + movability, + fake_reads: opt_fake_reads, + } => { // see (*) above let operands: Vec<_> = upvars .into_iter() @@ -204,18 +210,19 @@ 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, - ); + if let Some(fake_reads) = opt_fake_reads { + for (thir_place, cause) in fake_reads.into_iter() { + let place_builder = + unpack!(block = this.as_place_builder(block, thir_place)); + + if let Ok(place_builder_resolved) = + place_builder.clone().try_upvars_resolved(this.tcx, this.typeck_results) + { + let mir_place = place_builder_resolved + .clone() + .into_place(this.tcx, this.typeck_results); + this.cfg.push_fake_read(block, source_info, cause, mir_place); + } } } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index e0ad34e08efd4..b2e8b2de1bc6a 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -280,7 +280,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let field_names: Vec<_> = (0..adt_def.variants[variant_index].fields.len()).map(Field::new).collect(); - // ROX: This is probably here the function record/struct update pattern is done. let fields: Vec<_> = if let Some(FruInfo { base, field_types }) = base { let place_builder = unpack!(block = this.as_place_builder(block, base)); diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 7285cf756234d..59b3fd8647897 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -1,6 +1,3 @@ -// ROX: this folder contains all code for handling patterns, including exhaustiveness checking etc. -// We want to be careful ^^' - //! Code related to match expressions. These are sufficiently complex to //! warrant their own module and submodules. :) This main module includes the //! high-level algorithm, the submodules contain the details. @@ -100,7 +97,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let scrutinee_place = unpack!(block = self.lower_scrutinee(block, scrutinee, scrutinee_span,)); - let mut arm_candidates = self.create_match_candidates(scrutinee_place.clone(), &arms); + let mut arm_candidates = + self.create_match_candidates(scrutinee_place.clone(), &arms.clone()); let match_has_guard = arms.iter().any(|arm| arm.guard.is_some()); let mut candidates = @@ -127,8 +125,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { scrutinee_span: Span, ) -> BlockAnd> { let scrutinee_place_builder = unpack!(block = self.as_place_builder(block, scrutinee)); - let scrutinee_place = - scrutinee_place_builder.clone().into_place(self.tcx, self.typeck_results); // Matching on a `scrutinee_place` with an uninhabited type doesn't // generate any memory reads by itself, and so if the place "expression" // contains unsafe operations like raw pointer dereferences or union @@ -146,7 +142,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // check safety. let cause_matched_place = FakeReadCause::ForMatchedPlace; let source_info = self.source_info(scrutinee_span); - self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place); + + if let Ok(scrutinee_builder) = + scrutinee_place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results) + { + let scrutinee_place = + scrutinee_builder.clone().into_place(self.tcx, self.typeck_results); + self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place); + } block.and(scrutinee_place_builder) } @@ -228,7 +231,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { fn lower_match_arms( &mut self, destination: Place<'tcx>, - scrutinee_place: PlaceBuilder<'tcx>, + scrutinee_place_builder: PlaceBuilder<'tcx>, scrutinee_span: Span, arm_candidates: Vec<(&'_ Arm<'_, 'tcx>, Candidate<'_, 'tcx>)>, outer_source_info: SourceInfo, @@ -242,17 +245,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let arm_source_info = self.source_info(arm.span); let arm_scope = (arm.scope, arm_source_info); self.in_scope(arm_scope, arm.lint_level, |this| { + let body = arm.body.clone(); + let mut opt_scrutinee_place: Option<(Option<&Place<'tcx>>, Span)> = None; + let scrutinee_place: Place<'tcx>; + if let Ok(scrutinee_builder) = scrutinee_place_builder + .clone() + .try_upvars_resolved(this.tcx, this.typeck_results) + { + scrutinee_place = + scrutinee_builder.clone().into_place(this.tcx, this.typeck_results); + opt_scrutinee_place = Some((Some(&scrutinee_place), scrutinee_span)); + } let scope = this.declare_bindings( None, arm.span, &arm.pattern, ArmHasGuard(arm.guard.is_some()), - Some(( - Some( - &scrutinee_place.clone().into_place(this.tcx, this.typeck_results), - ), - scrutinee_span, - )), + opt_scrutinee_place, ); let arm_block = this.bind_pattern( @@ -457,15 +466,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } _ => { - // Converts the destruct pattern into a place - // - // We don't want to convert to a place right away - // because in case of such pattern inside a closure, the projections matching a - // captured place might have not been applied. - // [FIXME] Need to find where this is happening and make the necessary changes there once - // Candidate is modified - // - // We want to use a place builder; Maybe use `as_place_builder` let place_builder = unpack!(block = self.as_place_builder(block, initializer)); self.place_into_pattern(block, irrefutable_pat, place_builder, true) } @@ -479,12 +479,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { initializer: PlaceBuilder<'tcx>, set_match_place: bool, ) -> BlockAnd<()> { - let place = initializer.clone().into_place(self.tcx, self.typeck_results); - let mut candidate = Candidate::new(initializer, &irrefutable_pat, false); - + let mut candidate = Candidate::new(initializer.clone(), &irrefutable_pat, false); let fake_borrow_temps = self.lower_match_tree(block, irrefutable_pat.span, false, &mut [&mut candidate]); - // For matches and function arguments, the place that is being matched // can be set when creating the variables. But the place for // let PATTERN = ... might not even exist until we do the assignment. @@ -499,7 +496,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. }, )))) = self.local_decls[local].local_info { - *match_place = Some(place); + if let Ok(match_pair_resolved) = + initializer.clone().try_upvars_resolved(self.tcx, self.typeck_results) + { + let place = match_pair_resolved + .clone() + .into_place(self.tcx, self.typeck_results); + *match_place = Some(place); + } } else { bug!("Let binding to non-user variable.") } @@ -1461,7 +1465,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Insert a Shallow borrow of any places that is switched on. if let Some(fb) = fake_borrows { - fb.insert(match_place.clone().into_place(self.tcx, self.typeck_results)); + if let Ok(match_place_resolved) = + match_place.clone().try_upvars_resolved(self.tcx, self.typeck_results) + { + let resolved_place = + match_place_resolved.clone().into_place(self.tcx, self.typeck_results); + fb.insert(resolved_place); + } } // perform the test, branching to one of N blocks. For each of @@ -1776,28 +1786,36 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } Guard::IfLet(pat, scrutinee) => { let scrutinee_span = scrutinee.span; - let scrutinee_place = unpack!( + let scrutinee_place_builder = unpack!( block = self.lower_scrutinee(block, scrutinee.clone(), scrutinee_span) ); - let mut guard_candidate = Candidate::new(scrutinee_place.clone(), &pat, false); + let mut guard_candidate = + Candidate::new(scrutinee_place_builder.clone(), &pat, false); let wildcard = Pat::wildcard_from_ty(pat.ty); let mut otherwise_candidate = - Candidate::new(scrutinee_place.clone(), &wildcard, false); + Candidate::new(scrutinee_place_builder.clone(), &wildcard, false); let fake_borrow_temps = self.lower_match_tree( block, pat.span, false, &mut [&mut guard_candidate, &mut otherwise_candidate], ); + let mut opt_scrutinee_place: Option<(Option<&Place<'tcx>>, Span)> = None; + let scrutinee_place: Place<'tcx>; + if let Ok(scrutinee_builder) = scrutinee_place_builder + .clone() + .try_upvars_resolved(self.tcx, self.typeck_results) + { + scrutinee_place = + scrutinee_builder.clone().into_place(self.tcx, self.typeck_results); + opt_scrutinee_place = Some((Some(&scrutinee_place), scrutinee_span)); + } self.declare_bindings( None, pat.span.to(arm_span.unwrap()), pat, ArmHasGuard(false), - Some(( - Some(&scrutinee_place.clone().into_place(tcx, self.typeck_results)), - scrutinee.span, - )), + opt_scrutinee_place, ); let post_guard_block = self.bind_pattern( self.source_info(pat.span), diff --git a/compiler/rustc_mir_build/src/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index d9c1d4abf0700..dc4f28864561a 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -149,9 +149,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { candidate: &mut Candidate<'pat, 'tcx>, ) -> Result<(), MatchPair<'pat, 'tcx>> { let tcx = self.tcx; - // Generate place to be used in Ascription - // Generate place to be used in Binding - let place = match_pair.place.clone().into_place(tcx, self.typeck_results); match *match_pair.pattern.kind { PatKind::AscribeUserType { ref subpattern, @@ -159,6 +156,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } => { // Apply the type ascription to the value at `match_pair.place`, which is the // value being matched, taking the variance field into account. + let place = match_pair.place.clone().into_place(self.tcx, self.typeck_results); candidate.ascriptions.push(Ascription { span: user_ty_span, user_ty, @@ -177,6 +175,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } PatKind::Binding { name, mutability, mode, var, ty, ref subpattern, is_primary: _ } => { + let place = match_pair.place.clone().into_place(self.tcx, self.typeck_results); candidate.bindings.push(Binding { name, mutability, diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index b804cd4574fa7..02f1998f49619 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -156,7 +156,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { test: &Test<'tcx>, make_target_blocks: impl FnOnce(&mut Self) -> Vec, ) { - let place = place_builder.clone().into_place(self.tcx, self.typeck_results); + let place: Place<'tcx>; + if let Ok(test_place_builder) = + place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results) + { + place = test_place_builder.clone().into_place(self.tcx, self.typeck_results); + } else { + return; + } debug!( "perform_test({:?}, {:?}: {:?}, {:?})", block, diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 25e08efb2e353..6b7bd2e9dcc8f 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -455,27 +455,32 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { ); 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) - }) - )), + Some(vals) => { + Some( + vals.iter() + .map(|(place, cause)| { + ( + self.arena.alloc( + self.convert_captured_hir_place(expr, place.clone()), + ), + *cause, + ) + // 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) + }) + .collect(), + ) + } None => None, }; @@ -1045,23 +1050,26 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { ExprKind::Deref { arg: ref_expr } } - fn fake_read_capture_upvar( + fn convert_captured_hir_place( &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 var_hir_id = match place.base { + HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, + base => bug!("Expected an upvar, found {:?}", base), + }; + let mut captured_place_expr = Expr { temp_lifetime, ty: var_ty, span: closure_expr.span, - kind: self.convert_var(hir_id), + kind: self.convert_var(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 => { @@ -1095,48 +1103,9 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { upvar_ty: Ty<'tcx>, ) -> Expr<'thir, 'tcx> { let upvar_capture = captured_place.info.capture_kind; + let captured_place_expr = + self.convert_captured_hir_place(closure_expr, captured_place.place.clone()); let temp_lifetime = self.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id); - let var_ty = captured_place.place.base_ty; - - // The result of capture analysis in `rustc_typeck/check/upvar.rs`represents a captured path - // as it's seen for use within the closure and not at the time of closure creation. - // - // That is we see expect to see it start from a captured upvar and not something that is local - // to the closure's parent. - let var_hir_id = match captured_place.place.base { - HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, - base => bug!("Expected an upvar, found {:?}", base), - }; - - let mut captured_place_expr = Expr { - temp_lifetime, - ty: var_ty, - span: closure_expr.span, - kind: self.convert_var(var_hir_id), - }; - - for proj in captured_place.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 }; - } match upvar_capture { ty::UpvarCapture::ByValue(_) => captured_place_expr, diff --git a/compiler/rustc_mir_build/src/thir/mod.rs b/compiler/rustc_mir_build/src/thir/mod.rs index 730c0f4a3df2a..a6ec3b7bdd20d 100644 --- a/compiler/rustc_mir_build/src/thir/mod.rs +++ b/compiler/rustc_mir_build/src/thir/mod.rs @@ -9,7 +9,7 @@ use rustc_hir as hir; use rustc_hir::def_id::DefId; use rustc_middle::infer::canonical::Canonical; use rustc_middle::middle::region; -use rustc_middle::mir::{BinOp, BorrowKind, Field, UnOp}; +use rustc_middle::mir::{BinOp, BorrowKind, FakeReadCause, Field, UnOp}; use rustc_middle::ty::adjustment::PointerCast; use rustc_middle::ty::subst::SubstsRef; use rustc_middle::ty::{AdtDef, Const, Ty, UpvarSubsts, UserType}; @@ -281,7 +281,7 @@ pub enum ExprKind<'thir, 'tcx> { substs: UpvarSubsts<'tcx>, upvars: &'thir [Expr<'thir, 'tcx>], movability: Option, - fake_reads: Option<&'thir mut [Expr<'thir, 'tcx>]>, + fake_reads: Option, FakeReadCause)>>, }, Literal { literal: &'tcx Const<'tcx>, diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 932f07ff1bd1b..9a43eaa3f5dc0 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -34,7 +34,6 @@ 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; @@ -42,6 +41,7 @@ use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_infer::infer::UpvarRegion; use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection, ProjectionKind}; +use rustc_middle::mir::FakeReadCause; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults, UpvarSubsts}; use rustc_session::lint; @@ -248,7 +248,8 @@ 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(); + let fake_reads = + delegate.fake_reads.into_iter().map(|(place, cause)| (place, cause)).collect(); self.typeck_results.borrow_mut().closure_fake_reads.insert(closure_def_id, fake_reads); // If we are also inferred the closure kind here, @@ -1153,7 +1154,7 @@ struct InferBorrowKind<'a, 'tcx> { /// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow } /// ``` capture_information: InferredCaptureInformation<'tcx>, - fake_reads: FxHashSet>, // these need to be fake read. + fake_reads: Vec<(Place<'tcx>, FakeReadCause)>, } impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { @@ -1415,9 +1416,9 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { } impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { - fn fake_read(&mut self, place: Place<'tcx>) { + fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause) { if let PlaceBase::Upvar(_) = place.base { - self.fake_reads.insert(place); + self.fake_reads.push((place, cause)); } } diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index 9cccda7768cc3..1be629ce9dc8f 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -12,6 +12,7 @@ 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::mir::FakeReadCause; use rustc_middle::ty::adjustment::{Adjust, Adjustment, PointerCast}; use rustc_middle::ty::fold::{TypeFoldable, TypeFolder}; use rustc_middle::ty::{self, Ty, TyCtxt}; @@ -368,18 +369,20 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { } fn visit_fake_reads_map(&mut self) { - let mut resolved_closure_fake_reads: FxHashMap>> = - Default::default(); + let mut resolved_closure_fake_reads: FxHashMap< + DefId, + Vec<(HirPlace<'tcx>, FakeReadCause)>, + > = Default::default(); for (closure_def_id, fake_reads) in self.fcx.typeck_results.borrow().closure_fake_reads.iter() { - let mut resolved_fake_reads = Vec::>::new(); - for fake_read in fake_reads.iter() { + let mut resolved_fake_reads = Vec::<(HirPlace<'tcx>, FakeReadCause)>::new(); + for (place, cause) 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); + let resolved_fake_read = self.resolve(place.clone(), &locatable); + resolved_fake_reads.push((resolved_fake_read, *cause)); } resolved_closure_fake_reads.insert(*closure_def_id, resolved_fake_reads); } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index ad39b93c067ee..6dbf8094548e8 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -8,13 +8,14 @@ pub use self::ConsumeMode::*; pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection}; use rustc_hir as hir; -use rustc_hir::def::Res; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::LocalDefId; -// use rustc_hir::Pat; use rustc_hir::PatKind; +use rustc_hir::QPath; use rustc_index::vec::Idx; use rustc_infer::infer::InferCtxt; use rustc_middle::hir::place::ProjectionKind; +use rustc_middle::mir::FakeReadCause; use rustc_middle::ty::{self, adjustment, TyCtxt}; use rustc_target::abi::VariantIdx; @@ -54,7 +55,7 @@ pub trait Delegate<'tcx> { 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: Place<'tcx>); + fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause); } #[derive(Copy, Clone, PartialEq, Debug)] @@ -237,18 +238,26 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { // 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; + let mut needs_to_be_read = 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; + needs_to_be_read = true; + } + } else if let PatKind::TupleStruct(qpath, _, _) = &pat.kind { + // If a TupleStruct has a Some PathSegment, we should read the discr_place + // regardless if it contains a Wild pattern later + if let QPath::Resolved(_, path) = qpath { + if let Res::Def(DefKind::Ctor(_, _), _) = path.res { + needs_to_be_read = true; + } } } })); } - if res { + if needs_to_be_read { self.borrow_expr(&discr, ty::ImmBorrow); } @@ -539,6 +548,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } fn walk_arm(&mut self, discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>) { + self.delegate.fake_read(discr_place.place.clone(), FakeReadCause::ForMatchedPlace); self.walk_pat(discr_place, &arm.pat); if let Some(hir::Guard::If(ref e)) = arm.guard { @@ -551,6 +561,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// Walks a pat that occurs in isolation (i.e., top-level of fn argument or /// let binding, and *not* a match arm or nested pat.) fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) { + self.delegate.fake_read(discr_place.place.clone(), FakeReadCause::ForLet); self.walk_pat(discr_place, pat); } @@ -558,8 +569,6 @@ 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.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| { @@ -620,10 +629,6 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// - When reporting the Place back to the Delegate, ensure that the UpvarId uses the enclosing /// closure as the DefId. fn walk_captures(&mut self, closure_expr: &hir::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); @@ -633,15 +638,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { ty::Closure(..) | ty::Generator(..) ); - // [FIXME] RFC2229 Closures within closures don't work if let Some(fake_reads) = self.mc.typeck_results.closure_fake_reads.get(&closure_def_id) { - for fake_read in fake_reads.iter() { - // Use this as a reference for if we should promote the fake read + for (fake_read, cause) in fake_reads.iter() { match fake_read.base { PlaceBase::Upvar(upvar_id) => { if upvars.map_or(body_owner_is_closure, |upvars| { !upvars.contains_key(&upvar_id.var_path.hir_id) }) { + // [FIXME] RFC2229 Update this comment // 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 // then for the current body (if it's a closure) these aren't captures, we will ignore them. @@ -655,14 +659,13 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { ); } }; - self.delegate.fake_read(fake_read.clone()); + self.delegate.fake_read(fake_read.clone(), *cause); } } 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 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 4863e6e2976d9..402f5e4f33e6f 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 @@ -7,19 +7,6 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -warning: irrefutable `if let` pattern - --> $DIR/closure-origin-single-variant-diagnostics.rs:18:9 - | -LL | / if let SingleVariant::Point(ref mut x, _) = point { -LL | | -LL | | *x += 1; -LL | | } - | |_________^ - | - = note: `#[warn(irrefutable_let_patterns)]` on by default - = note: this pattern will always match, so the `if let` is useless - = help: consider replacing the `if let` with a `let` - error[E0382]: use of moved value: `c` --> $DIR/closure-origin-single-variant-diagnostics.rs:21:13 | diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs index db067b6c9bca3..d91a28feae5c9 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs @@ -1,17 +1,19 @@ +//check-pass #![feature(capture_disjoint_fields)] -#![feature(rustc_attrs)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![warn(unused)] fn main() { let _z = 9; let t = (String::from("Hello"), String::from("World")); let g = (String::from("Mr"), String::from("Goose")); - let a = #[rustc_capture_analysis] || { + let a = || { let (_, g2) = g; - println!("{}", g2); - let c = #[rustc_capture_analysis] || { + //~^ WARN unused variable: `g2` + let c = || { let (_, t2) = t; - println!("{}", t2); + //~^ WARN unused variable: `t2` }; c(); diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr index c9a1a32fc579b..d88c16567b74e 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr @@ -1,23 +1,5 @@ -error[E0658]: attributes on expressions are experimental - --> $DIR/destructure-pattern-closure-within-closure.rs:9:13 - | -LL | let a = #[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/destructure-pattern-closure-within-closure.rs:12:17 - | -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 - warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/destructure-pattern-closure-within-closure.rs:1:12 + --> $DIR/destructure-pattern-closure-within-closure.rs:2:12 | LL | #![feature(capture_disjoint_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -25,86 +7,24 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -error: First Pass analysis includes: - --> $DIR/destructure-pattern-closure-within-closure.rs:12:43 - | -LL | let c = #[rustc_capture_analysis] || { - | ___________________________________________^ -LL | | let (_, t2) = t; -LL | | println!("{}", t2); -LL | | }; - | |_________^ - | -note: Capturing t[(1, 0)] -> ByValue - --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 - | -LL | let (_, t2) = t; - | ^ - -error: Min Capture analysis includes: - --> $DIR/destructure-pattern-closure-within-closure.rs:12:43 - | -LL | let c = #[rustc_capture_analysis] || { - | ___________________________________________^ -LL | | let (_, t2) = t; -LL | | println!("{}", t2); -LL | | }; - | |_________^ - | -note: Min Capture t[(1, 0)] -> ByValue - --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 +warning: unused variable: `t2` + --> $DIR/destructure-pattern-closure-within-closure.rs:15:21 | LL | let (_, t2) = t; - | ^ - -error: First Pass analysis includes: - --> $DIR/destructure-pattern-closure-within-closure.rs:9:39 + | ^^ help: if this is intentional, prefix it with an underscore: `_t2` | -LL | let a = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | let (_, g2) = g; -LL | | println!("{}", g2); -LL | | let c = #[rustc_capture_analysis] || { -... | -LL | | c(); -LL | | }; - | |_____^ - | -note: Capturing g[(1, 0)] -> ByValue - --> $DIR/destructure-pattern-closure-within-closure.rs:10:23 - | -LL | let (_, g2) = g; - | ^ -note: Capturing t[(1, 0)] -> ByValue - --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 +note: the lint level is defined here + --> $DIR/destructure-pattern-closure-within-closure.rs:4:9 | -LL | let (_, t2) = t; - | ^ +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` -error: Min Capture analysis includes: - --> $DIR/destructure-pattern-closure-within-closure.rs:9:39 - | -LL | let a = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | let (_, g2) = g; -LL | | println!("{}", g2); -LL | | let c = #[rustc_capture_analysis] || { -... | -LL | | c(); -LL | | }; - | |_____^ - | -note: Min Capture g[(1, 0)] -> ByValue - --> $DIR/destructure-pattern-closure-within-closure.rs:10:23 +warning: unused variable: `g2` + --> $DIR/destructure-pattern-closure-within-closure.rs:12:17 | LL | let (_, g2) = g; - | ^ -note: Min Capture t[(1, 0)] -> ByValue - --> $DIR/destructure-pattern-closure-within-closure.rs:13:27 - | -LL | let (_, t2) = t; - | ^ + | ^^ help: if this is intentional, prefix it with an underscore: `_g2` -error: aborting due to 6 previous errors; 1 warning emitted +warning: 3 warnings emitted -For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs deleted file mode 100644 index 757b506fd6ede..0000000000000 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.rs +++ /dev/null @@ -1,118 +0,0 @@ -//check-pass -#![feature(capture_disjoint_fields)] -//~^ WARNING: the feature `capture_disjoint_fields` is incomplete -#![warn(unused)] - -struct Point { - x: u32, - y: u32, -} - -fn test1() { - let _z = 9; - let t = (String::from("Hello"), String::from("World")); - - let c = || { - let (t1, t2) = t; - println!("{} {}", t1, t2); - }; - - c(); -} - -fn test2() { - let _z = 9; - let t = (String::from("Hello"), String::from("World")); - - let c = || { - let (t1, _) = t; - println!("{}", t1); - }; - - c(); -} - -fn test3() { - let _z = 9; - let t = (String::from("Hello"), String::from("World")); - - let c = || { - let (_, t2) = t; - println!("{}", t2); - }; - - c(); -} - -fn test4() { - let _z = 9; - let t = (String::from("Hello"), String::from("World")); - //~^ WARN unused variable: `t` - - let c = || { - let (_, _) = t; - }; - - c(); -} - -fn test5() { - let _z = 9; - let t = (String::new(), String::new()); - let _c = || { - let _a = match t { - (t1, _) => t1, - }; - }; -} - -fn test6() { - let _z = 9; - let t = (String::new(), String::new()); - let _c = || { - let _a = match t { - (_, t2) => t2, - }; - }; -} - -fn test7() { - let x = 0; - //~^ WARN unused variable: `x` - let tup = (1, 2); - //~^ WARN unused variable: `tup` - let p = Point { x: 10, y: 20 }; - - let c = || { - let _ = x; - let Point { x, y } = p; // 1 - //~^ WARN unused variable: `x` - println!("{}", y); - let (_, _) = tup; // 2 - }; - - c(); -} - -fn test8() { - let _z = 9; - let t = (String::from("Hello"), String::from("World")); - - let c = || { - let (_, t) = t; - println!("{}", t); - }; - - c(); -} - -fn main() { - test1(); - test2(); - test3(); - test4(); - test5(); - test6(); - test7(); - test8(); -} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr deleted file mode 100644 index 1ae64eb83ef78..0000000000000 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns-1.stderr +++ /dev/null @@ -1,42 +0,0 @@ -warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/destructure_patterns-1.rs:2:12 - | -LL | #![feature(capture_disjoint_fields)] - | ^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: `#[warn(incomplete_features)]` on by default - = note: see issue #53488 for more information - -warning: unused variable: `t` - --> $DIR/destructure_patterns-1.rs:49:9 - | -LL | let t = (String::from("Hello"), String::from("World")); - | ^ help: if this is intentional, prefix it with an underscore: `_t` - | -note: the lint level is defined here - --> $DIR/destructure_patterns-1.rs:4:9 - | -LL | #![warn(unused)] - | ^^^^^^ - = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` - -warning: unused variable: `x` - --> $DIR/destructure_patterns-1.rs:88:21 - | -LL | let Point { x, y } = p; // 1 - | ^ help: try ignoring the field: `x: _` - -warning: unused variable: `x` - --> $DIR/destructure_patterns-1.rs:80:9 - | -LL | let x = 0; - | ^ help: if this is intentional, prefix it with an underscore: `_x` - -warning: unused variable: `tup` - --> $DIR/destructure_patterns-1.rs:82:9 - | -LL | let tup = (1, 2); - | ^^^ help: if this is intentional, prefix it with an underscore: `_tup` - -warning: 5 warnings emitted - diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs index 6f958bb18c387..7d2c573dddd7c 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs @@ -1,52 +1,124 @@ +//check-pass #![feature(capture_disjoint_fields)] -#![feature(rustc_attrs)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![warn(unused)] -struct S { - a: String, - b: String, +struct Point { + x: u32, + y: u32, } -fn main() { +fn test1() { + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (t1, t2) = t; + //~^ WARN unused variable: `t1` + //~| WARN unused variable: `t2` + }; + + c(); +} + +fn test2() { + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (t1, _) = t; + //~^ WARN unused variable: `t1` + }; + + c(); +} + +fn test3() { + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (_, t2) = t; + //~^ WARN unused variable: `t2` + }; + + c(); +} + +fn test4() { + let t = (String::from("Hello"), String::from("World")); + //~^ WARN unused variable: `t` + + let c = || { + let (_, _) = t; + }; + + c(); +} + +fn test5() { let t = (String::new(), String::new()); + let _c = || { + let _a = match t { + (t1, _) => t1, + }; + }; +} - let s = S { - a: String::new(), - b: String::new(), +fn test6() { + let t = (String::new(), String::new()); + let _c = || { + let _a = match t { + (_, t2) => t2, + }; }; +} - let c = #[rustc_capture_analysis] || { - let (t1, t2) = t; +fn test7() { + let t = (String::new(), String::new()); + let _c = || { + let _a = match t { + (t1, t2) => (t1, t2), + }; }; +} +// [FIXME] RFC2229 Add an explanation for test +fn test8() { + let x = 0; + //~^ WARN unused variable: `x` + let tup = (1, 2); + //~^ WARN unused variable: `tup` + let p = Point { x: 10, y: 20 }; - // MIR Build - // - // Create place for the initalizer in let which is `t` - // - // I'm reading Field 1 from `t`, so apply Field projections; - // - // new place -> t[1] - // - // I'm reading Field 2 from `t`, so apply Field projections; - // - // new place -> t[2] - // - // New - // --------- - // - // I'm building something starting at `t` - // - // I read field 1 from `t` - // - // I need to use `t[1]`, therefore the place must be constructable - // - // Find the capture index for `t[1]` for this closure. - // - // I read field 2 from `t` - // - // I need to use `t[2]`, therefore the place must be constructable - // - // Find the capture index for `t[2]` for this closure. + let c = || { + let _ = x; + let Point { x, y } = p; // 1 + //~^ WARN unused variable: `x` + println!("{}", y); + let (_, _) = tup; // 2 + }; + + c(); +} + +fn test9() { + let _z = 9; + let t = (String::from("Hello"), String::from("World")); + + let c = || { + let (_, t) = t; + println!("{}", t); + }; c(); } + +fn main() { + test1(); + test2(); + test3(); + test4(); + test5(); + test6(); + test7(); + test8(); + test9(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr index 1ee5d2abf133a..b92bf089e9ed2 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr @@ -1,14 +1,5 @@ -error[E0658]: attributes on expressions are experimental - --> $DIR/destructure_patterns.rs:17: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 - warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/destructure_patterns.rs:1:12 + --> $DIR/destructure_patterns.rs:2:12 | LL | #![feature(capture_disjoint_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,46 +7,60 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -error: First Pass analysis includes: - --> $DIR/destructure_patterns.rs:17:39 +warning: unused variable: `t1` + --> $DIR/destructure_patterns.rs:15:14 | -LL | let c = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | let (t1, t2) = t; -LL | | }; - | |_____^ +LL | let (t1, t2) = t; + | ^^ help: if this is intentional, prefix it with an underscore: `_t1` | -note: Capturing t[(0, 0)] -> ByValue - --> $DIR/destructure_patterns.rs:18:24 +note: the lint level is defined here + --> $DIR/destructure_patterns.rs:4:9 | -LL | let (t1, t2) = t; - | ^ -note: Capturing t[(1, 0)] -> ByValue - --> $DIR/destructure_patterns.rs:18:24 +LL | #![warn(unused)] + | ^^^^^^ + = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` + +warning: unused variable: `t2` + --> $DIR/destructure_patterns.rs:15:18 | LL | let (t1, t2) = t; - | ^ + | ^^ help: if this is intentional, prefix it with an underscore: `_t2` -error: Min Capture analysis includes: - --> $DIR/destructure_patterns.rs:17:39 +warning: unused variable: `t1` + --> $DIR/destructure_patterns.rs:27:14 | -LL | let c = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | let (t1, t2) = t; -LL | | }; - | |_____^ +LL | let (t1, _) = t; + | ^^ help: if this is intentional, prefix it with an underscore: `_t1` + +warning: unused variable: `t2` + --> $DIR/destructure_patterns.rs:38:17 | -note: Min Capture t[(0, 0)] -> ByValue - --> $DIR/destructure_patterns.rs:18:24 +LL | let (_, t2) = t; + | ^^ help: if this is intentional, prefix it with an underscore: `_t2` + +warning: unused variable: `t` + --> $DIR/destructure_patterns.rs:46:9 | -LL | let (t1, t2) = t; - | ^ -note: Min Capture t[(1, 0)] -> ByValue - --> $DIR/destructure_patterns.rs:18:24 +LL | let t = (String::from("Hello"), String::from("World")); + | ^ help: if this is intentional, prefix it with an underscore: `_t` + +warning: unused variable: `x` + --> $DIR/destructure_patterns.rs:93:21 | -LL | let (t1, t2) = t; - | ^ +LL | let Point { x, y } = p; // 1 + | ^ help: try ignoring the field: `x: _` + +warning: unused variable: `x` + --> $DIR/destructure_patterns.rs:85:9 + | +LL | let x = 0; + | ^ help: if this is intentional, prefix it with an underscore: `_x` + +warning: unused variable: `tup` + --> $DIR/destructure_patterns.rs:87:9 + | +LL | let tup = (1, 2); + | ^^^ help: if this is intentional, prefix it with an underscore: `_tup` -error: aborting due to 3 previous errors; 1 warning emitted +warning: 9 warnings emitted -For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs index 1880351610176..6b3835634c6fe 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs @@ -1,12 +1,25 @@ +//check-pass #![feature(capture_disjoint_fields)] //~^ WARNING: the feature `capture_disjoint_fields` is incomplete -#![feature(rustc_attrs)] -fn main() { +fn test1() { let foo = [1, 2, 3]; - let c = #[rustc_capture_analysis] || { - //~^ ERROR: attributes on expressions are experimental - //~| ERROR: First Pass analysis includes: + let c = || { match foo { _ => () }; }; } + +fn test2() { + let foo = Some([1, 2, 3]); + let c = || { + match foo { + Some(_) => 1, + _ => 2 + }; + }; +} + +fn main() { + test1(); + test2(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr index e2e825fe9425e..6f5b28aa6cf67 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr @@ -1,14 +1,5 @@ -error[E0658]: attributes on expressions are experimental - --> $DIR/no_capture_with_wildcard_match.rs:7: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 - warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/no_capture_with_wildcard_match.rs:1:12 + --> $DIR/no_capture_with_wildcard_match.rs:2:12 | LL | #![feature(capture_disjoint_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,17 +7,5 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -error: First Pass analysis includes: - --> $DIR/no_capture_with_wildcard_match.rs:7:39 - | -LL | let c = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | -LL | | -LL | | match foo { _ => () }; -LL | | }; - | |_____^ - -error: aborting due to 2 previous errors; 1 warning emitted +warning: 1 warning emitted -For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs index 9757860fb4c7f..34710e69b9659 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs @@ -1,4 +1,6 @@ +//check-pass #![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete #![feature(rustc_attrs)] struct S { @@ -12,7 +14,7 @@ fn main() { b: String::new(), }; - let c = #[rustc_capture_analysis] || { + let c = || { let s2 = S { a: format!("New a"), ..s diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr index 15d8040c46c51..66171f35763d8 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr @@ -1,14 +1,5 @@ -error[E0658]: attributes on expressions are experimental - --> $DIR/struct_update_syntax.rs:15: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 - warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/struct_update_syntax.rs:1:12 + --> $DIR/struct_update_syntax.rs:2:12 | LL | #![feature(capture_disjoint_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,42 +7,5 @@ LL | #![feature(capture_disjoint_fields)] = note: `#[warn(incomplete_features)]` on by default = note: see issue #53488 for more information -error: First Pass analysis includes: - --> $DIR/struct_update_syntax.rs:15:39 - | -LL | let c = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | let s2 = S { -LL | | a: format!("New a"), -LL | | ..s -LL | | }; -LL | | }; - | |_____^ - | -note: Capturing s[(1, 0)] -> ByValue - --> $DIR/struct_update_syntax.rs:18:15 - | -LL | ..s - | ^ - -error: Min Capture analysis includes: - --> $DIR/struct_update_syntax.rs:15:39 - | -LL | let c = #[rustc_capture_analysis] || { - | _______________________________________^ -LL | | let s2 = S { -LL | | a: format!("New a"), -LL | | ..s -LL | | }; -LL | | }; - | |_____^ - | -note: Min Capture s[(1, 0)] -> ByValue - --> $DIR/struct_update_syntax.rs:18:15 - | -LL | ..s - | ^ - -error: aborting due to 3 previous errors; 1 warning emitted +warning: 1 warning emitted -For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr b/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr index 285c203f382df..a3469ee211425 100644 --- a/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr +++ b/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr @@ -189,18 +189,19 @@ LL | drop(_x3); error[E0382]: use of moved value: `tup` --> $DIR/borrowck-move-ref-pattern.rs:43:14 | -LL | let mut tup = (U, U, U); - | ------- move occurs because `tup` has type `(U, U, U)`, which does not implement the `Copy` trait -LL | let c1 = || { - | -- value moved into closure here -LL | let (ref _x0, _x1, _) = tup; - | --- variable moved due to use in closure -LL | }; -LL | let c2 = || { - | ^^ value used here after move -LL | -LL | let (ref mut _x0, _, _x2) = tup; - | --- use occurs due to use in closure +LL | let mut tup = (U, U, U); + | ------- move occurs because `tup` has type `(U, U, U)`, which does not implement the `Copy` trait +LL | let c1 = || { + | -- value moved into closure here +LL | let (ref _x0, _x1, _) = tup; + | --- variable moved due to use in closure +LL | }; +LL | let c2 = || { + | ______________^ +LL | | +LL | | let (ref mut _x0, _, _x2) = tup; +LL | | }; + | |_____^ value used here after move error: aborting due to 18 previous errors From 74fc64303fe4c6fcbbd7dae987b6d27c65720fa9 Mon Sep 17 00:00:00 2001 From: Roxane Date: Tue, 23 Feb 2021 23:39:33 -0500 Subject: [PATCH 5/8] Only borrow place for matching under specific conditions --- .../src/build/expr/as_place.rs | 2 +- .../rustc_mir_build/src/build/expr/into.rs | 2 + .../src/build/matches/simplify.rs | 9 ++-- compiler/rustc_mir_build/src/thir/cx/expr.rs | 43 +++++++---------- compiler/rustc_typeck/src/expr_use_visitor.rs | 38 ++++++++------- .../lit-pattern-matching-with-methods.rs | 24 ++++++++++ .../lit-pattern-matching-with-methods.stderr | 11 +++++ .../struct-pattern-matching-with-methods.rs | 48 +++++++++++++++++++ ...truct-pattern-matching-with-methods.stderr | 11 +++++ ...le-struct-pattern-matching-with-methods.rs | 43 +++++++++++++++++ ...truct-pattern-matching-with-methods.stderr | 11 +++++ 11 files changed, 193 insertions(+), 49 deletions(-) create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.stderr 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 6519d5fae1c7f..0e70cd36efba8 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -100,7 +100,7 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>( // single and multiple variants. // For single variants, enums are not captured completely. // We keep track of VariantIdx so we can use this information - // if the next ProjectionElem is a Field + // if the next ProjectionElem is a Field. variant = Some(*idx); continue; } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index b2e8b2de1bc6a..47f75825fb6af 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -420,6 +420,7 @@ 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); @@ -436,6 +437,7 @@ 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/build/matches/simplify.rs b/compiler/rustc_mir_build/src/build/matches/simplify.rs index dc4f28864561a..3ad143a57ff56 100644 --- a/compiler/rustc_mir_build/src/build/matches/simplify.rs +++ b/compiler/rustc_mir_build/src/build/matches/simplify.rs @@ -68,7 +68,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let match_pairs = mem::take(&mut candidate.match_pairs); if let [MatchPair { pattern: Pat { kind: box PatKind::Or { pats }, .. }, place }] = - &*match_pairs.clone() + &*match_pairs { existing_bindings.extend_from_slice(&new_bindings); mem::swap(&mut candidate.bindings, &mut existing_bindings); @@ -155,12 +155,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ascription: thir::pattern::Ascription { variance, user_ty, user_ty_span }, } => { // Apply the type ascription to the value at `match_pair.place`, which is the - // value being matched, taking the variance field into account. - let place = match_pair.place.clone().into_place(self.tcx, self.typeck_results); candidate.ascriptions.push(Ascription { span: user_ty_span, user_ty, - source: place, + source: match_pair.place.clone().into_place(self.tcx, self.typeck_results), variance, }); @@ -175,12 +173,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } PatKind::Binding { name, mutability, mode, var, ty, ref subpattern, is_primary: _ } => { - let place = match_pair.place.clone().into_place(self.tcx, self.typeck_results); candidate.bindings.push(Binding { name, mutability, span: match_pair.pattern.span, - source: place, + source: match_pair.place.clone().into_place(self.tcx, self.typeck_results), var_id: var, var_ty: ty, binding_mode: mode, diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 6b7bd2e9dcc8f..dee69793f2a80 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -455,32 +455,18 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { ); let fake_reads = match self.typeck_results().closure_fake_reads.get(&def_id) { - Some(vals) => { - Some( - vals.iter() - .map(|(place, cause)| { - ( - self.arena.alloc( - self.convert_captured_hir_place(expr, place.clone()), - ), - *cause, - ) - // 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) - }) - .collect(), - ) - } + Some(vals) => Some( + vals.iter() + .map(|(place, cause)| { + ( + self.arena.alloc( + self.convert_captured_hir_place(expr, place.clone()), + ), + *cause, + ) + }) + .collect(), + ), None => None, }; @@ -1058,6 +1044,11 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { let temp_lifetime = self.region_scope_tree.temporary_scope(closure_expr.hir_id.local_id); let var_ty = place.base_ty; + // The result of capture analysis in `rustc_typeck/check/upvar.rs`represents a captured path + // as it's seen for use within the closure and not at the time of closure creation. + // + // That is we see expect to see it start from a captured upvar and not something that is local + // to the closure's parent. let var_hir_id = match place.base { HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, base => bug!("Expected an upvar, found {:?}", base), diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 6dbf8094548e8..0172f993c266c 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -8,10 +8,10 @@ pub use self::ConsumeMode::*; pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection}; use rustc_hir as hir; -use rustc_hir::def::{DefKind, Res}; +use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; use rustc_hir::PatKind; -use rustc_hir::QPath; +//use rustc_hir::QPath; use rustc_index::vec::Idx; use rustc_infer::infer::InferCtxt; use rustc_middle::hir::place::ProjectionKind; @@ -54,7 +54,6 @@ pub trait Delegate<'tcx> { // `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: Place<'tcx>, cause: FakeReadCause); } @@ -235,24 +234,30 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { hir::ExprKind::Match(ref discr, arms, _) => { let discr_place = return_if_err!(self.mc.cat_expr(&discr)); + // Matching should not always be considered a use of the place, hence + // discr does not necessarily need to be borrowed. // We only want to borrow discr if the pattern contain something other - // than wildcards + // than wildcards. let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self; let mut needs_to_be_read = 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 { - needs_to_be_read = true; - } - } else if let PatKind::TupleStruct(qpath, _, _) = &pat.kind { - // If a TupleStruct has a Some PathSegment, we should read the discr_place - // regardless if it contains a Wild pattern later - if let QPath::Resolved(_, path) = qpath { - if let Res::Def(DefKind::Ctor(_, _), _) = path.res { + match &pat.kind { + PatKind::Binding(_, _, _, opt_sub_pat) => { + // If the opt_sub_pat is None, than the binding does not count as + // a wildcard for the purpose of borrowing discr + if let None = opt_sub_pat { needs_to_be_read = true; } } + PatKind::TupleStruct(_, _, _) + | PatKind::Struct(_, _, _) + | PatKind::Lit(_) => { + // If the PatKind is a TupleStruct, Struct, or Lit then we want + // to borrow discr + needs_to_be_read = true; + } + _ => {} } })); } @@ -629,6 +634,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// - When reporting the Place back to the Delegate, ensure that the UpvarId uses the enclosing /// closure as the DefId. fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) { + debug!("walk_captures({:?})", closure_expr); + 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); @@ -645,10 +652,9 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { if upvars.map_or(body_owner_is_closure, |upvars| { !upvars.contains_key(&upvar_id.var_path.hir_id) }) { - // [FIXME] RFC2229 Update this comment - // The nested closure might be capturing the current (enclosing) closure's local variables. + // The nested closure might be fake reading the current (enclosing) closure's local variables. // We check if the root variable is ever mentioned within the enclosing closure, if not - // then for the current body (if it's a closure) these aren't captures, we will ignore them. + // then for the current body (if it's a closure) these do not require fake_read, we will ignore them. continue; } } diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs new file mode 100644 index 0000000000000..97b56e7c4be22 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs @@ -0,0 +1,24 @@ +//check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![warn(unused)] +#![feature(rustc_attrs)] +#![feature(btree_drain_filter)] + +use std::collections::BTreeMap; +use std::panic::{catch_unwind, AssertUnwindSafe}; + +fn main() { + let mut map = BTreeMap::new(); + map.insert("a", ()); + map.insert("b", ()); + map.insert("c", ()); + + { + let mut it = map.drain_filter(|_, _| true); + catch_unwind(AssertUnwindSafe(|| while it.next().is_some() {})).unwrap_err(); + let result = catch_unwind(AssertUnwindSafe(|| it.next())); + assert!(matches!(result, Ok(None))); + } + +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.stderr new file mode 100644 index 0000000000000..bc046ecad6867 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/lit-pattern-matching-with-methods.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs new file mode 100644 index 0000000000000..2f13590a2bdab --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs @@ -0,0 +1,48 @@ +//check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![warn(unused)] +#![feature(rustc_attrs)] + +#[derive(Debug, Clone, Copy)] +enum PointType { + TwoD { x: u32, y: u32 }, + + ThreeD{ x: u32, y: u32, z: u32 } +} + +struct Points { + points: Vec, +} + +impl Points { + pub fn test1(&mut self) -> Vec { + (0..self.points.len()) + .filter_map(|i| { + let idx = i as usize; + match self.test2(idx) { + PointType::TwoD { .. } => Some(i), + PointType::ThreeD { .. } => None, + } + }) + .collect() + } + + pub fn test2(&mut self, i: usize) -> PointType { + self.points[i] + } +} + +fn main() { + let mut points = Points { + points: Vec::::new() + }; + + points.points.push(PointType::ThreeD { x:0, y:0, z:0 }); + points.points.push(PointType::TwoD{ x:0, y:0 }); + points.points.push(PointType::ThreeD{ x:0, y:0, z:0 }); + points.points.push(PointType::TwoD{ x:0, y:0 }); + + println!("{:?}", points.test1()); + println!("{:?}", points.points); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.stderr new file mode 100644 index 0000000000000..3e4303a3710df --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/struct-pattern-matching-with-methods.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs new file mode 100644 index 0000000000000..0948039287b2c --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs @@ -0,0 +1,43 @@ +//check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete + +#[derive(Copy, Clone)] +enum PointType { + TwoD(u32, u32), + ThreeD(u32, u32, u32) +} + +struct Points { + points: Vec, +} + +impl Points { + pub fn test1(&mut self) -> Vec { + (0..self.points.len()) + .filter_map(|i| { + match self.test2(i) { + PointType::TwoD (..) => Some(i), + PointType::ThreeD (..) => None, + } + }) + .collect() + } + + pub fn test2(&mut self, i: usize) -> PointType { + self.points[i] + } +} + +fn main() { + let mut points = Points { + points: Vec::::new() + }; + + points.points.push(PointType::ThreeD(0,0,0)); + points.points.push(PointType::TwoD(0,0)); + points.points.push(PointType::ThreeD(0,0,1)); + points.points.push(PointType::TwoD(0,1)); + + println!("{:?}", points.test1()); +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.stderr new file mode 100644 index 0000000000000..ded0e37b0f366 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/tuple-struct-pattern-matching-with-methods.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + From fb3b77a8c858b7bf673472a99737cd18f5ddbe17 Mon Sep 17 00:00:00 2001 From: Roxane Date: Thu, 25 Feb 2021 15:33:18 -0500 Subject: [PATCH 6/8] Add fake_read() to clippy --- src/tools/clippy/clippy_lints/src/escape.rs | 3 +++ src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs | 3 +++ src/tools/clippy/clippy_utils/src/usage.rs | 3 +++ 3 files changed, 9 insertions(+) diff --git a/src/tools/clippy/clippy_lints/src/escape.rs b/src/tools/clippy/clippy_lints/src/escape.rs index f8ef2a464d5c3..6994e9f7d2e69 100644 --- a/src/tools/clippy/clippy_lints/src/escape.rs +++ b/src/tools/clippy/clippy_lints/src/escape.rs @@ -2,6 +2,7 @@ use rustc_hir::intravisit; use rustc_hir::{self, AssocItemKind, Body, FnDecl, HirId, HirIdSet, Impl, ItemKind, Node}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::FakeReadCause; use rustc_middle::ty::{self, TraitRef, Ty}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -184,6 +185,8 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } } + + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause) { } } impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> { diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index cac4b2075114a..d5a9d5e4e13b7 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -11,6 +11,7 @@ use rustc_hir::intravisit::FnKind; use rustc_hir::{BindingAnnotation, Body, FnDecl, GenericArg, HirId, Impl, ItemKind, Node, PatKind, QPath, TyKind}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::mir::FakeReadCause; use rustc_middle::ty::{self, TypeFoldable}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::symbol::kw; @@ -333,4 +334,6 @@ impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { fn borrow(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {} fn mutate(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId) {} + + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause) { } } diff --git a/src/tools/clippy/clippy_utils/src/usage.rs b/src/tools/clippy/clippy_utils/src/usage.rs index d577827dcf3cc..8aa8781f6be38 100644 --- a/src/tools/clippy/clippy_utils/src/usage.rs +++ b/src/tools/clippy/clippy_utils/src/usage.rs @@ -7,6 +7,7 @@ use rustc_hir::intravisit::{NestedVisitorMap, Visitor}; use rustc_hir::{Expr, ExprKind, HirId, Path}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; +use rustc_middle::mir::FakeReadCause; use rustc_middle::hir::map::Map; use rustc_middle::ty; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; @@ -77,6 +78,8 @@ impl<'tcx> Delegate<'tcx> for MutVarsDelegate { fn mutate(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) { self.update(&cmt) } + + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause) { } } pub struct ParamBindingIdCollector { From 22eaffe71ae1665a0596d67acca7748276534dc9 Mon Sep 17 00:00:00 2001 From: Roxane Date: Thu, 25 Feb 2021 18:03:41 -0500 Subject: [PATCH 7/8] Add comments with examples and tests --- compiler/rustc_middle/src/ty/context.rs | 24 +- .../src/build/expr/as_place.rs | 28 ++- .../src/build/expr/as_rvalue.rs | 58 +++-- .../rustc_mir_build/src/build/matches/mod.rs | 31 ++- .../rustc_mir_build/src/build/matches/test.rs | 2 +- compiler/rustc_mir_build/src/thir/cx/expr.rs | 26 +-- compiler/rustc_mir_build/src/thir/mod.rs | 2 +- compiler/rustc_typeck/src/check/upvar.rs | 13 +- compiler/rustc_typeck/src/check/writeback.rs | 8 +- compiler/rustc_typeck/src/expr_use_visitor.rs | 96 ++++++-- .../pattern-matching-should-fail.rs | 84 +++++++ .../pattern-matching-should-fail.stderr | 72 ++++++ .../patterns-capture-analysis.rs | 139 ++++++++++++ .../patterns-capture-analysis.stderr | 212 ++++++++++++++++++ ...atch.rs => capture_with_wildcard_match.rs} | 7 +- ...err => capture_with_wildcard_match.stderr} | 2 +- ...tructure-pattern-closure-within-closure.rs | 1 - ...ture-pattern-closure-within-closure.stderr | 4 +- .../run_pass/destructure_patterns.rs | 5 +- .../run_pass/destructure_patterns.stderr | 8 +- .../run_pass/drop_then_use_fake_reads.rs | 12 + ...stderr => drop_then_use_fake_reads.stderr} | 2 +- .../lit-pattern-matching-with-methods.rs | 7 + .../struct-pattern-matching-with-methods.rs | 1 + .../run_pass/struct_update_syntax.rs | 25 --- ...le-struct-pattern-matching-with-methods.rs | 1 + .../use_of_mutable_borrow_and_fake_reads.rs | 12 + ...se_of_mutable_borrow_and_fake_reads.stderr | 11 + .../borrowck-move-ref-pattern.stderr | 25 +-- src/tools/clippy/clippy_lints/src/escape.rs | 2 +- .../src/needless_pass_by_value.rs | 2 +- src/tools/clippy/clippy_utils/src/usage.rs | 2 +- 32 files changed, 788 insertions(+), 136 deletions(-) create mode 100644 src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.stderr create mode 100644 src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.stderr rename src/test/ui/closures/2229_closure_analysis/run_pass/{no_capture_with_wildcard_match.rs => capture_with_wildcard_match.rs} (61%) rename src/test/ui/closures/2229_closure_analysis/run_pass/{no_capture_with_wildcard_match.stderr => capture_with_wildcard_match.stderr} (88%) create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.rs rename src/test/ui/closures/2229_closure_analysis/run_pass/{struct_update_syntax.stderr => drop_then_use_fake_reads.stderr} (89%) delete mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.rs create mode 100644 src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.stderr diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index 425e62d6f61a9..00c03d7538030 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -431,7 +431,29 @@ pub struct TypeckResults<'tcx> { /// see `MinCaptureInformationMap` for more details. pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>, - pub closure_fake_reads: FxHashMap, FakeReadCause)>>, + /// Tracks the fake reads required for a closure and the reason for the fake read. + /// When performing pattern matching for closures, there are times we don't end up + /// reading places that are mentioned in a closure (because of _ patterns). However, + /// to ensure the places are initialized, we introduce fake reads. + /// Consider these two examples: + /// ``` (discriminant matching with only wildcard arm) + /// let x: u8; + /// let c = || match x { _ => () }; + /// ``` + /// In this example, we don't need to actually read/borrow `x` in `c`, and so we don't + /// want to capture it. However, we do still want an error here, because `x` should have + /// to be initialized at the point where c is created. Therefore, we add a "fake read" + /// instead. + /// ``` (destructured assignments) + /// let c = || { + /// let (t1, t2) = t; + /// } + /// ``` + /// In the second example, we capture the disjoint fields of `t` (`t.0` & `t.1`), but + /// we never capture `t`. This becomes an issue when we build MIR as we require + /// information on `t` in order to create place `t.0` and `t.1`. We can solve this + /// issue by fake reading `t`. + pub closure_fake_reads: FxHashMap, FakeReadCause, hir::HirId)>>, /// Stores the type, expression, span and optional scope span of all types /// that are live across the yield of this generator (if a generator). 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 0e70cd36efba8..fbc9c30fe539c 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -90,15 +90,13 @@ fn convert_to_hir_projections_and_truncate_for_capture<'tcx>( let hir_projection = match mir_projection { ProjectionElem::Deref => HirProjectionKind::Deref, ProjectionElem::Field(field, _) => { - // We will never encouter this for multivariant enums, - // read the comment for `Downcast`. let variant = variant.unwrap_or(VariantIdx::new(0)); HirProjectionKind::Field(field.index() as u32, variant) } ProjectionElem::Downcast(.., idx) => { - // This projections exist for enums that have - // single and multiple variants. - // For single variants, enums are not captured completely. + // We don't expect to see multi-variant enums here, as earlier + // phases will have truncated them already. However, there can + // still be downcasts, thanks to single-variant enums. // We keep track of VariantIdx so we can use this information // if the next ProjectionElem is a Field. variant = Some(*idx); @@ -200,7 +198,7 @@ fn find_capture_matching_projections<'a, 'tcx>( /// Takes a PlaceBuilder and resolves the upvar (if any) within it, so that the /// `PlaceBuilder` now starts from `PlaceBase::Local`. /// -/// Returns a Result with the error being the HirId of the Upvar that was not found. +/// Returns a Result with the error being the PlaceBuilder (`from_builder`) that was not found. fn to_upvars_resolved_place_builder<'a, 'tcx>( from_builder: PlaceBuilder<'tcx>, tcx: TyCtxt<'tcx>, @@ -305,15 +303,23 @@ impl<'tcx> PlaceBuilder<'tcx> { to_upvars_resolved_place_builder(self, tcx, typeck_results).unwrap() } + /// Attempts to resolve the `PlaceBuilder`. + /// On success, it will return the resolved `PlaceBuilder`. + /// On failure, it will return itself. + /// + /// Upvars resolve may fail for a `PlaceBuilder` when attempting to + /// resolve a disjoint field whose root variable is not captured + /// (destructured assignments) or when attempting to resolve a root + /// variable (discriminant matching with only wildcard arm) that is + /// not captured. This can happen because the final mir that will be + /// generated doesn't require a read for this place. Failures will only + /// happen inside closures. crate fn try_upvars_resolved<'a>( self, tcx: TyCtxt<'tcx>, typeck_results: &'a ty::TypeckResults<'tcx>, ) -> Result, PlaceBuilder<'tcx>> { - match to_upvars_resolved_place_builder(self, tcx, typeck_results) { - Ok(upvars_resolved) => Ok(upvars_resolved), - Err(upvars_unresolved) => Err(upvars_unresolved), - } + to_upvars_resolved_place_builder(self, tcx, typeck_results) } crate fn base(&self) -> PlaceBase { @@ -662,7 +668,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block, source_info, len, - Rvalue::Len(slice.clone().into_place(self.tcx, self.typeck_results)), + Rvalue::Len(slice.into_place(self.tcx, self.typeck_results)), ); // lt = idx < len self.cfg.push_assign( 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 c7f16fd4e4049..53c87d71e138b 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -165,13 +165,42 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields)) } - ExprKind::Closure { - closure_id, - substs, - upvars, - movability, - fake_reads: opt_fake_reads, - } => { + ExprKind::Closure { closure_id, substs, upvars, movability, fake_reads } => { + // Convert the closure fake reads, if any, from `ExprRef` to mir `Place` + // and push the fake reads. + // This must come before creating the operands. This is required in case + // there is a fake read and a borrow of the same path, since otherwise the + // fake read might interfere with the borrow. Consider an example like this + // one: + // ``` + // let mut x = 0; + // let c = || { + // &mut x; // mutable borrow of `x` + // match x { _ => () } // fake read of `x` + // }; + // ``` + + // FIXME(RFC2229): Remove feature gate once diagnostics are improved + if this.tcx.features().capture_disjoint_fields { + for (thir_place, cause, hir_id) in fake_reads.into_iter() { + let place_builder = + unpack!(block = this.as_place_builder(block, thir_place)); + + if let Ok(place_builder_resolved) = + place_builder.try_upvars_resolved(this.tcx, this.typeck_results) + { + let mir_place = + place_builder_resolved.into_place(this.tcx, this.typeck_results); + this.cfg.push_fake_read( + block, + this.source_info(this.tcx.hir().span(hir_id)), + cause, + mir_place, + ); + } + } + } + // see (*) above let operands: Vec<_> = upvars .into_iter() @@ -210,21 +239,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } }) .collect(); - if let Some(fake_reads) = opt_fake_reads { - for (thir_place, cause) in fake_reads.into_iter() { - let place_builder = - unpack!(block = this.as_place_builder(block, thir_place)); - - if let Ok(place_builder_resolved) = - place_builder.clone().try_upvars_resolved(this.tcx, this.typeck_results) - { - let mir_place = place_builder_resolved - .clone() - .into_place(this.tcx, this.typeck_results); - this.cfg.push_fake_read(block, source_info, cause, mir_place); - } - } - } let result = match substs { UpvarSubsts::Generator(substs) => { diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 59b3fd8647897..6e8b25c91628b 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -146,8 +146,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let Ok(scrutinee_builder) = scrutinee_place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results) { - let scrutinee_place = - scrutinee_builder.clone().into_place(self.tcx, self.typeck_results); + let scrutinee_place = scrutinee_builder.into_place(self.tcx, self.typeck_results); self.cfg.push_fake_read(block, source_info, cause_matched_place, scrutinee_place); } @@ -245,7 +244,19 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let arm_source_info = self.source_info(arm.span); let arm_scope = (arm.scope, arm_source_info); self.in_scope(arm_scope, arm.lint_level, |this| { - let body = arm.body.clone(); + let body = arm.body; + + // `try_upvars_resolved` may fail if it is unable to resolve the given + // `PlaceBuilder` inside a closure. In this case, we don't want to include + // a scrutinee place. `scrutinee_place_builder` will fail to be resolved + // if the only match arm is a wildcard (`_`). + // Example: + // ``` + // let foo = (0, 1); + // let c = || { + // match foo { _ => () }; + // }; + // ``` let mut opt_scrutinee_place: Option<(Option<&Place<'tcx>>, Span)> = None; let scrutinee_place: Place<'tcx>; if let Ok(scrutinee_builder) = scrutinee_place_builder @@ -496,6 +507,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { VarBindingForm { opt_match_place: Some((ref mut match_place, _)), .. }, )))) = self.local_decls[local].local_info { + // `try_upvars_resolved` may fail if it is unable to resolve the given + // `PlaceBuilder` inside a closure. In this case, we don't want to include + // a scrutinee place. `scrutinee_place_builder` will fail for destructured + // assignments. This is because a closure only captures the precise places + // that it will read and as a result a closure may not capture the entire + // tuple/struct and rather have individual places that will be read in the + // final MIR. + // Example: + // ``` + // let foo = (0, 1); + // let c = || { + // let (v1, v2) = foo; + // }; + // ``` if let Ok(match_pair_resolved) = initializer.clone().try_upvars_resolved(self.tcx, self.typeck_results) { diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index 02f1998f49619..b13a0ef820157 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -160,7 +160,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let Ok(test_place_builder) = place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results) { - place = test_place_builder.clone().into_place(self.tcx, self.typeck_results); + place = test_place_builder.into_place(self.tcx, self.typeck_results); } else { return; } diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index dee69793f2a80..2f58f2975b115 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -454,20 +454,20 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { .map(|(captured_place, ty)| self.capture_upvar(expr, captured_place, ty)), ); + // Convert the closure fake reads, if any, from hir `Place` to ExprRef let fake_reads = match self.typeck_results().closure_fake_reads.get(&def_id) { - Some(vals) => Some( - vals.iter() - .map(|(place, cause)| { - ( - self.arena.alloc( - self.convert_captured_hir_place(expr, place.clone()), - ), - *cause, - ) - }) - .collect(), - ), - None => None, + Some(fake_reads) => fake_reads + .iter() + .map(|(place, cause, hir_id)| { + ( + self.arena + .alloc(self.convert_captured_hir_place(expr, place.clone())), + *cause, + *hir_id, + ) + }) + .collect(), + None => Vec::new(), }; ExprKind::Closure { diff --git a/compiler/rustc_mir_build/src/thir/mod.rs b/compiler/rustc_mir_build/src/thir/mod.rs index a6ec3b7bdd20d..71d3093854d6c 100644 --- a/compiler/rustc_mir_build/src/thir/mod.rs +++ b/compiler/rustc_mir_build/src/thir/mod.rs @@ -281,7 +281,7 @@ pub enum ExprKind<'thir, 'tcx> { substs: UpvarSubsts<'tcx>, upvars: &'thir [Expr<'thir, 'tcx>], movability: Option, - fake_reads: Option, FakeReadCause)>>, + fake_reads: Vec<(&'thir mut Expr<'thir, 'tcx>, FakeReadCause, hir::HirId)>, }, Literal { literal: &'tcx Const<'tcx>, diff --git a/compiler/rustc_typeck/src/check/upvar.rs b/compiler/rustc_typeck/src/check/upvar.rs index 9a43eaa3f5dc0..e79349796d226 100644 --- a/compiler/rustc_typeck/src/check/upvar.rs +++ b/compiler/rustc_typeck/src/check/upvar.rs @@ -248,8 +248,11 @@ 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(|(place, cause)| (place, cause)).collect(); + let fake_reads = delegate + .fake_reads + .into_iter() + .map(|(place, cause, hir_id)| (place, cause, hir_id)) + .collect(); self.typeck_results.borrow_mut().closure_fake_reads.insert(closure_def_id, fake_reads); // If we are also inferred the closure kind here, @@ -1154,7 +1157,7 @@ struct InferBorrowKind<'a, 'tcx> { /// Place { V1, [ProjectionKind::Field(Index=1, Variant=0)] } : CaptureKind { E2, MutableBorrow } /// ``` capture_information: InferredCaptureInformation<'tcx>, - fake_reads: Vec<(Place<'tcx>, FakeReadCause)>, + fake_reads: Vec<(Place<'tcx>, FakeReadCause, hir::HirId)>, } impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { @@ -1416,9 +1419,9 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> { } impl<'a, 'tcx> euv::Delegate<'tcx> for InferBorrowKind<'a, 'tcx> { - fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause) { + fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId) { if let PlaceBase::Upvar(_) = place.base { - self.fake_reads.push((place, cause)); + self.fake_reads.push((place, cause, diag_expr_id)); } } diff --git a/compiler/rustc_typeck/src/check/writeback.rs b/compiler/rustc_typeck/src/check/writeback.rs index 1be629ce9dc8f..b88a96de6987e 100644 --- a/compiler/rustc_typeck/src/check/writeback.rs +++ b/compiler/rustc_typeck/src/check/writeback.rs @@ -371,18 +371,18 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { fn visit_fake_reads_map(&mut self) { let mut resolved_closure_fake_reads: FxHashMap< DefId, - Vec<(HirPlace<'tcx>, FakeReadCause)>, + Vec<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>, > = 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>, FakeReadCause)>::new(); - for (place, cause) in fake_reads.iter() { + let mut resolved_fake_reads = Vec::<(HirPlace<'tcx>, FakeReadCause, hir::HirId)>::new(); + for (place, cause, hir_id) 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(place.clone(), &locatable); - resolved_fake_reads.push((resolved_fake_read, *cause)); + resolved_fake_reads.push((resolved_fake_read, *cause, *hir_id)); } resolved_closure_fake_reads.insert(*closure_def_id, resolved_fake_reads); } diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 0172f993c266c..dced4aac049ad 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -7,11 +7,11 @@ pub use self::ConsumeMode::*; // Export these here so that Clippy can use them. pub use rustc_middle::hir::place::{Place, PlaceBase, PlaceWithHirId, Projection}; +use rustc_data_structures::fx::FxIndexMap; use rustc_hir as hir; use rustc_hir::def::Res; use rustc_hir::def_id::LocalDefId; use rustc_hir::PatKind; -//use rustc_hir::QPath; use rustc_index::vec::Idx; use rustc_infer::infer::InferCtxt; use rustc_middle::hir::place::ProjectionKind; @@ -54,7 +54,8 @@ pub trait Delegate<'tcx> { // `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); - fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause); + // The `place` should be a fake read because of specified `cause`. + fn fake_read(&mut self, place: Place<'tcx>, cause: FakeReadCause, diag_expr_id: hir::HirId); } #[derive(Copy, Clone, PartialEq, Debug)] @@ -241,20 +242,33 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { let ExprUseVisitor { ref mc, body_owner: _, delegate: _ } = *self; let mut needs_to_be_read = false; for arm in arms.iter() { - return_if_err!(mc.cat_pattern(discr_place.clone(), &arm.pat, |_place, pat| { + return_if_err!(mc.cat_pattern(discr_place.clone(), &arm.pat, |place, pat| { match &pat.kind { - PatKind::Binding(_, _, _, opt_sub_pat) => { + PatKind::Binding(.., opt_sub_pat) => { // If the opt_sub_pat is None, than the binding does not count as - // a wildcard for the purpose of borrowing discr - if let None = opt_sub_pat { + // a wildcard for the purpose of borrowing discr. + if opt_sub_pat.is_none() { needs_to_be_read = true; } } - PatKind::TupleStruct(_, _, _) - | PatKind::Struct(_, _, _) - | PatKind::Lit(_) => { - // If the PatKind is a TupleStruct, Struct, or Lit then we want - // to borrow discr + PatKind::TupleStruct(..) + | PatKind::Path(..) + | PatKind::Struct(..) + | PatKind::Tuple(..) => { + // If the PatKind is a TupleStruct, Struct or Tuple then we want to check + // whether the Variant is a MultiVariant or a SingleVariant. We only want + // to borrow discr if it is a MultiVariant. + // If it is a SingleVariant and creates a binding we will handle that when + // this callback gets called again. + if let ty::Adt(def, _) = place.place.base_ty.kind() { + if def.variants.len() > 1 { + needs_to_be_read = true; + } + } + } + PatKind::Lit(_) => { + // If the PatKind is a Lit then we want + // to borrow discr. needs_to_be_read = true; } _ => {} @@ -264,6 +278,16 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { if needs_to_be_read { self.borrow_expr(&discr, ty::ImmBorrow); + } else { + self.delegate.fake_read( + discr_place.place.clone(), + FakeReadCause::ForMatchedPlace, + discr_place.hir_id, + ); + + // We always want to walk the discriminant. We want to make sure, for instance, + // that the discriminant has been initialized. + self.walk_expr(&discr); } // treatment of the discriminant is handled while walking the arms. @@ -553,7 +577,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } fn walk_arm(&mut self, discr_place: &PlaceWithHirId<'tcx>, arm: &hir::Arm<'_>) { - self.delegate.fake_read(discr_place.place.clone(), FakeReadCause::ForMatchedPlace); + self.delegate.fake_read( + discr_place.place.clone(), + FakeReadCause::ForMatchedPlace, + discr_place.hir_id, + ); self.walk_pat(discr_place, &arm.pat); if let Some(hir::Guard::If(ref e)) = arm.guard { @@ -566,7 +594,11 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// Walks a pat that occurs in isolation (i.e., top-level of fn argument or /// let binding, and *not* a match arm or nested pat.) fn walk_irrefutable_pat(&mut self, discr_place: &PlaceWithHirId<'tcx>, pat: &hir::Pat<'_>) { - self.delegate.fake_read(discr_place.place.clone(), FakeReadCause::ForLet); + self.delegate.fake_read( + discr_place.place.clone(), + FakeReadCause::ForLet, + discr_place.hir_id, + ); self.walk_pat(discr_place, pat); } @@ -634,6 +666,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { /// - When reporting the Place back to the Delegate, ensure that the UpvarId uses the enclosing /// closure as the DefId. fn walk_captures(&mut self, closure_expr: &hir::Expr<'_>) { + fn upvar_is_local_variable( + upvars: Option<&'tcx FxIndexMap>, + upvar_id: &hir::HirId, + body_owner_is_closure: bool, + ) -> bool { + upvars.map(|upvars| !upvars.contains_key(upvar_id)).unwrap_or(body_owner_is_closure) + } + debug!("walk_captures({:?})", closure_expr); let closure_def_id = self.tcx().hir().local_def_id(closure_expr.hir_id).to_def_id(); @@ -645,16 +685,32 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { ty::Closure(..) | ty::Generator(..) ); + // If we have a nested closure, we want to include the fake reads present in the nested closure. if let Some(fake_reads) = self.mc.typeck_results.closure_fake_reads.get(&closure_def_id) { - for (fake_read, cause) in fake_reads.iter() { + for (fake_read, cause, hir_id) in fake_reads.iter() { match fake_read.base { PlaceBase::Upvar(upvar_id) => { - if upvars.map_or(body_owner_is_closure, |upvars| { - !upvars.contains_key(&upvar_id.var_path.hir_id) - }) { + if upvar_is_local_variable( + upvars, + &upvar_id.var_path.hir_id, + body_owner_is_closure, + ) { // The nested closure might be fake reading the current (enclosing) closure's local variables. - // We check if the root variable is ever mentioned within the enclosing closure, if not - // then for the current body (if it's a closure) these do not require fake_read, we will ignore them. + // The only places we want to fake read before creating the parent closure are the ones that + // are not local to it/ defined by it. + // + // ```rust,ignore(cannot-test-this-because-pseduo-code) + // let v1 = (0, 1); + // let c = || { // fake reads: v1 + // let v2 = (0, 1); + // let e = || { // fake reads: v1, v2 + // let (_, t1) = v1; + // let (_, t2) = v2; + // } + // } + // ``` + // This check is performed when visiting the body of the outermost closure (`c`) and ensures + // that we don't add a fake read of v2 in c. continue; } } @@ -665,7 +721,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { ); } }; - self.delegate.fake_read(fake_read.clone(), *cause); + self.delegate.fake_read(fake_read.clone(), *cause, *hir_id); } } diff --git a/src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.rs b/src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.rs new file mode 100644 index 0000000000000..609a11a578ae8 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.rs @@ -0,0 +1,84 @@ +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +//~| `#[warn(incomplete_features)]` on by default +//~| see issue #53488 +#![feature(never_type)] + +// Should fake read the discriminant and throw an error +fn test1() { + let x: !; + let c1 = || match x { }; + //~^ ERROR: use of possibly-uninitialized variable: `x` +} + +// Should fake read the discriminant and throw an error +fn test2() { + let x: !; + let c2 = || match x { _ => () }; + //~^ ERROR: borrow of possibly-uninitialized variable: `x` +} + +// Testing single variant patterns +enum SingleVariant { + Points(u32) +} + +// Should fake read the discriminant and throw an error +fn test3() { + let variant: !; + let c = || { + //~^ ERROR: borrow of possibly-uninitialized variable: `variant` + match variant { + SingleVariant::Points(_) => {} + } + }; + c(); +} + +// Should fake read the discriminant and throw an error +fn test4() { + let variant: !; + let c = || { + //~^ ERROR: borrow of possibly-uninitialized variable: `variant` + match variant { + SingleVariant::Points(a) => { + println!("{:?}", a); + } + } + }; + c(); +} + +fn test5() { + let t: !; + let g: !; + + let a = || { + match g { }; + //~^ ERROR: use of possibly-uninitialized variable: `g` + let c = || { + match t { }; + //~^ ERROR: use of possibly-uninitialized variable: `t` + }; + + c(); + }; + +} + +// Should fake read the discriminant and throw an error +fn test6() { + let x: u8; + let c1 = || match x { }; + //~^ ERROR: use of possibly-uninitialized variable: `x` + //~| ERROR: non-exhaustive patterns: type `u8` is non-empty +} + +fn main() { + test1(); + test2(); + test3(); + test4(); + test5(); + test6(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.stderr b/src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.stderr new file mode 100644 index 0000000000000..c225abb58b731 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/pattern-matching-should-fail.stderr @@ -0,0 +1,72 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/pattern-matching-should-fail.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error[E0004]: non-exhaustive patterns: type `u8` is non-empty + --> $DIR/pattern-matching-should-fail.rs:72:23 + | +LL | let c1 = || match x { }; + | ^ + | + = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms + = note: the matched value is of type `u8` + +error[E0381]: use of possibly-uninitialized variable: `x` + --> $DIR/pattern-matching-should-fail.rs:10:23 + | +LL | let c1 = || match x { }; + | ^ use of possibly-uninitialized `x` + +error[E0381]: borrow of possibly-uninitialized variable: `x` + --> $DIR/pattern-matching-should-fail.rs:17:14 + | +LL | let c2 = || match x { _ => () }; + | ^^ - borrow occurs due to use in closure + | | + | use of possibly-uninitialized `x` + +error[E0381]: borrow of possibly-uninitialized variable: `variant` + --> $DIR/pattern-matching-should-fail.rs:29:13 + | +LL | let c = || { + | ^^ use of possibly-uninitialized `variant` +LL | +LL | match variant { + | ------- borrow occurs due to use in closure + +error[E0381]: borrow of possibly-uninitialized variable: `variant` + --> $DIR/pattern-matching-should-fail.rs:41:13 + | +LL | let c = || { + | ^^ use of possibly-uninitialized `variant` +LL | +LL | match variant { + | ------- borrow occurs due to use in closure + +error[E0381]: use of possibly-uninitialized variable: `g` + --> $DIR/pattern-matching-should-fail.rs:57:15 + | +LL | match g { }; + | ^ use of possibly-uninitialized `g` + +error[E0381]: use of possibly-uninitialized variable: `t` + --> $DIR/pattern-matching-should-fail.rs:60:19 + | +LL | match t { }; + | ^ use of possibly-uninitialized `t` + +error[E0381]: use of possibly-uninitialized variable: `x` + --> $DIR/pattern-matching-should-fail.rs:72:23 + | +LL | let c1 = || match x { }; + | ^ use of possibly-uninitialized `x` + +error: aborting due to 8 previous errors; 1 warning emitted + +Some errors have detailed explanations: E0004, E0381. +For more information about an error, try `rustc --explain E0004`. diff --git a/src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.rs b/src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.rs new file mode 100644 index 0000000000000..0a877dd366c7f --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.rs @@ -0,0 +1,139 @@ +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +//~| NOTE: `#[warn(incomplete_features)]` on by default +//~| NOTE: see issue #53488 +#![feature(rustc_attrs)] + +// Should capture the discriminant since a variant of a multivariant enum is +// mentioned in the match arm; the discriminant is captured by the closure regardless +// of if it creates a binding +fn test_1_should_capture() { + let variant = Some(2229); + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + + || { + //~^ First Pass analysis includes: + //~| Min Capture analysis includes: + match variant { + //~^ NOTE: Capturing variant[] -> ImmBorrow + //~| NOTE: Min Capture variant[] -> ImmBorrow + Some(_) => {} + _ => {} + } + }; + c(); +} + +// Should not capture the discriminant since only a wildcard is mentioned in the +// match arm +fn test_2_should_not_capture() { + let variant = Some(2229); + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ First Pass analysis includes: + match variant { + _ => {} + } + }; + c(); +} + +// Testing single variant patterns +enum SingleVariant { + Points(u32) +} + +// Should not capture the discriminant since the single variant mentioned +// in the match arm does not trigger a binding +fn test_3_should_not_capture_single_variant() { + let variant = SingleVariant::Points(1); + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ First Pass analysis includes: + match variant { + SingleVariant::Points(_) => {} + } + }; + c(); +} + +// Should not capture the discriminant since the single variant mentioned +// in the match arm does not trigger a binding +fn test_6_should_capture_single_variant() { + let variant = SingleVariant::Points(1); + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ First Pass analysis includes: + //~| Min Capture analysis includes: + match variant { + //~^ NOTE: Capturing variant[] -> ImmBorrow + //~| NOTE: Capturing variant[(0, 0)] -> ImmBorrow + //~| NOTE: Min Capture variant[] -> ImmBorrow + SingleVariant::Points(a) => { + println!("{:?}", a); + } + } + }; + c(); +} + +// Should not capture the discriminant since only wildcards are mentioned in the +// match arm +fn test_4_should_not_capture_array() { + let array: [i32; 3] = [0; 3]; + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ First Pass analysis includes: + match array { + [_,_,_] => {} + } + }; + c(); +} + +// Testing MultiVariant patterns +enum MVariant { + A, + B, + C, +} + +// Should capture the discriminant since a variant of the multi variant enum is +// mentioned in the match arm; the discriminant is captured by the closure +// regardless of if it creates a binding +fn test_5_should_capture_multi_variant() { + let variant = MVariant::A; + let c = #[rustc_capture_analysis] + //~^ ERROR: attributes on expressions are experimental + //~| NOTE: see issue #15701 + || { + //~^ First Pass analysis includes: + //~| Min Capture analysis includes: + match variant { + //~^ NOTE: Capturing variant[] -> ImmBorrow + //~| NOTE: Min Capture variant[] -> ImmBorrow + MVariant::A => {} + _ => {} + } + }; + c(); +} + +fn main() { + test_1_should_capture(); + test_2_should_not_capture(); + test_3_should_not_capture_single_variant(); + test_6_should_capture_single_variant(); + test_4_should_not_capture_array(); + test_5_should_capture_multi_variant(); +} diff --git a/src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.stderr b/src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.stderr new file mode 100644 index 0000000000000..ad3e96a5753e3 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/patterns-capture-analysis.stderr @@ -0,0 +1,212 @@ +error[E0658]: attributes on expressions are experimental + --> $DIR/patterns-capture-analysis.rs:12:14 + | +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/patterns-capture-analysis.rs:33:14 + | +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/patterns-capture-analysis.rs:54:14 + | +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/patterns-capture-analysis.rs:70:14 + | +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/patterns-capture-analysis.rs:92:14 + | +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/patterns-capture-analysis.rs:116:14 + | +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 + +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/patterns-capture-analysis.rs:1:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +error: First Pass analysis includes: + --> $DIR/patterns-capture-analysis.rs:16:5 + | +LL | / || { +LL | | +LL | | +LL | | match variant { +... | +LL | | } +LL | | }; + | |_____^ + | +note: Capturing variant[] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:19:15 + | +LL | match variant { + | ^^^^^^^ + +error: Min Capture analysis includes: + --> $DIR/patterns-capture-analysis.rs:16:5 + | +LL | / || { +LL | | +LL | | +LL | | match variant { +... | +LL | | } +LL | | }; + | |_____^ + | +note: Min Capture variant[] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:19:15 + | +LL | match variant { + | ^^^^^^^ + +error: First Pass analysis includes: + --> $DIR/patterns-capture-analysis.rs:36:5 + | +LL | / || { +LL | | +LL | | match variant { +LL | | _ => {} +LL | | } +LL | | }; + | |_____^ + +error: First Pass analysis includes: + --> $DIR/patterns-capture-analysis.rs:57:5 + | +LL | / || { +LL | | +LL | | match variant { +LL | | SingleVariant::Points(_) => {} +LL | | } +LL | | }; + | |_____^ + +error: First Pass analysis includes: + --> $DIR/patterns-capture-analysis.rs:73:5 + | +LL | / || { +LL | | +LL | | +LL | | match variant { +... | +LL | | } +LL | | }; + | |_____^ + | +note: Capturing variant[] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:76:15 + | +LL | match variant { + | ^^^^^^^ +note: Capturing variant[(0, 0)] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:76:15 + | +LL | match variant { + | ^^^^^^^ + +error: Min Capture analysis includes: + --> $DIR/patterns-capture-analysis.rs:73:5 + | +LL | / || { +LL | | +LL | | +LL | | match variant { +... | +LL | | } +LL | | }; + | |_____^ + | +note: Min Capture variant[] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:76:15 + | +LL | match variant { + | ^^^^^^^ + +error: First Pass analysis includes: + --> $DIR/patterns-capture-analysis.rs:95:5 + | +LL | / || { +LL | | +LL | | match array { +LL | | [_,_,_] => {} +LL | | } +LL | | }; + | |_____^ + +error: First Pass analysis includes: + --> $DIR/patterns-capture-analysis.rs:119:5 + | +LL | / || { +LL | | +LL | | +LL | | match variant { +... | +LL | | } +LL | | }; + | |_____^ + | +note: Capturing variant[] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:122:15 + | +LL | match variant { + | ^^^^^^^ + +error: Min Capture analysis includes: + --> $DIR/patterns-capture-analysis.rs:119:5 + | +LL | / || { +LL | | +LL | | +LL | | match variant { +... | +LL | | } +LL | | }; + | |_____^ + | +note: Min Capture variant[] -> ImmBorrow + --> $DIR/patterns-capture-analysis.rs:122:15 + | +LL | match variant { + | ^^^^^^^ + +error: aborting due to 15 previous errors; 1 warning emitted + +For more information about this error, try `rustc --explain E0658`. diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/capture_with_wildcard_match.rs similarity index 61% rename from src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs rename to src/test/ui/closures/2229_closure_analysis/run_pass/capture_with_wildcard_match.rs index 6b3835634c6fe..eaea0dbfb5e8c 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/capture_with_wildcard_match.rs @@ -3,20 +3,23 @@ //~^ WARNING: the feature `capture_disjoint_fields` is incomplete fn test1() { - let foo = [1, 2, 3]; + let foo : [Vec; 3] = ["String".into(), "String".into(), "String".into()]; let c = || { match foo { _ => () }; }; + drop(foo); + c(); } fn test2() { - let foo = Some([1, 2, 3]); + let foo : Option<[Vec; 3]> = Some(["String".into(), "String".into(), "String".into()]); let c = || { match foo { Some(_) => 1, _ => 2 }; }; + c(); } fn main() { diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/capture_with_wildcard_match.stderr similarity index 88% rename from src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr rename to src/test/ui/closures/2229_closure_analysis/run_pass/capture_with_wildcard_match.stderr index 6f5b28aa6cf67..2c17a189afbbc 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/no_capture_with_wildcard_match.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/capture_with_wildcard_match.stderr @@ -1,5 +1,5 @@ warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/no_capture_with_wildcard_match.rs:2:12 + --> $DIR/capture_with_wildcard_match.rs:2:12 | LL | #![feature(capture_disjoint_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs index d91a28feae5c9..3ad083a92d569 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.rs @@ -4,7 +4,6 @@ #![warn(unused)] fn main() { - let _z = 9; let t = (String::from("Hello"), String::from("World")); let g = (String::from("Mr"), String::from("Goose")); diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr index d88c16567b74e..c4abf934123b3 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure-pattern-closure-within-closure.stderr @@ -8,7 +8,7 @@ LL | #![feature(capture_disjoint_fields)] = note: see issue #53488 for more information warning: unused variable: `t2` - --> $DIR/destructure-pattern-closure-within-closure.rs:15:21 + --> $DIR/destructure-pattern-closure-within-closure.rs:14:21 | LL | let (_, t2) = t; | ^^ help: if this is intentional, prefix it with an underscore: `_t2` @@ -21,7 +21,7 @@ LL | #![warn(unused)] = note: `#[warn(unused_variables)]` implied by `#[warn(unused)]` warning: unused variable: `g2` - --> $DIR/destructure-pattern-closure-within-closure.rs:12:17 + --> $DIR/destructure-pattern-closure-within-closure.rs:11:17 | LL | let (_, g2) = g; | ^^ help: if this is intentional, prefix it with an underscore: `_g2` diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs index 7d2c573dddd7c..65527648b2c99 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs @@ -80,7 +80,6 @@ fn test7() { }; } -// [FIXME] RFC2229 Add an explanation for test fn test8() { let x = 0; //~^ WARN unused variable: `x` @@ -90,10 +89,10 @@ fn test8() { let c = || { let _ = x; - let Point { x, y } = p; // 1 + let Point { x, y } = p; //~^ WARN unused variable: `x` println!("{}", y); - let (_, _) = tup; // 2 + let (_, _) = tup; }; c(); diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr index b92bf089e9ed2..fcfe9ee95f185 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/destructure_patterns.stderr @@ -45,19 +45,19 @@ LL | let t = (String::from("Hello"), String::from("World")); | ^ help: if this is intentional, prefix it with an underscore: `_t` warning: unused variable: `x` - --> $DIR/destructure_patterns.rs:93:21 + --> $DIR/destructure_patterns.rs:92:21 | -LL | let Point { x, y } = p; // 1 +LL | let Point { x, y } = p; | ^ help: try ignoring the field: `x: _` warning: unused variable: `x` - --> $DIR/destructure_patterns.rs:85:9 + --> $DIR/destructure_patterns.rs:84:9 | LL | let x = 0; | ^ help: if this is intentional, prefix it with an underscore: `_x` warning: unused variable: `tup` - --> $DIR/destructure_patterns.rs:87:9 + --> $DIR/destructure_patterns.rs:86:9 | LL | let tup = (1, 2); | ^^^ help: if this is intentional, prefix it with an underscore: `_tup` diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.rs new file mode 100644 index 0000000000000..dae50854d82fb --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.rs @@ -0,0 +1,12 @@ +//check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![feature(rustc_attrs)] + +fn main() { + let mut x = 1; + let c = || { + drop(&mut x); + match x { _ => () } + }; +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.stderr similarity index 89% rename from src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr rename to src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.stderr index 66171f35763d8..7f811875d1363 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.stderr +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/drop_then_use_fake_reads.stderr @@ -1,5 +1,5 @@ warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes - --> $DIR/struct_update_syntax.rs:2:12 + --> $DIR/drop_then_use_fake_reads.rs:2:12 | LL | #![feature(capture_disjoint_fields)] | ^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs index 97b56e7c4be22..9c086fe4bdfe7 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/lit-pattern-matching-with-methods.rs @@ -21,4 +21,11 @@ fn main() { assert!(matches!(result, Ok(None))); } + { + let mut it = map.drain_filter(|_, _| true); + catch_unwind(AssertUnwindSafe(|| while let Some(_) = it.next() {})).unwrap_err(); + let result = catch_unwind(AssertUnwindSafe(|| it.next())); + assert!(matches!(result, Ok(None))); + } + } diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs index 2f13590a2bdab..d260a448926d6 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/struct-pattern-matching-with-methods.rs @@ -11,6 +11,7 @@ enum PointType { ThreeD{ x: u32, y: u32, z: u32 } } +// Testing struct patterns struct Points { points: Vec, } diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs deleted file mode 100644 index 34710e69b9659..0000000000000 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/struct_update_syntax.rs +++ /dev/null @@ -1,25 +0,0 @@ -//check-pass -#![feature(capture_disjoint_fields)] -//~^ WARNING: the feature `capture_disjoint_fields` is incomplete -#![feature(rustc_attrs)] - -struct S { - a: String, - b: String, -} - -fn main() { - let s = S { - a: String::new(), - b: String::new(), - }; - - let c = || { - let s2 = S { - a: format!("New a"), - ..s - }; - }; - - c(); -} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs index 0948039287b2c..b3bee79254ec4 100644 --- a/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/tuple-struct-pattern-matching-with-methods.rs @@ -8,6 +8,7 @@ enum PointType { ThreeD(u32, u32, u32) } +// Testing tuple struct patterns struct Points { points: Vec, } diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.rs new file mode 100644 index 0000000000000..0e6da8f4f1889 --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.rs @@ -0,0 +1,12 @@ +//check-pass +#![feature(capture_disjoint_fields)] +//~^ WARNING: the feature `capture_disjoint_fields` is incomplete +#![feature(rustc_attrs)] + +fn main() { + let mut x = 0; + let c = || { + &mut x; // mutable borrow of `x` + match x { _ => () } // fake read of `x` + }; +} diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.stderr new file mode 100644 index 0000000000000..7d16d77bf737e --- /dev/null +++ b/src/test/ui/closures/2229_closure_analysis/run_pass/use_of_mutable_borrow_and_fake_reads.stderr @@ -0,0 +1,11 @@ +warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes + --> $DIR/use_of_mutable_borrow_and_fake_reads.rs:2:12 + | +LL | #![feature(capture_disjoint_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `#[warn(incomplete_features)]` on by default + = note: see issue #53488 for more information + +warning: 1 warning emitted + diff --git a/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr b/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr index a3469ee211425..285c203f382df 100644 --- a/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr +++ b/src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.stderr @@ -189,19 +189,18 @@ LL | drop(_x3); error[E0382]: use of moved value: `tup` --> $DIR/borrowck-move-ref-pattern.rs:43:14 | -LL | let mut tup = (U, U, U); - | ------- move occurs because `tup` has type `(U, U, U)`, which does not implement the `Copy` trait -LL | let c1 = || { - | -- value moved into closure here -LL | let (ref _x0, _x1, _) = tup; - | --- variable moved due to use in closure -LL | }; -LL | let c2 = || { - | ______________^ -LL | | -LL | | let (ref mut _x0, _, _x2) = tup; -LL | | }; - | |_____^ value used here after move +LL | let mut tup = (U, U, U); + | ------- move occurs because `tup` has type `(U, U, U)`, which does not implement the `Copy` trait +LL | let c1 = || { + | -- value moved into closure here +LL | let (ref _x0, _x1, _) = tup; + | --- variable moved due to use in closure +LL | }; +LL | let c2 = || { + | ^^ value used here after move +LL | +LL | let (ref mut _x0, _, _x2) = tup; + | --- use occurs due to use in closure error: aborting due to 18 previous errors diff --git a/src/tools/clippy/clippy_lints/src/escape.rs b/src/tools/clippy/clippy_lints/src/escape.rs index 6994e9f7d2e69..972167575475e 100644 --- a/src/tools/clippy/clippy_lints/src/escape.rs +++ b/src/tools/clippy/clippy_lints/src/escape.rs @@ -186,7 +186,7 @@ impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> { } } - fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause) { } + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause, _: HirId) { } } impl<'a, 'tcx> EscapeDelegate<'a, 'tcx> { diff --git a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs index d5a9d5e4e13b7..d439577f9c33b 100644 --- a/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs +++ b/src/tools/clippy/clippy_lints/src/needless_pass_by_value.rs @@ -335,5 +335,5 @@ impl<'tcx> euv::Delegate<'tcx> for MovedVariablesCtxt { fn mutate(&mut self, _: &euv::PlaceWithHirId<'tcx>, _: HirId) {} - fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause) { } + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause, _: HirId) { } } diff --git a/src/tools/clippy/clippy_utils/src/usage.rs b/src/tools/clippy/clippy_utils/src/usage.rs index 8aa8781f6be38..0b1ab6b7ea188 100644 --- a/src/tools/clippy/clippy_utils/src/usage.rs +++ b/src/tools/clippy/clippy_utils/src/usage.rs @@ -79,7 +79,7 @@ impl<'tcx> Delegate<'tcx> for MutVarsDelegate { self.update(&cmt) } - fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause) { } + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause, _:HirId) { } } pub struct ParamBindingIdCollector { From 189d2065229944258fe8f621c5e1ec4386b637d4 Mon Sep 17 00:00:00 2001 From: Roxane Date: Sun, 14 Mar 2021 23:53:43 -0400 Subject: [PATCH 8/8] Fix error after rebase --- .../src/build/expr/as_rvalue.rs | 7 ++--- .../rustc_mir_build/src/build/matches/mod.rs | 31 +++++++------------ .../rustc_mir_build/src/build/matches/test.rs | 2 +- compiler/rustc_mir_build/src/thir/cx/expr.rs | 19 +++--------- compiler/rustc_mir_build/src/thir/mod.rs | 2 +- .../clippy_lints/src/loops/mut_range_bound.rs | 3 ++ 6 files changed, 25 insertions(+), 39 deletions(-) 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 53c87d71e138b..ce816b6229e4b 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -165,7 +165,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { block.and(Rvalue::Aggregate(box AggregateKind::Tuple, fields)) } - ExprKind::Closure { closure_id, substs, upvars, movability, fake_reads } => { + ExprKind::Closure { closure_id, substs, upvars, movability, ref fake_reads } => { // Convert the closure fake reads, if any, from `ExprRef` to mir `Place` // and push the fake reads. // This must come before creating the operands. This is required in case @@ -179,7 +179,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // match x { _ => () } // fake read of `x` // }; // ``` - // FIXME(RFC2229): Remove feature gate once diagnostics are improved if this.tcx.features().capture_disjoint_fields { for (thir_place, cause, hir_id) in fake_reads.into_iter() { @@ -193,8 +192,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { place_builder_resolved.into_place(this.tcx, this.typeck_results); this.cfg.push_fake_read( block, - this.source_info(this.tcx.hir().span(hir_id)), - cause, + this.source_info(this.tcx.hir().span(*hir_id)), + *cause, mir_place, ); } diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 6e8b25c91628b..73fd3f0feb591 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -97,8 +97,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let scrutinee_place = unpack!(block = self.lower_scrutinee(block, scrutinee, scrutinee_span,)); - let mut arm_candidates = - self.create_match_candidates(scrutinee_place.clone(), &arms.clone()); + let mut arm_candidates = self.create_match_candidates(scrutinee_place.clone(), &arms); let match_has_guard = arms.iter().any(|arm| arm.guard.is_some()); let mut candidates = @@ -244,8 +243,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let arm_source_info = self.source_info(arm.span); let arm_scope = (arm.scope, arm_source_info); self.in_scope(arm_scope, arm.lint_level, |this| { - let body = arm.body; - // `try_upvars_resolved` may fail if it is unable to resolve the given // `PlaceBuilder` inside a closure. In this case, we don't want to include // a scrutinee place. `scrutinee_place_builder` will fail to be resolved @@ -264,7 +261,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { .try_upvars_resolved(this.tcx, this.typeck_results) { scrutinee_place = - scrutinee_builder.clone().into_place(this.tcx, this.typeck_results); + scrutinee_builder.into_place(this.tcx, this.typeck_results); opt_scrutinee_place = Some((Some(&scrutinee_place), scrutinee_span)); } let scope = this.declare_bindings( @@ -524,9 +521,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let Ok(match_pair_resolved) = initializer.clone().try_upvars_resolved(self.tcx, self.typeck_results) { - let place = match_pair_resolved - .clone() - .into_place(self.tcx, self.typeck_results); + let place = + match_pair_resolved.into_place(self.tcx, self.typeck_results); *match_place = Some(place); } } else { @@ -1480,7 +1476,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } TestKind::Switch { adt_def: _, ref mut variants } => { for candidate in candidates.iter() { - if !self.add_variants_to_switch(&match_place.clone(), candidate, variants) { + if !self.add_variants_to_switch(&match_place, candidate, variants) { break; } } @@ -1493,8 +1489,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { if let Ok(match_place_resolved) = match_place.clone().try_upvars_resolved(self.tcx, self.typeck_results) { - let resolved_place = - match_place_resolved.clone().into_place(self.tcx, self.typeck_results); + let resolved_place = match_place_resolved.into_place(self.tcx, self.typeck_results); fb.insert(resolved_place); } } @@ -1577,7 +1572,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { target_blocks }; - self.perform_test(block, match_place.clone(), &test, make_target_blocks); + self.perform_test(block, match_place, &test, make_target_blocks); } /// Determine the fake borrows that are needed from a set of places that @@ -1811,9 +1806,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { } Guard::IfLet(pat, scrutinee) => { let scrutinee_span = scrutinee.span; - let scrutinee_place_builder = unpack!( - block = self.lower_scrutinee(block, scrutinee.clone(), scrutinee_span) - ); + let scrutinee_place_builder = + unpack!(block = self.lower_scrutinee(block, scrutinee, scrutinee_span)); let mut guard_candidate = Candidate::new(scrutinee_place_builder.clone(), &pat, false); let wildcard = Pat::wildcard_from_ty(pat.ty); @@ -1827,12 +1821,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ); let mut opt_scrutinee_place: Option<(Option<&Place<'tcx>>, Span)> = None; let scrutinee_place: Place<'tcx>; - if let Ok(scrutinee_builder) = scrutinee_place_builder - .clone() - .try_upvars_resolved(self.tcx, self.typeck_results) + if let Ok(scrutinee_builder) = + scrutinee_place_builder.try_upvars_resolved(self.tcx, self.typeck_results) { scrutinee_place = - scrutinee_builder.clone().into_place(self.tcx, self.typeck_results); + scrutinee_builder.into_place(self.tcx, self.typeck_results); opt_scrutinee_place = Some((Some(&scrutinee_place), scrutinee_span)); } self.declare_bindings( diff --git a/compiler/rustc_mir_build/src/build/matches/test.rs b/compiler/rustc_mir_build/src/build/matches/test.rs index b13a0ef820157..6820d4ba35ae3 100644 --- a/compiler/rustc_mir_build/src/build/matches/test.rs +++ b/compiler/rustc_mir_build/src/build/matches/test.rs @@ -158,7 +158,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ) { let place: Place<'tcx>; if let Ok(test_place_builder) = - place_builder.clone().try_upvars_resolved(self.tcx, self.typeck_results) + place_builder.try_upvars_resolved(self.tcx, self.typeck_results) { place = test_place_builder.into_place(self.tcx, self.typeck_results); } else { diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index 2f58f2975b115..43d63d59ed964 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -455,28 +455,19 @@ impl<'thir, 'tcx> Cx<'thir, 'tcx> { ); // Convert the closure fake reads, if any, from hir `Place` to ExprRef - let fake_reads = match self.typeck_results().closure_fake_reads.get(&def_id) { + let fake_reads = match self.typeck_results.closure_fake_reads.get(&def_id) { Some(fake_reads) => fake_reads .iter() .map(|(place, cause, hir_id)| { - ( - self.arena - .alloc(self.convert_captured_hir_place(expr, place.clone())), - *cause, - *hir_id, - ) + let expr = self.convert_captured_hir_place(expr, place.clone()); + let expr_ref: &'thir Expr<'thir, 'tcx> = self.arena.alloc(expr); + (expr_ref, *cause, *hir_id) }) .collect(), None => Vec::new(), }; - ExprKind::Closure { - closure_id: def_id, - substs, - upvars, - movability, - fake_reads: fake_reads, - } + ExprKind::Closure { closure_id: def_id, substs, upvars, movability, fake_reads } } hir::ExprKind::Path(ref qpath) => { diff --git a/compiler/rustc_mir_build/src/thir/mod.rs b/compiler/rustc_mir_build/src/thir/mod.rs index 71d3093854d6c..6f20195db0b5e 100644 --- a/compiler/rustc_mir_build/src/thir/mod.rs +++ b/compiler/rustc_mir_build/src/thir/mod.rs @@ -281,7 +281,7 @@ pub enum ExprKind<'thir, 'tcx> { substs: UpvarSubsts<'tcx>, upvars: &'thir [Expr<'thir, 'tcx>], movability: Option, - fake_reads: Vec<(&'thir mut Expr<'thir, 'tcx>, FakeReadCause, hir::HirId)>, + fake_reads: Vec<(&'thir Expr<'thir, 'tcx>, FakeReadCause, hir::HirId)>, }, Literal { literal: &'tcx Const<'tcx>, diff --git a/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs b/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs index 3ae592950f13b..cb56512db60fe 100644 --- a/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs +++ b/src/tools/clippy/clippy_lints/src/loops/mut_range_bound.rs @@ -4,6 +4,7 @@ use if_chain::if_chain; use rustc_hir::{BindingAnnotation, Expr, HirId, Node, PatKind}; use rustc_infer::infer::TyCtxtInferExt; use rustc_lint::LateContext; +use rustc_middle::mir::FakeReadCause; use rustc_middle::ty; use rustc_span::source_map::Span; use rustc_typeck::expr_use_visitor::{ConsumeMode, Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; @@ -106,6 +107,8 @@ impl<'tcx> Delegate<'tcx> for MutatePairDelegate<'_, 'tcx> { } } } + + fn fake_read(&mut self, _: rustc_typeck::expr_use_visitor::Place<'tcx>, _: FakeReadCause, _:HirId) { } } impl MutatePairDelegate<'_, '_> {