Skip to content

Commit

Permalink
Kill conflicting borrows of places with projections.
Browse files Browse the repository at this point in the history
Resolves rust-lang#62007.

Due to a bug, the previous version of this check did not actually kill
any conflicting borrows unless the borrowed place had no projections.
Specifically, `entry_set` will always be empty when `statement_effect`
is called. It does not contain the set of borrows which are live at this
point in the program.
  • Loading branch information
ecstatic-morse committed Jun 20, 2019
1 parent f693d33 commit f045008
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 30 deletions.
55 changes: 25 additions & 30 deletions src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,43 +193,38 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
place: &Place<'tcx>
) {
debug!("kill_borrows_on_place: place={:?}", place);
// Handle the `Place::Local(..)` case first and exit early.
if let Place::Base(PlaceBase::Local(local)) = place {
if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) {
debug!("kill_borrows_on_place: borrow_indices={:?}", borrow_indices);
sets.kill_all(borrow_indices);

if let Some(local) = place.base_local() {
let other_borrows_of_local = self
.borrow_set
.local_map
.get(&local)
.into_iter()
.flat_map(|bs| bs.into_iter());

// If the borrowed place is a local with no projections, all other borrows of this
// local must conflict. This is purely an optimization so we don't have to call
// `places_conflict` for every borrow.
if let Place::Base(PlaceBase::Local(_)) = place {
sets.kill_all(other_borrows_of_local);
return;
}
}

// Otherwise, look at all borrows that are live and if they conflict with the assignment
// into our place then we can kill them.
let mut borrows = sets.on_entry.clone();
let _ = borrows.union(sets.gen_set);
for borrow_index in borrows.iter() {
let borrow_data = &self.borrows()[borrow_index];
debug!(
"kill_borrows_on_place: borrow_index={:?} borrow_data={:?}",
borrow_index, borrow_data,
);

// By passing `PlaceConflictBias::NoOverlap`, we conservatively assume that any given
// pair of array indices are unequal, so that when `places_conflict` returns true, we
// will be assured that two places being compared definitely denotes the same sets of
// locations.
if places_conflict::places_conflict(
self.tcx,
self.body,
&borrow_data.borrowed_place,
place,
places_conflict::PlaceConflictBias::NoOverlap,
) {
debug!(
"kill_borrows_on_place: (kill) borrow_index={:?} borrow_data={:?}",
borrow_index, borrow_data,
);
sets.kill(borrow_index);
}
let definitely_conflicting_borrows = other_borrows_of_local
.filter(|&&i| {
places_conflict::places_conflict(
self.tcx,
self.body,
&self.borrow_set.borrows[i].borrowed_place,
place,
places_conflict::PlaceConflictBias::NoOverlap)
});

sets.kill_all(definitely_conflicting_borrows);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// run-pass
#![allow(dead_code)]

struct List<T> {
value: T,
next: Option<Box<List<T>>>,
}

fn to_refs<T>(mut list: (&mut List<T>,)) -> Vec<&mut T> {
let mut result = vec![];
loop {
result.push(&mut (list.0).value);
if let Some(n) = (list.0).next.as_mut() {
list.0 = n;
} else {
return result;
}
}
}

fn main() {}

0 comments on commit f045008

Please sign in to comment.