提交 3bc5073d 编写于 作者: F Felix S. Klock II

Expand two-phase-borrows so that a case like this still compiles:

```rust
fn main() {
    fn reuse<X>(_: &mut X) {}
    let mut t = 2f64;
    match t {
        ref mut _b if { false } => { reuse(_b); }
        _ => {}
    }
}
```

Note: The way this is currently written is confusing; when `autoref`
is off, then the arm body bindings (introduced by
`bind_matched_candidate_for_arm_body`) are *also* used for the guard.
(Any attempt to fix this needs to still ensure that the bindings used
by the guard are introduced before the guard is evaluated.)

(Once we turn NLL on by default, we can presumably simplify all of
that.)
上级 638acd30
......@@ -53,6 +53,17 @@ fn index(&self, index: BorrowIndex) -> &BorrowData<'tcx> {
}
}
/// Every two-phase borrow has *exactly one* use (or else it is not a
/// proper two-phase borrow under our current definition. However, not
/// all uses are actually ones that activate the reservation.. In
/// particular, a shared borrow of a `&mut` does not activate the
/// reservation.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
crate enum TwoPhaseUse {
MutActivate,
SharedUse,
}
#[derive(Debug)]
crate struct BorrowData<'tcx> {
/// Location where the borrow reservation starts.
......@@ -60,7 +71,7 @@ fn index(&self, index: BorrowIndex) -> &BorrowData<'tcx> {
crate reserve_location: Location,
/// Location where the borrow is activated. None if this is not a
/// 2-phase borrow.
crate activation_location: Option<Location>,
crate activation_location: Option<(TwoPhaseUse, Location)>,
/// What kind of borrow this is
crate kind: mir::BorrowKind,
/// The region for which this borrow is live
......@@ -215,9 +226,8 @@ fn visit_place(
Some(&borrow_index) => {
let borrow_data = &mut self.idx_vec[borrow_index];
// Watch out: the use of TMP in the borrow
// itself doesn't count as an
// activation. =)
// Watch out: the use of TMP in the borrow itself
// doesn't count as an activation. =)
if borrow_data.reserve_location == location && context == PlaceContext::Store {
return;
}
......@@ -225,7 +235,7 @@ fn visit_place(
if let Some(other_activation) = borrow_data.activation_location {
span_bug!(
self.mir.source_info(location).span,
"found two activations for 2-phase borrow temporary {:?}: \
"found two uses for 2-phase borrow temporary {:?}: \
{:?} and {:?}",
temp,
location,
......@@ -235,11 +245,25 @@ fn visit_place(
// Otherwise, this is the unique later use
// that we expect.
borrow_data.activation_location = Some(location);
self.activation_map
.entry(location)
.or_insert(Vec::new())
.push(borrow_index);
let two_phase_use;
match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => {
two_phase_use = TwoPhaseUse::SharedUse;
}
_ => {
two_phase_use = TwoPhaseUse::MutActivate;
self.activation_map
.entry(location)
.or_insert(Vec::new())
.push(borrow_index);
}
}
borrow_data.activation_location = Some((two_phase_use, location));
}
None => {}
......
......@@ -12,7 +12,7 @@
/// allowed to be split into separate Reservation and
/// Activation phases.
use borrow_check::ArtificialField;
use borrow_check::borrow_set::{BorrowSet, BorrowData};
use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseUse};
use borrow_check::{Context, Overlap};
use borrow_check::{ShallowOrDeep, Deep, Shallow};
use dataflow::indexes::BorrowIndex;
......@@ -431,10 +431,13 @@ pub(super) fn is_active<'tcx>(
) -> bool {
debug!("is_active(borrow_data={:?}, location={:?})", borrow_data, location);
// If this is not a 2-phase borrow, it is always active.
let activation_location = match borrow_data.activation_location {
Some(v) => v,
// If this is not a 2-phase borrow, it is always active.
None => return true,
// And if the unique 2-phase use is not an activation, then it is *never* active.
Some((TwoPhaseUse::SharedUse, _)) => return false,
// Otherwise, we derive info from the activation point `v`:
Some((TwoPhaseUse::MutActivate, v)) => v,
};
// Otherwise, it is active for every location *except* in between
......
......@@ -11,7 +11,7 @@
//! See docs in build/expr/mod.rs
use build::{BlockAnd, BlockAndExtension, Builder};
use build::ForGuard::{OutsideGuard, WithinGuard};
use build::ForGuard::{OutsideGuard, RefWithinGuard, ValWithinGuard};
use build::expr::category::Category;
use hair::*;
use rustc::mir::*;
......@@ -88,10 +88,11 @@ fn expr_as_place(&mut self,
}
ExprKind::VarRef { id } => {
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() {
let index = this.var_local_id(id, RefWithinGuard);
Place::Local(index).deref()
} else {
let index = this.var_local_id(id, ValWithinGuard);
Place::Local(index)
}
} else {
......
......@@ -15,7 +15,7 @@
use build::{BlockAnd, BlockAndExtension, Builder};
use build::{GuardFrame, GuardFrameLocal, LocalsForNode};
use build::ForGuard::{self, OutsideGuard, WithinGuard};
use build::ForGuard::{self, OutsideGuard, RefWithinGuard, ValWithinGuard};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::bitvec::BitVector;
use rustc::ty::{self, Ty};
......@@ -88,12 +88,8 @@ pub fn match_expr(&mut self,
let mut arm_blocks = ArmBlocks {
blocks: arms.iter()
.map(|_| {
let arm_block = self.cfg.start_new_block();
arm_block
})
.collect(),
.map(|_| self.cfg.start_new_block())
.collect(),
};
// Get the arm bodies and their scopes, while declaring bindings.
......@@ -110,9 +106,7 @@ pub fn match_expr(&mut self,
// create binding start block for link them by false edges
let candidate_count = arms.iter().fold(0, |ac, c| ac + c.patterns.len());
let pre_binding_blocks: Vec<_> = (0..candidate_count + 1)
.map(|_| self.cfg.start_new_block())
.collect();
.map(|_| self.cfg.start_new_block()).collect();
// assemble a list of candidates: there is one candidate per
// pattern, which means there may be more than one candidate
......@@ -315,7 +309,7 @@ pub fn place_into_pattern(&mut self,
}
// now apply the bindings, which will also declare the variables
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, false);
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings);
block.unit()
}
......@@ -956,22 +950,6 @@ fn bind_and_guard_matched_candidate<'pat>(&mut self,
// (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 {
......@@ -985,7 +963,7 @@ fn bind_and_guard_matched_candidate<'pat>(&mut self,
debug!("Entering guard building context: {:?}", guard_frame);
self.guard_context.push(guard_frame);
} else {
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, false);
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings);
}
// the block to branch to if the guard fails; if there is no
......@@ -999,14 +977,47 @@ fn bind_and_guard_matched_candidate<'pat>(&mut self,
}
let false_edge_block = self.cfg.start_new_block();
// We want to ensure that the matched candidates are bound
// after we have confirmed this candidate *and* any
// associated guard; Binding them on `block` is too soon,
// because that would be before we've checked the result
// from the guard.
//
// But binding them on `arm_block` is *too late*, because
// then all of the candidates for a single arm would be
// bound in the same place, that would cause a case like:
//
// ```rust
// match (30, 2) {
// (mut x, 1) | (2, mut x) if { true } => { ... }
// ... // ^^^^^^^ (this is `arm_block`)
// }
// ```
//
// would yield a `arm_block` something like:
//
// ```
// StorageLive(_4); // _4 is `x`
// _4 = &mut (_1.0: i32); // this is handling `(mut x, 1)` case
// _4 = &mut (_1.1: i32); // this is handling `(2, mut x)` case
// ```
//
// and that is clearly not correct.
let post_guard_block = self.cfg.start_new_block();
self.cfg.terminate(block, source_info,
TerminatorKind::if_(self.hir.tcx(), cond, arm_block,
false_edge_block));
TerminatorKind::if_(self.hir.tcx(), cond, post_guard_block,
false_edge_block));
let otherwise = self.cfg.start_new_block();
if autoref {
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, true);
self.bind_matched_candidate_for_arm_body(post_guard_block, &candidate.bindings);
}
self.cfg.terminate(post_guard_block, source_info,
TerminatorKind::Goto { target: arm_block });
let otherwise = self.cfg.start_new_block();
self.cfg.terminate(false_edge_block, source_info,
TerminatorKind::FalseEdges {
real_target: otherwise,
......@@ -1015,13 +1026,18 @@ fn bind_and_guard_matched_candidate<'pat>(&mut self,
});
Some(otherwise)
} else {
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings, false);
// (Here, it is not too early to bind the matched
// candidate on `block`, because there is no guard result
// that we have to inspect before we bind them.)
self.bind_matched_candidate_for_arm_body(block, &candidate.bindings);
self.cfg.terminate(block, candidate_source_info,
TerminatorKind::Goto { target: arm_block });
None
}
}
// Only called when all_pat_vars_are_implicit_refs_within_guards,
// and thus all code/comments assume we are in that context.
fn bind_matched_candidate_for_guard(&mut self,
block: BasicBlock,
bindings: &[Binding<'tcx>]) {
......@@ -1034,61 +1050,54 @@ fn bind_matched_candidate_for_guard(&mut self,
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);
// For each pattern ident P of type T, `ref_for_guard` is
// a reference R: &T pointing to the location matched by
// the pattern, and every occurrence of P within a guard
// denotes *R.
let ref_for_guard = self.storage_live_binding(
block, binding.var_id, binding.span, RefWithinGuard);
// 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);
self.schedule_drop_for_binding(binding.var_id, binding.span, RefWithinGuard);
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);
self.cfg.push_assign(block, source_info, &ref_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
// 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.
// T` (i.e. a "borrow of a borrow") that is
// implicitly dereferenced.
//
// So:
// To borrow a borrow, we need that inner borrow
// to point to. So, create a temp for the inner
// borrow, and then take a reference to it.
//
// 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.
// Note: the temp created here is *not* the one
// used by the arm body itself. This eases
// observing two-phase borrow restrictions.
let val_for_guard = self.storage_live_binding(
block, binding.var_id, binding.span, ValWithinGuard);
self.schedule_drop_for_binding(binding.var_id, binding.span, ValWithinGuard);
// rust-lang/rust#27282: We reuse the two-phase
// borrow infrastructure so that the mutable
// borrow (whose mutabilty is *unusable* within
// the guard) does not conflict with the implicit
// borrow of the whole match input. See additional
// discussion on rust-lang/rust#49870.
let borrow_kind = match borrow_kind {
BorrowKind::Shared | BorrowKind::Unique => borrow_kind,
BorrowKind::Mut { .. } => BorrowKind::Mut { allow_two_phase_borrow: true },
};
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);
self.cfg.push_assign(block, source_info, &val_for_guard, rvalue);
let rvalue = Rvalue::Ref(region, BorrowKind::Shared, val_for_guard);
self.cfg.push_assign(block, source_info, &ref_for_guard, rvalue);
}
}
}
......@@ -1096,19 +1105,11 @@ fn bind_matched_candidate_for_guard(&mut self,
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);
bindings: &[Binding<'tcx>]) {
debug!("bind_matched_candidate_for_arm_body(block={:?}, bindings={:?}", block, bindings);
// 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,
OutsideGuard);
......@@ -1145,7 +1146,7 @@ fn declare_binding(&mut self,
var_id, name, var_ty, source_info, syntactic_scope);
let tcx = self.hir.tcx();
let for_arm_body = self.local_decls.push(LocalDecl::<'tcx> {
let local = LocalDecl::<'tcx> {
mutability,
ty: var_ty.clone(),
name: Some(name),
......@@ -1153,9 +1154,11 @@ fn declare_binding(&mut self,
syntactic_scope,
internal: false,
is_user_variable: true,
});
};
let for_arm_body = self.local_decls.push(local.clone());
let locals = if has_guard.0 && tcx.all_pat_vars_are_implicit_refs_within_guards() {
let for_guard = self.local_decls.push(LocalDecl::<'tcx> {
let val_for_guard = self.local_decls.push(local);
let ref_for_guard = self.local_decls.push(LocalDecl::<'tcx> {
mutability,
ty: tcx.mk_imm_ref(tcx.types.re_empty, var_ty),
name: Some(name),
......@@ -1164,7 +1167,7 @@ fn declare_binding(&mut self,
internal: false,
is_user_variable: true,
});
LocalsForNode::Two { for_guard, for_arm_body }
LocalsForNode::Three { val_for_guard, ref_for_guard, for_arm_body }
} else {
LocalsForNode::One(for_arm_body)
};
......
......@@ -293,7 +293,7 @@ fn var_local_id(&self, id: ast::NodeId, for_guard: ForGuard) -> Local {
#[derive(Debug)]
enum LocalsForNode {
One(Local),
Two { for_guard: Local, for_arm_body: Local },
Three { val_for_guard: Local, ref_for_guard: Local, for_arm_body: Local },
}
#[derive(Debug)]
......@@ -325,12 +325,15 @@ struct GuardFrame {
locals: Vec<GuardFrameLocal>,
}
/// 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.
/// ForGuard indicates whether we are talking about:
/// 1. the temp for a local binding used solely within guard expressions,
/// 2. the temp that holds reference to (1.), which is actually what the
/// guard expressions see, or
/// 3. the temp for use outside of guard expressions.
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
enum ForGuard {
WithinGuard,
ValWithinGuard,
RefWithinGuard,
OutsideGuard,
}
......@@ -338,11 +341,13 @@ 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) =>
(&LocalsForNode::Three { val_for_guard: local_id, .. }, ForGuard::ValWithinGuard) |
(&LocalsForNode::Three { ref_for_guard: local_id, .. }, ForGuard::RefWithinGuard) |
(&LocalsForNode::Three { for_arm_body: local_id, .. }, ForGuard::OutsideGuard) =>
local_id,
(&LocalsForNode::One(_), ForGuard::WithinGuard) =>
(&LocalsForNode::One(_), ForGuard::ValWithinGuard) |
(&LocalsForNode::One(_), ForGuard::RefWithinGuard) =>
bug!("anything with one local should never be within a guard."),
}
}
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册