Skip to content

Commit f96777c

Browse files
committed
After discussion with ariel, replacing a guard within kill_loans_out_of_scope_at_location.
Instead we are "just" careful to invoke it (which sets up a bunch of kill bits) before we go into the code that sets up the gen bits. That way, when the gen bits are set up, they will override any previously set kill-bits for those reservations or activations.
1 parent 3c7d9ff commit f96777c

File tree

1 file changed

+33
-33
lines changed

1 file changed

+33
-33
lines changed

src/librustc_mir/dataflow/impls/borrows.rs

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -292,32 +292,21 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
292292
location: Location,
293293
is_activations: bool) {
294294
if let Some(ref regioncx) = self.nonlexical_regioncx {
295+
// NOTE: The state associated with a given `location`
296+
// reflects the dataflow on entry to the statement. If it
297+
// does not contain `borrow_region`, then then that means
298+
// that the statement at `location` kills the borrow.
299+
//
300+
// We are careful always to call this function *before* we
301+
// set up the gen-bits for the statement or
302+
// termanator. That way, if the effect of the statement or
303+
// terminator *does* introduce a new loan of the same
304+
// region, then setting that gen-bit will override any
305+
// potential kill introduced here.
295306
for (borrow_index, borrow_data) in self.borrows.iter_enumerated() {
296307
let borrow_region = borrow_data.region.to_region_vid();
297308
if !regioncx.region_contains_point(borrow_region, location) {
298-
// The region checker really considers the borrow
299-
// to start at the point **after** the location of
300-
// the borrow, but the borrow checker puts the gen
301-
// directly **on** the location of the
302-
// borrow. This results in a gen/kill both being
303-
// generated for same point if we are not
304-
// careful. Probably we should change the point of
305-
// the gen, but for now we hackily account for the
306-
// mismatch here by not generating a kill for the
307-
// location on the borrow itself.
308-
if location != borrow_data.location {
309-
sets.kill(&ReserveOrActivateIndex::reserved(borrow_index));
310-
}
311-
312-
// FIXME: the logic used to justify the above
313-
// "accounting for mismatch" does not generalize
314-
// to activations, so we set the kill-bits without
315-
// that same location check here.
316-
//
317-
// But... can we get into a situation where the
318-
// gen/kill bits are both sets in this case, in
319-
// which case we *do* need an analogous guard of
320-
// some kind?
309+
sets.kill(&ReserveOrActivateIndex::reserved(borrow_index));
321310
if is_activations {
322311
sets.kill(&ReserveOrActivateIndex::active(borrow_index));
323312
}
@@ -340,14 +329,18 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
340329
panic!("could not find statement at location {:?}");
341330
});
342331

332+
// Do kills introduced by NLL before setting up any potential
333+
// gens. (See NOTE in kill_loans_out_of_scope_at_location.)
334+
self.kill_loans_out_of_scope_at_location(sets, location, is_activations);
335+
343336
if is_activations {
344-
// INVARIANT: At this point, `sets.on_entry` should
345-
// correctly reflect the reservations as we enter the
346-
// statement (because accumulates_intrablock_state is
347-
// overridden)
337+
// INVARIANT: `sets.on_entry` accurately captures
338+
// reservations on entry to statement (b/c
339+
// accumulates_intrablock_state is overridden for
340+
// ActiveBorrows).
348341
//
349-
// Now compute effect of the statement on the activations
350-
// themselves in the ActiveBorrows state.
342+
// Now compute the activations generated by uses within
343+
// the statement based on that reservation state.
351344
let mut find = FindPlaceUses { sets, assigned_map: &self.assigned_map };
352345
find.visit_statement(location.block, stmt, location);
353346
}
@@ -414,8 +407,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
414407
mir::StatementKind::Nop => {}
415408

416409
}
417-
418-
self.kill_loans_out_of_scope_at_location(sets, location, is_activations);
419410
}
420411

421412
/// Models terminator effect in Reservations and ActiveBorrows
@@ -429,9 +420,19 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
429420
panic!("could not find block at location {:?}", location);
430421
});
431422

423+
// Do kills introduced by NLL before setting up any potential
424+
// gens. (See NOTE in kill_loans_out_of_scope_at_location.)
425+
self.kill_loans_out_of_scope_at_location(sets, location, is_activations);
426+
432427
let term = block.terminator();
433428
if is_activations {
434-
// Any uses of reserved Places in the statement are now activated.
429+
// INVARIANT: `sets.on_entry` accurately captures
430+
// reservations on entry to terminator (b/c
431+
// accumulates_intrablock_state is overridden for
432+
// ActiveBorrows).
433+
//
434+
// Now compute effect of the terminator on the activations
435+
// themselves in the ActiveBorrows state.
435436
let mut find = FindPlaceUses { sets, assigned_map: &self.assigned_map };
436437
find.visit_terminator(location.block, term, location);
437438
}
@@ -474,7 +475,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
474475
mir::TerminatorKind::FalseEdges {..} |
475476
mir::TerminatorKind::Unreachable => {}
476477
}
477-
self.kill_loans_out_of_scope_at_location(sets, location, is_activations);
478478
}
479479
}
480480

0 commit comments

Comments
 (0)