Skip to content

Commit 6c528ce

Browse files
nikomatsakisnikomatsakis
committed
gather activation locations for 2-phase borrows in 1 pass
1 parent 517e7fd commit 6c528ce

File tree

1 file changed

+139
-98
lines changed

1 file changed

+139
-98
lines changed

‎src/librustc_mir/dataflow/impls/borrows.rs

Lines changed: 139 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use rustc::hir;
1313
use rustc::hir::def_id::DefId;
1414
use rustc::middle::region;
1515
use rustc::mir::{self, Location, Place, Mir};
16+
use rustc::mir::traversal;
1617
use rustc::mir::visit::{PlaceContext, Visitor};
1718
use rustc::ty::{self, Region, TyCtxt};
1819
use rustc::ty::RegionKind;
@@ -85,6 +86,9 @@ pub struct BorrowData<'tcx> {
8586
/// Location where the borrow reservation starts.
8687
/// In many cases, this will be equal to the activation location but not always.
8788
pub(crate) reserve_location: Location,
89+
/// Location where the borrow is activated. None if this is not a
90+
/// 2-phase borrow.
91+
pub(crate) activation_location: Option<Location>,
8892
/// What kind of borrow this is
8993
pub(crate) kind: mir::BorrowKind,
9094
/// The region for which this borrow is live
@@ -143,9 +147,26 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
143147
region_map: FxHashMap(),
144148
local_map: FxHashMap(),
145149
region_span_map: FxHashMap(),
146-
nonlexical_regioncx: nonlexical_regioncx.clone()
150+
nonlexical_regioncx: nonlexical_regioncx.clone(),
151+
pending_activations: FxHashMap(),
147152
};
148-
visitor.visit_mir(mir);
153+
for (block, block_data) in traversal::preorder(mir) {
154+
visitor.visit_basic_block_data(block, block_data);
155+
}
156+
157+
// Double check: We should have found an activation for every pending
158+
// activation.
159+
assert_eq!(
160+
visitor
161+
.pending_activations
162+
.iter()
163+
.find(|&(_local, &borrow_index)| {
164+
visitor.idx_vec[borrow_index].activation_location.is_none()
165+
}),
166+
None,
167+
"never found an activation for this borrow!",
168+
);
169+
149170
return Borrows { tcx: tcx,
150171
mir: mir,
151172
borrows: visitor.idx_vec,
@@ -168,6 +189,16 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
168189
local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,
169190
region_span_map: FxHashMap<RegionKind, Span>,
170191
nonlexical_regioncx: Option<Rc<RegionInferenceContext<'tcx>>>,
192+
193+
/// When we encounter a 2-phase borrow statement, it will always
194+
/// be assigning into a temporary TEMP:
195+
///
196+
/// TEMP = &foo
197+
///
198+
/// We add TEMP into this map with `b`, where `b` is the index of
199+
/// the borrow. When we find a later use of this activation, we
200+
/// remove from the map (and add to the "tombstone" set below).
201+
pending_activations: FxHashMap<mir::Local, BorrowIndex>,
171202
}
172203

173204
impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
@@ -187,20 +218,25 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
187218
if let mir::Rvalue::Ref(region, kind, ref borrowed_place) = *rvalue {
188219
if is_unsafe_place(self.tcx, self.mir, borrowed_place) { return; }
189220

190-
let activate_location = self.compute_activation_location(location,
191-
&assigned_place,
192-
region,
193-
kind);
194221
let borrow = BorrowData {
195-
kind, region,
222+
kind,
223+
region,
196224
reserve_location: location,
225+
activation_location: None,
197226
borrowed_place: borrowed_place.clone(),
198227
assigned_place: assigned_place.clone(),
199228
};
200229
let idx = self.idx_vec.push(borrow);
201230
self.location_map.insert(location, idx);
202231

203-
insert(&mut self.activation_map, &activate_location, idx);
232+
self.insert_as_pending_if_two_phase(
233+
location,
234+
&assigned_place,
235+
region,
236+
kind,
237+
idx,
238+
);
239+
204240
insert(&mut self.region_map, &region, idx);
205241
if let Some(local) = root_local(borrowed_place) {
206242
insert(&mut self.local_map, &local, idx);
@@ -220,25 +256,69 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
220256
}
221257
}
222258

