Skip to content

Commit e34e832

Browse files
committed
New ActiveBorrows dataflow for two-phase &mut; not yet borrowed-checked.
High-level picture: The old `Borrows` analysis is now called `Reservations` (implemented as a newtype wrapper around `Borrows`); this continues to compute whether a `Rvalue::Ref` can reach a statement without an intervening `EndRegion`. In addition, we also track what `Place` each such `Rvalue::Ref` was immediately assigned to in a given borrow (yay for MIR-structural properties!). The new `ActiveBorrows` analysis then tracks the initial use of any of those assigned `Places` for a given borrow. I.e. a borrow becomes "active" immediately after it starts being "used" in some way. (This is conservative in the sense that we will treat a copy `x = y;` as a use of `y`; in principle one might further delay activation in such cases.) The new `ActiveBorrows` analysis needs to take the `Reservations` results as an initial input, because the reservation state influences the gen/kill sets for `ActiveBorrows`. In particular, a use of `a` activates a borrow `a = &b` if and only if there exists a path (in the control flow graph) from the borrow to that use. So we need to know if the borrow reaches a given use to know if it really gets a gen-bit or not. * Incorporating the output from one dataflow analysis into the input of another required more changes to the infrastructure than I had expected, and even after those changes, the resulting code is still a bit subtle. * In particular, Since we need to know the intrablock reservation state, we need to dynamically update a bitvector for the reservations as we are also trying to compute the gen/kills bitvector for the active borrows. * The way I ended up deciding to do this (after also toying with at least two other designs) is to put both the reservation state and the active borrow state into a single bitvector. That is what we now have separate (but related) `BorrowIndex` and `ResActIndex`: each borrow index maps to a pair of neighboring reservation and activation indexes. As noted above, these changes are solely adding the active borrows dataflow analysis (and updating the existing code to cope with the switch from `Borrows` to `Reservations`). The code to process the bitvector in the borrow checker currently just skips over all of the active borrow bits. But atop this commit, one *can* observe the analysis results by looking at the graphviz output, e.g. via ```rust #[rustc_mir(borrowck_graphviz_preflow="pre_two_phase.dot", borrowck_graphviz_postflow="post_two_phase.dot")] ```
1 parent 535f4f4 commit e34e832

File tree

4 files changed

+521
-126
lines changed

4 files changed

+521
-126
lines changed

src/librustc_mir/borrow_check/mod.rs

Lines changed: 52 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,19 @@ use syntax_pos::Span;
2929
use dataflow::{do_dataflow, DebugFormatted};
3030
use dataflow::MoveDataParamEnv;
3131
use dataflow::{SetTriple, BlockSets};
32-
use dataflow::{BitDenotation, DataflowResults, DataflowResultsConsumer};
32+
use dataflow::{BitDenotation, DataflowAnalysis, DataflowResults, DataflowResultsConsumer};
3333
use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals};
3434
use dataflow::{EverInitializedLvals, MovingOutStatements};
35-
use dataflow::{BorrowData, BorrowIndex, Borrows};
35+
use dataflow::{Borrows, BorrowData, ResActIndex};
36+
use dataflow::{ActiveBorrows, Reservations};
3637
use dataflow::move_paths::{IllegalMoveOriginKind, MoveError};
3738
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MoveOutIndex, MovePathIndex};
3839
use util::borrowck_errors::{BorrowckErrors, Origin};
3940

4041
use self::MutateMode::{JustWrite, WriteAndRead};
4142

43+
use std::borrow::Cow;
44+
4245
pub(crate) mod nll;
4346

4447
pub fn provide(providers: &mut Providers) {
@@ -193,18 +196,38 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
193196
storage_dead_or_drop_error_reported: FxHashSet(),
194197
};
195198

