Skip to content

Commit

Permalink
Auto merge of #48770 - bobtwinkles:two_phase_borrows_rewrite, r=pnkfelix
Browse files Browse the repository at this point in the history
Two phase borrows rewrite

This definitely needs a careful review. Both @pnkfelix and @nikomatsakis  were involved with the design of this so they're natural choices here. I'm r?'ing @pnkfelix since they wrote the original two-phase borrow implementation. Also ping @KiChjang who expressed interest in working on this. I'm going to leave a few comments below pointing out some of the more dangerous changes I made (i.e. what I would like reviewers to pay special attention too.)

r? @pnkfelix
  • Loading branch information
bors committed Mar 12, 2018
2 parents 222b0eb + 9a5d61a commit 883e746
Show file tree
Hide file tree
Showing 13 changed files with 572 additions and 600 deletions.
6 changes: 6 additions & 0 deletions src/libgraphviz/lib.rs
Expand Up @@ -711,6 +711,12 @@ impl<'a> IntoCow<'a, str> for &'a str {
}
}

impl<'a> IntoCow<'a, str> for Cow<'a, str> {
fn into_cow(self) -> Cow<'a, str> {
self
}
}

impl<'a, T: Clone> IntoCow<'a, [T]> for Vec<T> {
fn into_cow(self) -> Cow<'a, [T]> {
Cow::Owned(self)
Expand Down
12 changes: 6 additions & 6 deletions src/librustc_mir/borrow_check/error_reporting.rs
Expand Up @@ -18,7 +18,7 @@ use rustc_data_structures::sync::Lrc;

use super::{Context, MirBorrowckCtxt};
use super::{InitializationRequiringAction, PrefixSet};
use dataflow::{ActiveBorrows, BorrowData, FlowAtLocation, MovingOutStatements};
use dataflow::{Borrows, BorrowData, FlowAtLocation, MovingOutStatements};
use dataflow::move_paths::MovePathIndex;
use util::borrowck_errors::{BorrowckErrors, Origin};

Expand Down Expand Up @@ -250,7 +250,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

let new_closure_span = self.find_closure_span(span, context.loc);
let span = new_closure_span.map(|(args, _)| args).unwrap_or(span);
let old_closure_span = self.find_closure_span(issued_span, issued_borrow.location);
let old_closure_span = self.find_closure_span(issued_span, issued_borrow.reserve_location);
let issued_span = old_closure_span
.map(|(args, _)| args)
.unwrap_or(issued_span);
Expand Down Expand Up @@ -372,15 +372,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
context: Context,
borrow: &BorrowData<'tcx>,
drop_span: Span,
borrows: &ActiveBorrows<'cx, 'gcx, 'tcx>,
borrows: &Borrows<'cx, 'gcx, 'tcx>
) {
let end_span = borrows.opt_region_end_span(&borrow.region);
let scope_tree = borrows.0.scope_tree();
let scope_tree = borrows.scope_tree();
let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All)
.last()
.unwrap();

