Skip to content

Commit ced5a70

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 why we now have separate (but related) `BorrowIndex` and `ReserveOrActivateIndex`: 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")] ``` Includes doc for `FindPlaceUses`, as well as `Reservations` and `ActiveBorrows` structs, which are wrappers are the `Borrows` struct that dictate which flow analysis should be performed.
1 parent ef64ace commit ced5a70

File tree

7 files changed

+552
-151
lines changed

7 files changed

+552
-151
lines changed

src/librustc_mir/borrow_check/error_reporting.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::rc::Rc;
1919

2020
use super::{MirBorrowckCtxt, Context};
2121
use super::{InitializationRequiringAction, PrefixSet};
22-
use dataflow::{BorrowData, Borrows, FlowAtLocation, MovingOutStatements};
22+
use dataflow::{ActiveBorrows, BorrowData, FlowAtLocation, MovingOutStatements};
2323
use dataflow::move_paths::MovePathIndex;
2424
use util::borrowck_errors::{BorrowckErrors, Origin};
2525

@@ -324,10 +324,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
324324
_: Context,
325325
borrow: &BorrowData<'tcx>,
326326
drop_span: Span,
327-
borrows: &Borrows<'cx, 'gcx, 'tcx>
327+
borrows: &ActiveBorrows<'cx, 'gcx, 'tcx>
328328
) {
329329
let end_span = borrows.opt_region_end_span(&borrow.region);
330-
let scope_tree = borrows.scope_tree();
330+
let scope_tree = borrows.0.scope_tree();
331331
let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All).last().unwrap();
332332

333333
match root_place {

src/librustc_mir/borrow_check/flows.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,13 @@ use rustc::mir::{BasicBlock, Location};
1717

1818
use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals};
1919
use dataflow::{EverInitializedLvals, MovingOutStatements};
20-
use dataflow::{Borrows, FlowAtLocation, FlowsAtLocation};
20+
use dataflow::{ActiveBorrows, FlowAtLocation, FlowsAtLocation};
2121
use dataflow::move_paths::HasMoveData;
2222
use std::fmt;
2323

2424
// (forced to be `pub` due to its use as an associated type below.)
25-
pub struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> {
26-
pub borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
25+
pub(crate) struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> {
26+
pub borrows: FlowAtLocation<ActiveBorrows<'b, 'gcx, 'tcx>>,
2727
pub inits: FlowAtLocation<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
2828
pub uninits: FlowAtLocation<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
2929
pub move_outs: FlowAtLocation<MovingOutStatements<'b, 'gcx, 'tcx>>,
@@ -32,7 +32,7 @@ pub struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> {
3232

3333
impl<'b, 'gcx, 'tcx> Flows<'b, 'gcx, 'tcx> {
3434
pub fn new(
35-
borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
35+
borrows: FlowAtLocation<ActiveBorrows<'b, 'gcx, 'tcx>>,
3636
inits: FlowAtLocation<MaybeInitializedLvals<'b, 'gcx, 'tcx>>,
3737
uninits: FlowAtLocation<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>,
3838
move_outs: FlowAtLocation<MovingOutStatements<'b, 'gcx, 'tcx>>,
@@ -87,7 +87,7 @@ impl<'b, 'gcx, 'tcx> fmt::Display for Flows<'b, 'gcx, 'tcx> {
8787
s.push_str(", ");
8888
};
8989
saw_one = true;
90-
let borrow_data = &self.borrows.operator().borrows()[borrow];
90+
let borrow_data = &self.borrows.operator().borrows()[borrow.borrow_index()];
9191
s.push_str(&format!("{}", borrow_data));
9292
});
9393
s.push_str("] ");
@@ -99,7 +99,7 @@ impl<'b, 'gcx, 'tcx> fmt::Display for Flows<'b, 'gcx, 'tcx> {
9999
s.push_str(", ");
100100
};
101101
saw_one = true;
102-
let borrow_data = &self.borrows.operator().borrows()[borrow];
102+
let borrow_data = &self.borrows.operator().borrows()[borrow.borrow_index()];
103103
s.push_str(&format!("{}", borrow_data));
104104
});
105105
s.push_str("] ");

src/librustc_mir/borrow_check/mod.rs

Lines changed: 52 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ use syntax_pos::Span;
3030

3131
use dataflow::{do_dataflow, DebugFormatted};
3232
use dataflow::MoveDataParamEnv;
33-
use dataflow::DataflowResultsConsumer;
33+
use dataflow::{DataflowAnalysis, DataflowResultsConsumer};
3434
use dataflow::{FlowAtLocation, FlowsAtLocation};
3535
use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals};
3636
use dataflow::{EverInitializedLvals, MovingOutStatements};
37-
use dataflow::{BorrowData, BorrowIndex, Borrows};
37+
use dataflow::{Borrows, BorrowData, ReserveOrActivateIndex};
38+
use dataflow::{ActiveBorrows, Reservations};
3839
use dataflow::move_paths::{IllegalMoveOriginKind, MoveError};
3940
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex};
4041
use util::borrowck_errors::{BorrowckErrors, Origin};
@@ -48,6 +49,9 @@ use self::MutateMode::{JustWrite, WriteAndRead};
4849
mod error_reporting;
4950
mod flows;
5051
mod prefixes;
52+
53+
use std::borrow::Cow;
54+
5155
pub(crate) mod nll;
5256