196-
let flow_borrows = FlowInProgress::new(do_dataflow(
199+
let borrows = Borrows::new(tcx, mir, opt_regioncx);
200+
let flow_reservations = do_dataflow(
197201
tcx,
198202
mir,
199203
id,
200204
&attributes,
201205
&dead_unwinds,
202-
Borrows::new(tcx, mir, opt_regioncx),
203-
|bd, i| DebugFormatted::new(bd.location(i)),
204-
));
206+
Reservations::new(borrows),
207+
|rs, i| {
208+
// In principle we could make the dataflow ensure that
209+
// only reservation bits show up, and assert so here.
210+
//
211+
// In practice it is easier to be looser; in particular,
212+
// it is okay for the kill-sets to hold activation bits.
213+
DebugFormatted::new(&(i.kind(), rs.location(i)))
214+
});
215+
let flow_active_borrows = {
216+
let reservations_on_entry = flow_reservations.0.sets.entry_set_state();
217+
let reservations = flow_reservations.0.operator;
218+
let a = DataflowAnalysis::new_with_entry_sets(mir,
219+
&dead_unwinds,
220+
Cow::Borrowed(reservations_on_entry),
221+
ActiveBorrows::new(reservations));
222+
let results = a.run(tcx,
223+
id,
224+
&attributes,
225+
|ab, i| DebugFormatted::new(&(i.kind(), ab.location(i))));
226+
FlowInProgress::new(results)
227+
};
205228