let borrow_span = self.mir.source_info(borrow.location).span;
let borrow_span = self.mir.source_info(borrow.reserve_location).span;
let proper_span = match *root_place {
Place::Local(local) => self.mir.local_decls[local].source_info.span,
_ => drop_span,
Expand Down Expand Up @@ -817,7 +817,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

// Retrieve span of given borrow from the current MIR representation
pub fn retrieve_borrow_span(&self, borrow: &BorrowData) -> Span {
self.mir.source_info(borrow.location).span
self.mir.source_info(borrow.reserve_location).span
}

// Retrieve type of a place for the current MIR representation
Expand Down
7 changes: 4 additions & 3 deletions src/librustc_mir/borrow_check/flows.rs
Expand Up @@ -17,13 +17,14 @@ use rustc::mir::{BasicBlock, Location};

use dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
use dataflow::{EverInitializedPlaces, MovingOutStatements};
use dataflow::{ActiveBorrows, FlowAtLocation, FlowsAtLocation};
use dataflow::{Borrows};
use dataflow::{FlowAtLocation, FlowsAtLocation};
use dataflow::move_paths::HasMoveData;
use std::fmt;

// (forced to be `pub` due to its use as an associated type below.)
pub(crate) struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> {
pub borrows: FlowAtLocation<ActiveBorrows<'b, 'gcx, 'tcx>>,
pub borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
pub inits: FlowAtLocation<MaybeInitializedPlaces<'b, 'gcx, 'tcx>>,
pub uninits: FlowAtLocation<MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
pub move_outs: FlowAtLocation<MovingOutStatements<'b, 'gcx, 'tcx>>,
Expand All @@ -32,7 +33,7 @@ pub(crate) struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> {

impl<'b, 'gcx, 'tcx> Flows<'b, 'gcx, 'tcx> {
pub fn new(
borrows: FlowAtLocation<ActiveBorrows<'b, 'gcx, 'tcx>>,
borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
inits: FlowAtLocation<MaybeInitializedPlaces<'b, 'gcx, 'tcx>>,
uninits: FlowAtLocation<MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
move_outs: FlowAtLocation<MovingOutStatements<'b, 'gcx, 'tcx>>,
Expand Down
53 changes: 15 additions & 38 deletions src/librustc_mir/borrow_check/mod.rs
Expand Up @@ -34,11 +34,10 @@ use syntax_pos::Span;
use dataflow::{do_dataflow, DebugFormatted};
use dataflow::FlowAtLocation;
use dataflow::MoveDataParamEnv;
use dataflow::{DataflowAnalysis, DataflowResultsConsumer};
use dataflow::{DataflowResultsConsumer};
use dataflow::{MaybeInitializedPlaces, MaybeUninitializedPlaces};
use dataflow::{EverInitializedPlaces, MovingOutStatements};
use dataflow::{BorrowData, Borrows, ReserveOrActivateIndex};
use dataflow::{ActiveBorrows, Reservations};
use dataflow::indexes::BorrowIndex;
use dataflow::move_paths::{IllegalMoveOriginKind, MoveError};
use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex};
Expand All @@ -54,8 +53,6 @@ mod error_reporting;
mod flows;
mod prefixes;

use std::borrow::Cow;

pub(crate) mod nll;

pub fn provide(providers: &mut Providers) {
Expand Down Expand Up @@ -209,6 +206,18 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
};
let flow_inits = flow_inits; // remove mut

let flow_borrows = FlowAtLocation::new(do_dataflow(
tcx,
mir,
id,
&attributes,
&dead_unwinds,
Borrows::new(tcx, mir, opt_regioncx.clone(), def_id, body_id),
|rs, i| {
DebugFormatted::new(&(i.kind(), rs.location(i.borrow_index())))
}
));

let movable_generator = !match tcx.hir.get(id) {
hir::map::Node::NodeExpr(&hir::Expr {
node: hir::ExprClosure(.., Some(hir::GeneratorMovability::Static)),
Expand All @@ -230,44 +239,12 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
},
access_place_error_reported: FxHashSet(),
reservation_error_reported: FxHashSet(),
nonlexical_regioncx: opt_regioncx.clone(),
nonlexical_regioncx: opt_regioncx,
nonlexical_cause_info: None,
};

let borrows = Borrows::new(tcx, mir, opt_regioncx, def_id, body_id);
let flow_reservations = do_dataflow(
tcx,
mir,
id,
&attributes,
&dead_unwinds,
Reservations::new(borrows),
|rs, i| {
// In principle we could make the dataflow ensure that
// only reservation bits show up, and assert so here.
//
// In practice it is easier to be looser; in particular,
// it is okay for the kill-sets to hold activation bits.
DebugFormatted::new(&(i.kind(), rs.location(i)))
},
);
let flow_active_borrows = {
let reservations_on_entry = flow_reservations.0.sets.entry_set_state();
let reservations = flow_reservations.0.operator;
let a = DataflowAnalysis::new_with_entry_sets(
mir,
&dead_unwinds,
Cow::Borrowed(reservations_on_entry),
ActiveBorrows::new(reservations),
);
let results = a.run(tcx, id, &attributes, |ab, i| {
DebugFormatted::new(&(i.kind(), ab.location(i)))
});
FlowAtLocation::new(results)
};

let mut state = Flows::new(
flow_active_borrows,
flow_borrows,
flow_inits,
flow_uninits,
flow_move_outs,
Expand Down

0 comments on commit 883e746

Please sign in to comment.