5357
pub fn provide(providers: &mut Providers) {
@@ -205,23 +209,6 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
205209
};
206210
let flow_inits = flow_inits; // remove mut
207211

208-
let flow_borrows = FlowAtLocation::new(do_dataflow(
209-
tcx,
210-
mir,
211-
id,
212-
&attributes,
213-
&dead_unwinds,
214-
Borrows::new(tcx, mir, opt_regioncx, def_id, body_id),
215-
|bd, i| DebugFormatted::new(bd.location(i)),
216-
));
217-
218-
let mut state = Flows::new(
219-
flow_borrows,
220-
flow_inits,
221-
flow_uninits,
222-
flow_move_outs,
223-
flow_ever_inits,
224-
);
225212
let mut mbcx = MirBorrowckCtxt {
226213
tcx: tcx,
227214
mir: mir,
@@ -237,6 +224,44 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
237224
storage_dead_or_drop_error_reported_s: FxHashSet(),
238225
};
239226

227+
let borrows = Borrows::new(tcx, mir, opt_regioncx, def_id, body_id);
228+
let flow_reservations = do_dataflow(
229+
tcx,
230+
mir,
231+
id,
232+
&attributes,
233+
&dead_unwinds,
234+
Reservations::new(borrows),
235+
|rs, i| {
236+
// In principle we could make the dataflow ensure that
237+
// only reservation bits show up, and assert so here.
238+
//
239+
// In practice it is easier to be looser; in particular,
240+
// it is okay for the kill-sets to hold activation bits.
241+
DebugFormatted::new(&(i.kind(), rs.location(i)))
242+
});
243+
let flow_active_borrows = {
244+
let reservations_on_entry = flow_reservations.0.sets.entry_set_state();
245+
let reservations = flow_reservations.0.operator;
246+
let a = DataflowAnalysis::new_with_entry_sets(mir,
247+
&dead_unwinds,
248+
Cow::Borrowed(reservations_on_entry),
249+
ActiveBorrows::new(reservations));
250+
let results = a.run(tcx,
251+
id,
252+
&attributes,
253+
|ab, i| DebugFormatted::new(&(i.kind(), ab.location(i))));
254+
FlowAtLocation::new(results)
255+
};
256+
257+
let mut state = Flows::new(
258+
flow_active_borrows,
259+
flow_inits,
260+
flow_uninits,
261+
flow_move_outs,
262+
flow_ever_inits,
263+
);
264+
240265
mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer
241266

242267
opt_closure_req
@@ -504,9 +529,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx
504529
let data = domain.borrows();
505530
flow_state.borrows.with_elems_outgoing(|borrows| {
506531
for i in borrows {
507-
let borrow = &data[i];
532+
let borrow = &data[i.borrow_index()];
508533
let context = ContextKind::StorageDead.new(loc);
509-
510534
self.check_for_invalidation_at_exit(context, borrow, span, flow_state);
511535
}
512536
});
@@ -721,7 +745,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
721745
WriteKind::StorageDeadOrDrop => {
722746
error_reported = true;
723747
this.report_borrowed_value_does_not_live_long_enough(
724-
context, borrow, place_span.1, flow_state.borrows.operator());
748+
context, borrow, place_span.1,
749+
flow_state.borrows.operator());
725750
}
726751
WriteKind::Mutate => {
727752
error_reported = true;
@@ -1778,7 +1803,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
17781803
flow_state: &Flows<'cx, 'gcx, 'tcx>,
17791804
mut op: F,
17801805
) where
1781-
F: FnMut(&mut Self, BorrowIndex, &BorrowData<'tcx>) -> Control,
1806+
F: FnMut(&mut Self, ReserveOrActivateIndex, &BorrowData<'tcx>) -> Control,
17821807
{
17831808
let (access, place) = access_place;
17841809

@@ -1790,7 +1815,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
17901815
// check for loan restricting path P being used. Accounts for
17911816
// borrows of P, P.a.b, etc.
17921817
for i in flow_state.borrows.elems_incoming() {
1793-
let borrowed = &data[i];
1818+
// FIXME for now, just skip the activation state.
1819+
if i.is_activation() { continue }
1820+
1821+
let borrowed = &data[i.borrow_index()];
17941822

17951823
if self.places_conflict(&borrowed.borrowed_place, place, access) {
17961824
let ctrl = op(self, i, borrowed);

src/librustc_mir/dataflow/at_location.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,9 +121,8 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD>
121121
fn reconstruct_statement_effect(&mut self, loc: Location) {
122122
self.stmt_gen.reset_to_empty();
123123
self.stmt_kill.reset_to_empty();
124-
let mut ignored = IdxSetBuf::new_empty(0);
125124
let mut sets = BlockSets {
126-
on_entry: &mut ignored,
125+
on_entry: &mut self.curr_state,
127126
gen_set: &mut self.stmt_gen,
128127
kill_set: &mut self.stmt_kill,
129128
};
@@ -135,9 +134,8 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD>
135134
fn reconstruct_terminator_effect(&mut self, loc: Location) {
136135
self.stmt_gen.reset_to_empty();
137136
self.stmt_kill.reset_to_empty();
138-
let mut ignored = IdxSetBuf::new_empty(0);
139137
let mut sets = BlockSets {
140-
on_entry: &mut ignored,
138+
on_entry: &mut self.curr_state,
141139
gen_set: &mut self.stmt_gen,
142140
kill_set: &mut self.stmt_kill,
143141
};

0 commit comments

Comments
 (0)