Skip to content

Commit

Permalink
Complete re-implementation of 2-phase borrows
Browse files Browse the repository at this point in the history
See rust-lang#48431 for discussion as to why this was necessary and what we hoped to
accomplish. A brief summary:
   - the first implementation of 2-phase borrows was hard to limit in the way we
   wanted. That is, it was too good at accepting all 2-phase borrows rather than
   just autorefs =)
   - Numerous diagnostic regressions were introduced by 2-phase borrow support
   which were difficult to fix
  • Loading branch information
sapphire-arches committed Mar 9, 2018
1 parent 047bec6 commit 47d75af
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 33 deletions.
64 changes: 32 additions & 32 deletions src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,10 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {

// Perform the DFS.
// `stack` is the stack of locations still under consideration
// `visited` is the set of points we have already visited
// `found_use` is an Option that becomes Some when we find a use
let mut stack = vec![start_location];
let mut visited = FxHashSet();
let mut found_use = None;
while let Some(curr_loc) = stack.pop() {
let block_data = &self.mir.basic_blocks()
Expand All @@ -467,6 +469,11 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
continue;
}

if !visited.insert(curr_loc) {
debug!(" Already visited {:?}", curr_loc);
continue;
}

if self.location_contains_use(curr_loc, assigned_place) {
// TODO: Handle this case a little more gracefully. Perhaps collect
// all uses in a vector, and find the point in the CFG that dominates
Expand Down Expand Up @@ -529,19 +536,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
fn kill_loans_out_of_scope_at_location(&self,
sets: &mut BlockSets<ReserveOrActivateIndex>,
location: Location) {
/*
XXX: bob_twinkles reintroduce this
let block_data = &self.mir[location.block];
if location.statement_index != block_data.statements.len() {
let statement = &block_data.statements[location.statement_index];
if let mir::StatementKind::EndRegion(region_scope) = statement.kind {
for &borrow_index in &self.region_map[&ReScope(region_scope)] {
sets.kill(&ReserveOrActivateIndex::reserved(borrow_index));
sets.kill(&ReserveOrActivateIndex::active(borrow_index));
}
}
}
*/
if let Some(ref regioncx) = self.nonlexical_regioncx {
// NOTE: The state associated with a given `location`
// reflects the dataflow on entry to the statement. If it
Expand Down Expand Up @@ -575,6 +569,20 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
.map(|b| ReserveOrActivateIndex::active(*b)));
}
}

/// Performs the activations for a given location
fn perform_activations_at_location(&self,
sets: &mut BlockSets<ReserveOrActivateIndex>,
location: Location) {
// Handle activations
match self.activation_map.get(&location) {
Some(&activated) => {
debug!("activating borrow {:?}", activated);
sets.gen(&ReserveOrActivateIndex::active(activated))
}
None => {}
}
}
}

impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -605,13 +613,8 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
panic!("could not find statement at location {:?}");
});

// Handle activations
match self.activation_map.get(&location) {
Some(&activated) => {
sets.gen(&ReserveOrActivateIndex::active(activated))
}
None => {}
}
self.perform_activations_at_location(sets, location);
self.kill_loans_out_of_scope_at_location(sets, location);

match stmt.kind {
// EndRegion kills any borrows (reservations and active borrows both)
Expand Down Expand Up @@ -643,15 +646,17 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {

if let mir::Rvalue::Ref(region, _, ref place) = *rhs {
if is_unsafe_place(self.tcx, self.mir, place) { return; }
let index = self.location_map.get(&location).unwrap_or_else(|| {
panic!("could not find BorrowIndex for location {:?}", location);
});

if let RegionKind::ReEmpty = region {
// If the borrowed value is dead, the region for it
// can be empty. Don't track the borrow in that case.
// 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.
sets.kill(&ReserveOrActivateIndex::active(*index));
return
}

let index = self.location_map.get(&location).unwrap_or_else(|| {
panic!("could not find BorrowIndex for location {:?}", location);
});
assert!(self.region_map.get(region).unwrap_or_else(|| {
panic!("could not find BorrowIndexs for region {:?}", region);
}).contains(&index));
Expand Down Expand Up @@ -714,14 +719,9 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
});

let term = block.terminator();
self.perform_activations_at_location(sets, location);
self.kill_loans_out_of_scope_at_location(sets, location);

// Handle activations
match self.activation_map.get(&location) {
Some(&activated) => {
sets.gen(&ReserveOrActivateIndex::active(activated))
}
None => {}
}

match term.kind {
mir::TerminatorKind::Resume |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ fn not_ok() {
*y += 1;
//[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
//[nll_beyond]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
//[nll_target]~^^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
read(z);
}

Expand Down

0 comments on commit 47d75af

Please sign in to comment.