259+
fn visit_place(
260+
&mut self,
261+
place: &mir::Place<'tcx>,
262+
context: PlaceContext<'tcx>,
263+
location: Location,
264+
) {
265+
self.super_place(place, context, location);
266+
267+
// We found a use of some temporary TEMP...
268+
if let Place::Local(temp) = place {
269+
// ... check whether we (earlier) saw a 2-phase borrow like
270+
//
271+
// TMP = &mut place
272+
match self.pending_activations.get(temp) {
273+
Some(&borrow_index) => {
274+
let borrow_data = &mut self.idx_vec[borrow_index];
275+
276+
// Watch out: the use of TMP in the borrow
277+
// itself doesn't count as an
278+
// activation. =)
279+
if borrow_data.reserve_location == location
280+
&& context == PlaceContext::Store
281+
{
282+
return;
283+
}
284+
285+
if let Some(other_activation) = borrow_data.activation_location {
286+
span_bug!(
287+
self.mir.source_info(location).span,
288+
"found two activations for 2-phase borrow temporary {:?}: \
289+
{:?} and {:?}",
290+
temp,
291+
location,
292+
other_activation,
293+
);
294+
}
295+
296+
// Otherwise, this is the unique later use
297+
// that we expect.
298+
borrow_data.activation_location = Some(location);
299+
self.activation_map
300+
.entry(location)
301+
.or_insert(FxHashSet())
302+
.insert(borrow_index);
303+
}
304+
305+
None => {}
306+
}
307+
}
308+
}
309+
223310
fn visit_rvalue(&mut self,
224311
rvalue: &mir::Rvalue<'tcx>,
225312
location: mir::Location) {
226313
if let mir::Rvalue::Ref(region, kind, ref place) = *rvalue {
227314
// double-check that we already registered a BorrowData for this
228315

229-
let mut found_it = false;
230-
for idx in &self.region_map[region] {
231-
let bd = &self.idx_vec[*idx];
232-
if bd.reserve_location == location &&
233-
bd.kind == kind &&
234-
bd.region == region &&
235-
bd.borrowed_place == *place
236-
{
237-
found_it = true;
238-
break;
239-
}
240-
}
241-
assert!(found_it, "Ref {:?} at {:?} missing BorrowData", rvalue, location);
316+
let borrow_index = self.location_map[&location];
317+
let borrow_data = &self.idx_vec[borrow_index];
318+
assert_eq!(borrow_data.reserve_location, location);
319+
assert_eq!(borrow_data.kind, kind);
320+
assert_eq!(borrow_data.region, region);
321+
assert_eq!(borrow_data.borrowed_place, *place);
242322
}
243323

244324
return self.super_rvalue(rvalue, location);
@@ -380,87 +460,48 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
380460
false
381461
}
382462

