diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index ce4439e7c5464..3d154e43a9ae1 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1467,6 +1467,14 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { self.borrowck_mode().use_mir() } + /// If true, pattern variables for use in guards on match arms + /// will be bound as references to the data, and occurrences of + /// those variables in the guard expression will implicitly + /// dereference those bindings. (See rust-lang/rust#27282.) + pub fn all_pat_vars_are_implicit_refs_within_guards(self) -> bool { + self.borrowck_mode().use_mir() + } + /// If true, we should enable two-phase borrows checks. This is /// done with either `-Ztwo-phase-borrows` or with /// `#![feature(nll)]`. diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 3dc5a7a84900c..db2e078586eda 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -471,7 +471,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { | (RegionKind::ReClosureBound(_), _) | (RegionKind::ReCanonical(_), _) | (RegionKind::ReErased, _) => { - span_bug!(drop_span, "region does not make sense in this context"); + span_bug!(drop_span, "region {:?} does not make sense in this context", + borrow.region); } } } diff --git a/src/librustc_mir/build/block.rs b/src/librustc_mir/build/block.rs index 94702927d2600..fae06db31629b 100644 --- a/src/librustc_mir/build/block.rs +++ b/src/librustc_mir/build/block.rs @@ -9,6 +9,8 @@ // except according to those terms. use build::{BlockAnd, BlockAndExtension, Builder}; +use build::ForGuard::OutsideGuard; +use build::matches::ArmHasGuard; use hair::*; use rustc::mir::*; use rustc::hir; @@ -113,7 +115,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Declare the bindings, which may create a visibility scope. let remainder_span = remainder_scope.span(this.hir.tcx(), &this.hir.region_scope_tree); - let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern); + let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern, + ArmHasGuard(false)); // Evaluate the initializer, if present. if let Some(init) = initializer { @@ -135,8 +138,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } this.visit_bindings(&pattern, &mut |this, _, _, node, span, _| { - this.storage_live_binding(block, node, span); - this.schedule_drop_for_binding(node, span); + this.storage_live_binding(block, node, span, OutsideGuard); + this.schedule_drop_for_binding(node, span, OutsideGuard); }) } diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 19ec13324d6b4..365b9babd0869 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -11,6 +11,7 @@ //! See docs in build/expr/mod.rs use build::{BlockAnd, BlockAndExtension, Builder}; +use build::ForGuard::{OutsideGuard, WithinGuard}; use build::expr::category::Category; use hair::*; use rustc::mir::*; @@ -86,8 +87,18 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { block.and(Place::Local(Local::new(1))) } ExprKind::VarRef { id } => { - let index = this.var_indices[&id]; - block.and(Place::Local(index)) + let place = if this.is_bound_var_in_guard(id) { + let index = this.var_local_id(id, WithinGuard); + if this.hir.tcx().all_pat_vars_are_implicit_refs_within_guards() { + Place::Local(index).deref() + } else { + Place::Local(index) + } + } else { + let index = this.var_local_id(id, OutsideGuard); + Place::Local(index) + }; + block.and(place) } ExprKind::StaticRef { id } => { block.and(Place::Static(Box::new(Static { def_id: id, ty: expr.ty }))) diff --git a/src/librustc_mir/build/matches/mod.rs b/src/librustc_mir/build/matches/mod.rs index 7eb52a3cdee93..6946ac4c7b277 100644 --- a/src/librustc_mir/build/matches/mod.rs +++ b/src/librustc_mir/build/matches/mod.rs @@ -14,6 +14,8 @@ //! details. use build::{BlockAnd, BlockAndExtension, Builder}; +use build::{GuardFrame, GuardFrameLocal, LocalsForNode}; +use build::ForGuard::{self, OutsideGuard, WithinGuard}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::bitvec::BitVector; use rustc::ty::{self, Ty}; @@ -28,6 +30,11 @@ mod simplify; mod test; mod util; +/// ArmHasGuard is isomorphic to a boolean flag. It indicates whether +/// a match arm has a guard expression attached to it. +#[derive(Copy, Clone, Debug)] +pub(crate) struct ArmHasGuard(pub bool); + impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { pub fn match_expr(&mut self, destination: &Place<'tcx>, @@ -66,7 +73,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let body = self.hir.mirror(arm.body.clone()); let scope = self.declare_bindings(None, body.span, LintLevel::Inherited, - &arm.patterns[0]); + &arm.patterns[0], + ArmHasGuard(arm.guard.is_some())); (body, scope.unwrap_or(self.visibility_scope)) }).collect(); @@ -149,7 +157,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { var: NodeId, span: Span) { if self.hir.tcx().sess.opts.debugging_opts.disable_nll_user_type_assert { return; } - let local_id = self.var_indices[&var]; + let local_id = self.var_local_id(var, OutsideGuard); let source_info = self.source_info(span); debug!("user_assert_ty: local_id={:?}", hir_id.local_id); @@ -173,14 +181,15 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { PatternKind::Binding { mode: BindingMode::ByValue, var, subpattern: None, .. } => { - let place = self.storage_live_binding(block, var, irrefutable_pat.span); + let place = self.storage_live_binding(block, var, irrefutable_pat.span, + OutsideGuard); if let Some(ty) = ty { self.user_assert_ty(block, ty, var, irrefutable_pat.span); } unpack!(block = self.into(&place, block, initializer)); - self.schedule_drop_for_binding(var, irrefutable_pat.span); + self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard); block.unit() } _ => { @@ -220,7 +229,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } // now apply the bindings, which will also declare the variables - self.bind_matched_candidate(block, candidate.bindings); + self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, false); block.unit() } @@ -232,7 +241,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { mut var_scope: Option, scope_span: Span, lint_level: LintLevel, - pattern: &Pattern<'tcx>) + pattern: &Pattern<'tcx>, + has_guard: ArmHasGuard) -> Option { assert!(!(var_scope.is_some() && lint_level.is_explicit()), "can't have both a var and a lint scope at the same time"); @@ -254,15 +264,20 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { span, scope: var_scope.unwrap() }; - this.declare_binding(source_info, syntactic_scope, mutability, name, var, ty); + this.declare_binding(source_info, syntactic_scope, mutability, name, var, + ty, has_guard); }); var_scope } - pub fn storage_live_binding(&mut self, block: BasicBlock, var: NodeId, span: Span) + pub fn storage_live_binding(&mut self, + block: BasicBlock, + var: NodeId, + span: Span, + for_guard: ForGuard) -> Place<'tcx> { - let local_id = self.var_indices[&var]; + let local_id = self.var_local_id(var, for_guard); let source_info = self.source_info(span); self.cfg.push(block, Statement { source_info, @@ -271,8 +286,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { Place::Local(local_id) } - pub fn schedule_drop_for_binding(&mut self, var: NodeId, span: Span) { - let local_id = self.var_indices[&var]; + pub fn schedule_drop_for_binding(&mut self, + var: NodeId, + span: Span, + for_guard: ForGuard) { + let local_id = self.var_local_id(var, for_guard); let var_ty = self.local_decls[local_id].ty; let hir_id = self.hir.tcx().hir.node_to_hir_id(var); let region_scope = self.hir.region_scope_tree.var_scope(hir_id.local_id); @@ -770,14 +788,129 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { vec![candidate.next_candidate_pre_binding_block], }); - self.bind_matched_candidate(block, candidate.bindings); + // rust-lang/rust#27282: The `autoref` business deserves some + // explanation here. + // + // The intent of the `autoref` flag is that when it is true, + // then any pattern bindings of type T will map to a `&T` + // within the context of the guard expression, but will + // continue to map to a `T` in the context of the arm body. To + // avoid surfacing this distinction in the user source code + // (which would be a severe change to the language and require + // far more revision to the compiler), when `autoref` is true, + // then any occurrence of the identifier in the guard + // expression will automatically get a deref op applied to it. + // + // So an input like: + // + // ``` + // let place = Foo::new(); + // match place { foo if inspect(foo) + // => feed(foo), ... } + // ``` + // + // will be treated as if it were really something like: + // + // ``` + // let place = Foo::new(); + // match place { Foo { .. } if { let tmp1 = &place; inspect(*tmp1) } + // => { let tmp2 = place; feed(tmp2) }, ... } + // + // And an input like: + // + // ``` + // let place = Foo::new(); + // match place { ref mut foo if inspect(foo) + // => feed(foo), ... } + // ``` + // + // will be treated as if it were really something like: + // + // ``` + // let place = Foo::new(); + // match place { Foo { .. } if { let tmp1 = & &mut place; inspect(*tmp1) } + // => { let tmp2 = &mut place; feed(tmp2) }, ... } + // ``` + // + // In short, any pattern binding will always look like *some* + // kind of `&T` within the guard at least in terms of how the + // MIR-borrowck views it, and this will ensure that guard + // expressions cannot mutate their the match inputs via such + // bindings. (It also ensures that guard expressions can at + // most *copy* values from such bindings; non-Copy things + // cannot be moved via pattern bindings in guard expressions.) + // + // ---- + // + // Implementation notes (under assumption `autoref` is true). + // + // To encode the distinction above, we must inject the + // temporaries `tmp1` and `tmp2`. + // + // There are two cases of interest: binding by-value, and binding by-ref. + // + // 1. Binding by-value: Things are simple. + // + // * Establishing `tmp1` creates a reference into the + // matched place. This code is emitted by + // bind_matched_candidate_for_guard. + // + // * `tmp2` is only initialized "lazily", after we have + // checked the guard. Thus, the code that can trigger + // moves out of the candidate can only fire after the + // guard evaluated to true. This initialization code is + // emitted by bind_matched_candidate_for_arm. + // + // 2. Binding by-reference: Things are tricky. + // + // * Here, the guard expression wants a `&&` or `&&mut` + // into the original input. This means we need to borrow + // a reference that we do not immediately have at hand + // (because all we have is the places associated with the + // match input itself; it is up to us to create a place + // holding a `&` or `&mut` that we can then borrow). + // + // * Therefore, when the binding is by-reference, we + // *eagerly* introduce the binding for the arm body + // (`tmp2`) and then borrow it (`tmp1`). + // + // * This is documented with "NOTE tricky business" below. + // + // FIXME The distinction in how `tmp2` is initialized is + // currently encoded in a pretty awkward fashion; namely, by + // passing a boolean to bind_matched_candidate_for_arm_body + // indicating whether all of the by-ref bindings were already + // initialized. + // + // * Also: pnkfelix thinks "laziness" is natural; but since + // MIR-borrowck did not complain with earlier (universally + // eager) MIR codegen, laziness might not be *necessary*. + + let autoref = self.hir.tcx().all_pat_vars_are_implicit_refs_within_guards(); if let Some(guard) = candidate.guard { + if autoref { + self.bind_matched_candidate_for_guard(block, &candidate.bindings); + let guard_frame = GuardFrame { + locals: candidate.bindings.iter() + .map(|b| GuardFrameLocal::new(b.var_id, b.binding_mode)) + .collect(), + }; + debug!("Entering guard translation context: {:?}", guard_frame); + self.guard_context.push(guard_frame); + } else { + self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, false); + } + // the block to branch to if the guard fails; if there is no // guard, this block is simply unreachable let guard = self.hir.mirror(guard); let source_info = self.source_info(guard.span); let cond = unpack!(block = self.as_local_operand(block, guard)); + if autoref { + let guard_frame = self.guard_context.pop().unwrap(); + debug!("Exiting guard translation context with locals: {:?}", guard_frame); + } let false_edge_block = self.cfg.start_new_block(); self.cfg.terminate(block, source_info, @@ -785,6 +918,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { false_edge_block)); let otherwise = self.cfg.start_new_block(); + if autoref { + self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, true); + } self.cfg.terminate(false_edge_block, source_info, TerminatorKind::FalseEdges { real_target: otherwise, @@ -793,47 +929,137 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { }); Some(otherwise) } else { + self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, false); self.cfg.terminate(block, candidate_source_info, TerminatorKind::Goto { target: arm_block }); None } } - fn bind_matched_candidate(&mut self, - block: BasicBlock, - bindings: Vec>) { - debug!("bind_matched_candidate(block={:?}, bindings={:?})", + fn bind_matched_candidate_for_guard(&mut self, + block: BasicBlock, + bindings: &[Binding<'tcx>]) { + debug!("bind_matched_candidate_for_guard(block={:?}, bindings={:?})", block, bindings); + // Assign each of the bindings. Since we are binding for a + // guard expression, this will never trigger moves out of the + // candidate. + let re_empty = self.hir.tcx().types.re_empty; + for binding in bindings { + let source_info = self.source_info(binding.span); + let local_for_guard = self.storage_live_binding( + block, binding.var_id, binding.span, WithinGuard); + // Question: Why schedule drops if bindings are all + // shared-&'s? Answer: Because schedule_drop_for_binding + // also emits StorageDead's for those locals. + self.schedule_drop_for_binding(binding.var_id, binding.span, WithinGuard); + match binding.binding_mode { + BindingMode::ByValue => { + let rvalue = Rvalue::Ref(re_empty, BorrowKind::Shared, binding.source.clone()); + self.cfg.push_assign(block, source_info, &local_for_guard, rvalue); + } + BindingMode::ByRef(region, borrow_kind) => { + // NOTE tricky business: For `ref id` and `ref mut + // id` patterns, we want `id` within the guard to + // correspond to a temp of type `& &T` or `& &mut + // T`, while within the arm body it will + // correspond to a temp of type `&T` or `&mut T`, + // as usual. + // + // But to inject the level of indirection, we need + // something to point to. + // + // So: + // + // 1. First set up the local for the arm body + // (even though we have not yet evaluated the + // guard itself), + // + // 2. Then setup the local for the guard, which is + // just a reference to the local from step 1. + // + // Note that since we are setting up the local for + // the arm body a bit eagerly here (and likewise + // scheduling its drop code), we should *not* do + // it redundantly later on. + // + // While we could have kept track of this with a + // flag or collection of such bindings, the + // treatment of all of these cases is uniform, so + // we should be safe just avoiding the code + // without maintaining such state.) + let local_for_arm_body = self.storage_live_binding( + block, binding.var_id, binding.span, OutsideGuard); + self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard); + + // rust-lang/rust#27282: this potentially mutable + // borrow may require a cast in the future to + // avoid conflicting with an implicit borrow of + // the whole match input; or maybe it just + // requires an extension of our two-phase borrows + // system. See discussion on rust-lang/rust#49870. + let rvalue = Rvalue::Ref(region, borrow_kind, binding.source.clone()); + self.cfg.push_assign(block, source_info, &local_for_arm_body, rvalue); + let rvalue = Rvalue::Ref(region, BorrowKind::Shared, local_for_arm_body); + self.cfg.push_assign(block, source_info, &local_for_guard, rvalue); + } + } + } + } + + fn bind_matched_candidate_for_arm_body(&mut self, + block: BasicBlock, + bindings: &[Binding<'tcx>], + already_initialized_state_for_refs: bool) { + debug!("bind_matched_candidate_for_arm_body(block={:?}, bindings={:?}, \ + already_initialized_state_for_refs={:?})", + block, bindings, already_initialized_state_for_refs); + // Assign each of the bindings. This may trigger moves out of the candidate. for binding in bindings { + if let BindingMode::ByRef(..) = binding.binding_mode { + // See "NOTE tricky business" above + if already_initialized_state_for_refs { continue; } + } + let source_info = self.source_info(binding.span); - let local = self.storage_live_binding(block, binding.var_id, binding.span); - self.schedule_drop_for_binding(binding.var_id, binding.span); + let local = self.storage_live_binding(block, binding.var_id, binding.span, + OutsideGuard); + self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard); let rvalue = match binding.binding_mode { - BindingMode::ByValue => - Rvalue::Use(self.consume_by_copy_or_move(binding.source)), - BindingMode::ByRef(region, borrow_kind) => - Rvalue::Ref(region, borrow_kind, binding.source), + BindingMode::ByValue => { + Rvalue::Use(self.consume_by_copy_or_move(binding.source.clone())) + } + BindingMode::ByRef(region, borrow_kind) => { + Rvalue::Ref(region, borrow_kind, binding.source.clone()) + } }; self.cfg.push_assign(block, source_info, &local, rvalue); } } + /// Each binding (`ref mut var`/`ref var`/`mut var`/`var`, where + /// the bound `var` has type `T` in the arm body) in a pattern + /// maps to *two* locals. The first local is a binding for + /// occurrences of `var` in the guard, which will all have type + /// `&T`. The second local is a binding for occurrences of `var` + /// in the arm body, which will have type `T`. fn declare_binding(&mut self, source_info: SourceInfo, syntactic_scope: VisibilityScope, mutability: Mutability, name: Name, var_id: NodeId, - var_ty: Ty<'tcx>) - -> Local + var_ty: Ty<'tcx>, + has_guard: ArmHasGuard) { debug!("declare_binding(var_id={:?}, name={:?}, var_ty={:?}, source_info={:?}, \ syntactic_scope={:?})", var_id, name, var_ty, source_info, syntactic_scope); - let var = self.local_decls.push(LocalDecl::<'tcx> { + let tcx = self.hir.tcx(); + let for_arm_body = self.local_decls.push(LocalDecl::<'tcx> { mutability, ty: var_ty.clone(), name: Some(name), @@ -842,10 +1068,21 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { internal: false, is_user_variable: true, }); - self.var_indices.insert(var_id, var); - - debug!("declare_binding: var={:?}", var); - - var + let locals = if has_guard.0 && tcx.all_pat_vars_are_implicit_refs_within_guards() { + let for_guard = self.local_decls.push(LocalDecl::<'tcx> { + mutability, + ty: tcx.mk_imm_ref(tcx.types.re_empty, var_ty), + name: Some(name), + source_info, + syntactic_scope, + internal: false, + is_user_variable: true, + }); + LocalsForNode::Two { for_guard, for_arm_body } + } else { + LocalsForNode::One(for_arm_body) + }; + debug!("declare_binding: vars={:?}", locals); + self.var_indices.insert(var_id, locals); } } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 0d836f5cb9737..dd2a336af41d0 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -291,8 +291,14 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { visibility_scope_info: IndexVec, visibility_scope: VisibilityScope, + /// the guard-context: each time we build the guard expression for + /// a match arm, we push onto this stack, and then pop when we + /// finish building it. + guard_context: Vec, + /// Maps node ids of variable bindings to the `Local`s created for them. - var_indices: NodeMap, + /// (A match binding can have two locals; the 2nd is for the arm's guard.) + var_indices: NodeMap, local_decls: IndexVec>, unit_temp: Option>, @@ -305,6 +311,74 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { cached_unreachable_block: Option, } +impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { + fn is_bound_var_in_guard(&self, id: ast::NodeId) -> bool { + self.guard_context.iter().any(|frame| frame.locals.iter().any(|local| local.id == id)) + } + + fn var_local_id(&self, id: ast::NodeId, for_guard: ForGuard) -> Local { + self.var_indices[&id].local_id(for_guard) + } +} + +#[derive(Debug)] +enum LocalsForNode { + One(Local), + Two { for_guard: Local, for_arm_body: Local }, +} + +#[derive(Debug)] +struct GuardFrameLocal { + id: ast::NodeId, +} + +impl GuardFrameLocal { + fn new(id: ast::NodeId, _binding_mode: BindingMode) -> Self { + GuardFrameLocal { + id: id, + } + } +} + +#[derive(Debug)] +struct GuardFrame { + /// These are the id's of names that are bound by patterns of the + /// arm of *this* guard. + /// + /// (Frames higher up the stack will have the id's bound in arms + /// further out, such as in a case like: + /// + /// match E1 { + /// P1(id1) if (... (match E2 { P2(id2) if ... => B2 })) => B1, + /// } + /// + /// here, when building for FIXME + locals: Vec, +} + +/// ForGuard is isomorphic to a boolean flag. It indicates whether we are +/// talking about the temp for a local binding for a use within a guard expression, +/// or a temp for use outside of a guard expressions. +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum ForGuard { + WithinGuard, + OutsideGuard, +} + +impl LocalsForNode { + fn local_id(&self, for_guard: ForGuard) -> Local { + match (self, for_guard) { + (&LocalsForNode::One(local_id), ForGuard::OutsideGuard) | + (&LocalsForNode::Two { for_guard: local_id, .. }, ForGuard::WithinGuard) | + (&LocalsForNode::Two { for_arm_body: local_id, .. }, ForGuard::OutsideGuard) => + local_id, + + (&LocalsForNode::One(_), ForGuard::WithinGuard) => + bug!("anything with one local should never be within a guard."), + } + } +} + struct CFG<'tcx> { basic_blocks: IndexVec>, } @@ -548,6 +622,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { visibility_scopes: IndexVec::new(), visibility_scope: ARGUMENT_VISIBILITY_SCOPE, visibility_scope_info: IndexVec::new(), + guard_context: vec![], push_unsafe_count: 0, unpushed_unsafe: safety, breakable_scopes: vec![], @@ -636,11 +711,12 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { // Don't introduce extra copies for simple bindings PatternKind::Binding { mutability, var, mode: BindingMode::ByValue, .. } => { self.local_decls[local].mutability = mutability; - self.var_indices.insert(var, local); + self.var_indices.insert(var, LocalsForNode::One(local)); } _ => { scope = self.declare_bindings(scope, ast_body.span, - LintLevel::Inherited, &pattern); + LintLevel::Inherited, &pattern, + matches::ArmHasGuard(false)); unpack!(block = self.place_into_pattern(block, pattern, &place)); } } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index a21691813a4d4..098ad8e558fa9 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -179,8 +179,14 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { if let RegionKind::ReEmpty = region { // If the borrowed value dies before the borrow is used, the region for // the borrow can be empty. Don't track the borrow in that case. + debug!("Borrows::statement_effect_on_borrows \ + location: {:?} stmt: {:?} has empty region, killing {:?}", + location, stmt.kind, index); sets.kill(&index); return + } else { + debug!("Borrows::statement_effect_on_borrows location: {:?} stmt: {:?}", + location, stmt.kind); } assert!(self.borrow_set.region_map.get(region).unwrap_or_else(|| { diff --git a/src/test/compile-fail/nll/match-guards-always-borrow.rs b/src/test/compile-fail/nll/match-guards-always-borrow.rs new file mode 100644 index 0000000000000..985531446270e --- /dev/null +++ b/src/test/compile-fail/nll/match-guards-always-borrow.rs @@ -0,0 +1,61 @@ +// Copyright 2012-2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//revisions: ast mir +//[mir] compile-flags: -Z borrowck=mir + +#![feature(rustc_attrs)] + +// Here is arielb1's basic example from rust-lang/rust#27282 +// that AST borrowck is flummoxed by: + +fn should_reject_destructive_mutate_in_guard() { + match Some(&4) { + None => {}, + ref mut foo if { + (|| { let bar = foo; bar.take() })(); + //[mir]~^ ERROR cannot move out of borrowed content [E0507] + false } => { }, + Some(s) => std::process::exit(*s), + } +} + +// Here below is a case that needs to keep working: we only use the +// binding via immutable-borrow in the guard, and we mutate in the arm +// body. +fn allow_mutate_in_arm_body() { + match Some(&4) { + None => {}, + ref mut foo if foo.is_some() && false => { foo.take(); () } + Some(s) => std::process::exit(*s), + } +} + +// Here below is a case that needs to keep working: we only use the +// binding via immutable-borrow in the guard, and we move into the arm +// body. +fn allow_move_into_arm_body() { + match Some(&4) { + None => {}, + mut foo if foo.is_some() && false => { foo.take(); () } + Some(s) => std::process::exit(*s), + } +} + +// Since this is a compile-fail test that is explicitly encoding the +// different behavior of AST- vs MIR-borrowck where AST-borrowck does +// not error, we need to use rustc_error to placate the test harness +// that wants *some* error to occur. +#[rustc_error] +fn main() { //[ast]~ ERROR compilation successful + should_reject_destructive_mutate_in_guard(); + allow_mutate_in_arm_body(); + allow_move_into_arm_body(); +} diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index 53f178619975e..a31298a0f5160 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -54,17 +54,17 @@ fn main() { // ... // _2 = std::option::Option::Some(const 42i32,); // _3 = discriminant(_2); -// _6 = discriminant(_2); -// switchInt(move _6) -> [0isize: bb6, 1isize: bb4, otherwise: bb8]; +// _7 = discriminant(_2); +// switchInt(move _7) -> [0isize: bb6, 1isize: bb4, otherwise: bb8]; // } // bb1: { // resume; // } // bb2: { // arm1 -// StorageLive(_8); -// _8 = _4; -// _1 = (const 1i32, move _8); -// StorageDead(_8); +// StorageLive(_9); +// _9 = _4; +// _1 = (const 1i32, move _9); +// StorageDead(_9); // goto -> bb13; // } // bb3: { // binding3(empty) and arm3 @@ -87,24 +87,26 @@ fn main() { // unreachable; // } // bb9: { // binding1 and guard -// StorageLive(_4); -// _4 = ((_2 as Some).0: i32); -// StorageLive(_7); -// _7 = const guard() -> [return: bb10, unwind: bb1]; +// StorageLive(_5); +// _5 = &((_2 as Some).0: i32); +// StorageLive(_8); +// _8 = const guard() -> [return: bb10, unwind: bb1]; // } // bb10: { // end of guard -// switchInt(move _7) -> [false: bb11, otherwise: bb2]; +// StorageLive(_4); +// _4 = ((_2 as Some).0: i32); +// switchInt(move _8) -> [false: bb11, otherwise: bb2]; // } // bb11: { // to pre_binding2 // falseEdges -> [real: bb5, imaginary: bb5]; // } // bb12: { // bindingNoLandingPads.before.mir2 and arm2 -// StorageLive(_5); -// _5 = ((_2 as Some).0: i32); -// StorageLive(_9); -// _9 = _5; -// _1 = (const 2i32, move _9); -// StorageDead(_9); +// StorageLive(_6); +// _6 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = _6; +// _1 = (const 2i32, move _10); +// StorageDead(_10); // goto -> bb13; // } // bb13: { @@ -118,17 +120,17 @@ fn main() { // ... // _2 = std::option::Option::Some(const 42i32,); // _3 = discriminant(_2); -// _6 = discriminant(_2); -// switchInt(move _6) -> [0isize: bb5, 1isize: bb4, otherwise: bb8]; +// _7 = discriminant(_2); +// switchInt(move _7) -> [0isize: bb5, 1isize: bb4, otherwise: bb8]; // } // bb1: { // resume; // } // bb2: { // arm1 -// StorageLive(_8); -// _8 = _4; -// _1 = (const 1i32, move _8); -// StorageDead(_8); +// StorageLive(_9); +// _9 = _4; +// _1 = (const 1i32, move _9); +// StorageDead(_9); // goto -> bb13; // } // bb3: { // binding3(empty) and arm3 @@ -151,24 +153,26 @@ fn main() { // unreachable; // } // bb9: { // binding1 and guard -// StorageLive(_4); -// _4 = ((_2 as Some).0: i32); -// StorageLive(_7); -// _7 = const guard() -> [return: bb10, unwind: bb1]; +// StorageLive(_5); +// _5 = &((_2 as Some).0: i32); +// StorageLive(_8); +// _8 = const guard() -> [return: bb10, unwind: bb1]; // } // bb10: { // end of guard -// switchInt(move _7) -> [false: bb11, otherwise: bb2]; +// StorageLive(_4); +// _4 = ((_2 as Some).0: i32); +// switchInt(move _8) -> [false: bb11, otherwise: bb2]; // } // bb11: { // to pre_binding2 // falseEdges -> [real: bb6, imaginary: bb5]; // } // bb12: { // binding2 and arm2 -// StorageLive(_5); -// _5 = ((_2 as Some).0: i32); -// StorageLive(_9); -// _9 = _5; -// _1 = (const 2i32, move _9); -// StorageDead(_9); +// StorageLive(_6); +// _6 = ((_2 as Some).0: i32); +// StorageLive(_10); +// _10 = _6; +// _1 = (const 2i32, move _10); +// StorageDead(_10); // goto -> bb13; // } // bb13: { @@ -182,8 +186,8 @@ fn main() { // ... // _2 = std::option::Option::Some(const 1i32,); // _3 = discriminant(_2); -// _8 = discriminant(_2); -// switchInt(move _8) -> [1isize: bb4, otherwise: bb5]; +// _10 = discriminant(_2); +// switchInt(move _10) -> [1isize: bb4, otherwise: bb5]; // } // bb1: { // resume; @@ -213,41 +217,45 @@ fn main() { // unreachable; // } // bb9: { // binding1: Some(w) if guard() -// StorageLive(_4); -// _4 = ((_2 as Some).0: i32); -// StorageLive(_9); -// _9 = const guard() -> [return: bb10, unwind: bb1]; +// StorageLive(_5); +// _5 = &((_2 as Some).0: i32); +// StorageLive(_11); +// _11 = const guard() -> [return: bb10, unwind: bb1]; // } // bb10: { //end of guard -// switchInt(move _9) -> [false: bb11, otherwise: bb2]; +// StorageLive(_4); +// _4 = ((_2 as Some).0: i32); +// switchInt(move _11) -> [false: bb11, otherwise: bb2]; // } // bb11: { // to pre_binding2 // falseEdges -> [real: bb5, imaginary: bb5]; // } // bb12: { // binding2 & arm2 -// StorageLive(_5); -// _5 = _2; +// StorageLive(_6); +// _6 = _2; // _1 = const 2i32; // goto -> bb17; // } // bb13: { // binding3: Some(y) if guard2(y) -// StorageLive(_6); -// _6 = ((_2 as Some).0: i32); -// StorageLive(_11); -// StorageLive(_12); -// _12 = _6; -// _11 = const guard2(move _12) -> [return: bb14, unwind: bb1]; +// StorageLive(_8); +// _8 = &((_2 as Some).0: i32); +// StorageLive(_13); +// StorageLive(_14); +// _14 = (*_8); +// _13 = const guard2(move _14) -> [return: bb14, unwind: bb1]; // } // bb14: { // end of guard2 -// StorageDead(_12); -// switchInt(move _11) -> [false: bb15, otherwise: bb3]; +// StorageDead(_14); +// StorageLive(_7); +// _7 = ((_2 as Some).0: i32); +// switchInt(move _13) -> [false: bb15, otherwise: bb3]; // } // bb15: { // to pre_binding4 // falseEdges -> [real: bb7, imaginary: bb7]; // } // bb16: { // binding4 & arm4 -// StorageLive(_7); -// _7 = _2; +// StorageLive(_9); +// _9 = _2; // _1 = const 4i32; // goto -> bb17; // } diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index d1dac370c9ed9..01d9f52424da8 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2735,10 +2735,12 @@ impl<'test> TestCx<'test> { panic!( "Did not find expected line, error: {}\n\ Expected Line: {:?}\n\ + Test Name: {}\n\ Expected:\n{}\n\ Actual:\n{}", extra_msg, expected_line, + test_name, expected_content, normalize_all );