206229
let mut state = InProgress::new(
207-
flow_borrows,
230+
flow_active_borrows,
208231
flow_inits,
209232
flow_uninits,
210233
flow_move_outs,
@@ -229,7 +252,7 @@ pub struct MirBorrowckCtxt<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
229252

230253
// (forced to be `pub` due to its use as an associated type below.)
231254
pub struct InProgress<'b, 'gcx: 'tcx, 'tcx: 'b> {
232-
borrows: FlowInProgress<Borrows<'b, 'gcx, 'tcx>>,
255+
borrows: FlowInProgress<ActiveBorrows<'b, 'gcx, 'tcx>>,
233256
inits: FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
234257
uninits: FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
235258
move_outs: FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>,
@@ -535,7 +558,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
535558
let data = domain.borrows();
536559
flow_state.borrows.with_elems_outgoing(|borrows| {
537560
for i in borrows {
538-
let borrow = &data[i];
561+
// FIXME: does this need to care about reserved/active distinction?
562+
let borrow = &data[i.borrow_index()];
539563

540564
if self.place_is_invalidated_at_exit(&borrow.borrowed_place) {
541565
debug!("borrow conflicts at exit {:?}", borrow);
@@ -1485,7 +1509,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
14851509
flow_state: &InProgress<'cx, 'gcx, 'tcx>,
14861510
mut op: F,
14871511
) where
1488-
F: FnMut(&mut Self, BorrowIndex, &BorrowData<'tcx>, &Place<'tcx>) -> Control,
1512+
F: FnMut(&mut Self, ResActIndex, &BorrowData<'tcx>, &Place<'tcx>) -> Control,
14891513
{
14901514
let (access, place) = access_place;
14911515

@@ -1498,7 +1522,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
14981522
// check for loan restricting path P being used. Accounts for
14991523
// borrows of P, P.a.b, etc.
15001524
'next_borrow: for i in flow_state.borrows.elems_incoming() {
1501-
let borrowed = &data[i];
1525+
// FIXME for now, just skip the activation state.
1526+
if i.is_activation() { continue 'next_borrow; }
1527+
1528+
let borrowed = &data[i.borrow_index()];
15021529

15031530
// Is `place` (or a prefix of it) already borrowed? If
15041531
// so, that's relevant.
@@ -2337,7 +2364,7 @@ impl ContextKind {
23372364

23382365
impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> {
23392366
fn new(
2340-
borrows: FlowInProgress<Borrows<'b, 'gcx, 'tcx>>,
2367+
borrows: FlowInProgress<ActiveBorrows<'b, 'gcx, 'tcx>>,
23412368
inits: FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
23422369
uninits: FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
23432370
move_outs: FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>,
@@ -2360,7 +2387,7 @@ impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> {
23602387
mut xform_move_outs: XM,
23612388
mut xform_ever_inits: XE,
23622389
) where
2363-
XB: FnMut(&mut FlowInProgress<Borrows<'b, 'gcx, 'tcx>>),
2390+
XB: FnMut(&mut FlowInProgress<ActiveBorrows<'b, 'gcx, 'tcx>>),
23642391
XI: FnMut(&mut FlowInProgress<MaybeInitializedLvals<'b, 'gcx, 'tcx>>),
23652392
XU: FnMut(&mut FlowInProgress<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>),
23662393
XM: FnMut(&mut FlowInProgress<MovingOutStatements<'b, 'gcx, 'tcx>>),
@@ -2378,25 +2405,25 @@ impl<'b, 'gcx, 'tcx> InProgress<'b, 'gcx, 'tcx> {
23782405

23792406
s.push_str("borrows in effect: [");
23802407
let mut saw_one = false;
2381-
self.borrows.each_state_bit(|borrow| {
2408+
self.borrows.each_state_bit(|ra| {
23822409
if saw_one {
23832410
s.push_str(", ");
23842411
};
23852412
saw_one = true;
2386-
let borrow_data = &self.borrows.base_results.operator().borrows()[borrow];
2387-
s.push_str(&format!("{}", borrow_data));
2413+
let borrow_data = &self.borrows.base_results.operator().borrows()[ra.borrow_index()];
2414+
s.push_str(&format!("{} {}", borrow_data, ra.kind()));
23882415
});
23892416
s.push_str("] ");
23902417

23912418
s.push_str("borrows generated: [");
23922419
let mut saw_one = false;
2393-
self.borrows.each_gen_bit(|borrow| {
2420+
self.borrows.each_gen_bit(|ra| {
23942421
if saw_one {
23952422
s.push_str(", ");
23962423
};
23972424
saw_one = true;
2398-
let borrow_data = &self.borrows.base_results.operator().borrows()[borrow];
2399-
s.push_str(&format!("{}", borrow_data));
2425+
let borrow_data = &self.borrows.base_results.operator().borrows()[ra.borrow_index()];
2426+
s.push_str(&format!("{} {}", borrow_data, ra.kind()));
24002427
});
24012428
s.push_str("] ");
24022429

@@ -2526,9 +2553,10 @@ where
25262553
fn reconstruct_statement_effect(&mut self, loc: Location) {
25272554
self.sets.gen_set.reset_to_empty();
25282555
self.sets.kill_set.reset_to_empty();
2529-
let mut ignored = IdxSetBuf::new_empty(0);
2556+
// Note BD's that override accumulates_intrablock_state need
2557+
// sets.on_entry to match state immediately before this stmt.
25302558
let mut sets = BlockSets {
2531-
on_entry: &mut ignored,
2559+
on_entry: &mut self.sets.on_entry,
25322560
gen_set: &mut self.sets.gen_set,
25332561
kill_set: &mut self.sets.kill_set,
25342562
};
@@ -2540,9 +2568,10 @@ where
25402568
fn reconstruct_terminator_effect(&mut self, loc: Location) {
25412569
self.sets.gen_set.reset_to_empty();
25422570
self.sets.kill_set.reset_to_empty();
2543-
let mut ignored = IdxSetBuf::new_empty(0);
2571+
// Note BD's that override accumulates_intrablock_state need
2572+
// sets.on_entry to match state immediately before terminator.
25442573
let mut sets = BlockSets {
2545-
on_entry: &mut ignored,
2574+
on_entry: &mut self.sets.on_entry,
25462575
gen_set: &mut self.sets.gen_set,
25472576
kill_set: &mut self.sets.kill_set,
25482577
};

0 commit comments

Comments
 (0)