diff options
11 files changed, 193 insertions, 49 deletions
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 6519d5fae1c..0e70cd36efb 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 b2e8b2de1bc..47f75825fb6 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 dc4f2886456..3ad143a57ff 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 6b7bd2e9dcc..dee69793f2a 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 6dbf8094548..0172f993c26 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 00000000000..97b56e7c4be --- /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 00000000000..bc046ecad68 --- /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 <https://github.com/rust-lang/rust/issues/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 00000000000..2f13590a2bd --- /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<PointType>, +} + +impl Points { + pub fn test1(&mut self) -> Vec<usize> { + (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::<PointType>::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 00000000000..3e4303a3710 --- /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 <https://github.com/rust-lang/rust/issues/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 00000000000..0948039287b --- /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<PointType>, +} + +impl Points { + pub fn test1(&mut self) -> Vec<usize> { + (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::<PointType>::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 00000000000..ded0e37b0f3 --- /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 <https://github.com/rust-lang/rust/issues/53488> for more information + +warning: 1 warning emitted + |