383-
/// Computes the activation location of a borrow.
384-
/// The general idea is to start at the beginning of the region and perform a DFS
385-
/// until we exit the region, either via an explicit EndRegion or because NLL tells
386-
/// us so. If we find more than one valid activation point, we currently panic the
387-
/// compiler since two-phase borrows are only currently supported for compiler-
388-
/// generated code. More precisely, we only allow two-phase borrows for:
389-
/// - Function calls (fn some_func(&mut self, ....))
390-
/// - *Assign operators (a += b -> fn add_assign(&mut self, other: Self))
391-
/// See
392-
/// - https://github.com/rust-lang/rust/issues/48431
393-
/// for detailed design notes.
394-
/// See the FIXME in the body of the function for notes on extending support to more
395-
/// general two-phased borrows.
396-
fn compute_activation_location(&self,
397-
start_location: Location,
398-
assigned_place: &mir::Place<'tcx>,
399-
region: Region<'tcx>,
400-
kind: mir::BorrowKind) -> Location {
401-
debug!("Borrows::compute_activation_location({:?}, {:?}, {:?})",
402-
start_location,
403-
assigned_place,
404-
region);
463+
/// If this is a two-phase borrow, then we will record it
464+
/// as "pending" until we find the activating use.
465+
fn insert_as_pending_if_two_phase(
466+
&mut self,
467+
start_location: Location,
468+
assigned_place: &mir::Place<'tcx>,
469+
region: Region<'tcx>,
470+
kind: mir::BorrowKind,
471+
borrow_index: BorrowIndex,
472+
) {
473+
debug!(
474+
"Borrows::insert_as_pending_if_two_phase({:?}, {:?}, {:?}, {:?})",
475+
start_location, assigned_place, region, borrow_index,
476+
);
477+
405478
if !self.allow_two_phase_borrow(kind) {
406479
debug!(" -> {:?}", start_location);
407-
return start_location;
408-
}
409-
410-
// Perform the DFS.
411-
// `stack` is the stack of locations still under consideration
412-
// `visited` is the set of points we have already visited
413-
// `found_use` is an Option that becomes Some when we find a use
414-
let mut stack = vec![start_location];
415-
let mut visited = FxHashSet();
416-
let mut found_use = None;
417-
while let Some(curr_loc) = stack.pop() {
418-
let block_data = &self.mir.basic_blocks()
419-
.get(curr_loc.block)
420-
.unwrap_or_else(|| {
421-
panic!("could not find block at location {:?}", curr_loc);
422-
});
423-
424-
if self.region_terminated_after(region, curr_loc) {
425-
// No need to process this statement.
426-
// It's either an EndRegion (and thus couldn't use assigned_place) or not
427-
// contained in the NLL region and thus a use would be invalid
428-
continue;
429-
}
430-
431-
if !visited.insert(curr_loc) {
432-
debug!(" Already visited {:?}", curr_loc);
433-
continue;
434-
}
435-
436-
if self.location_contains_use(curr_loc, assigned_place) {
437-
// FIXME: Handle this case a little more gracefully. Perhaps collect
438-
// all uses in a vector, and find the point in the CFG that dominates
439-
// all of them?
440-
// Right now this is sufficient though since there should only be exactly
441-
// one borrow-activating use of the borrow.
442-
assert!(found_use.is_none(), "Found secondary use of place");
443-
found_use = Some(curr_loc);
444-
}
445-
446-
// Push the points we should consider next.
447-
if curr_loc.statement_index < block_data.statements.len() {
448-
stack.push(curr_loc.successor_within_block());
449-
} else {
450-
stack.extend(block_data.terminator().successors().iter().map(
451-
|&basic_block| {
452-
Location {
453-
statement_index: 0,
454-
block: basic_block
455-
}
456-
}
457-
))
458-
}
480+
return;
459481
}
460482

461-
let found_use = found_use.expect("Did not find use of two-phase place");
462-
debug!(" -> {:?}", found_use);
463-
found_use
483+
// When we encounter a 2-phase borrow statement, it will always
484+
// be assigning into a temporary TEMP:
485+
//
486+
// TEMP = &foo
487+
//
488+
// so extract `temp`.
489+
let temp = if let &mir::Place::Local(temp) = assigned_place {
490+
temp
491+
} else {
492+
span_bug!(
493+
self.mir.source_info(start_location).span,
494+
"expected 2-phase borrow to assign to a local, not `{:?}`",
495+
assigned_place,
496+
);
497+
};
498+
499+
// Insert `temp` into the list of pending activations. From
500+
// now on, we'll be on the lookout for a use of it. Note that
501+
// we are guaranteed that this use will come after the
502+
// assignment.
503+
let old_value = self.pending_activations.insert(temp, borrow_index);
504+
assert!(old_value.is_none());
464505
}
465506
}
466507
}

0 commit comments

Comments
 (0)