Skip to content

Commit

Permalink
Only handle ReVar regions in NLL borrowck
Browse files Browse the repository at this point in the history 
Now that lexical MIR borrowck is gone, there's no need to store Regions
unnecessarily.
  • Loading branch information
@matthewjasper
matthewjasper committed Nov 18, 2018
1 parent b16985a commit 6027e08
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 105 deletions.
28 changes: 12 additions & 16 deletions src/librustc_mir/borrow_check/borrow_set.rs
View file
Expand Up @@ -9,14 +9,15 @@
// except according to those terms.

use borrow_check::place_ext::PlaceExt;
use borrow_check::nll::ToRegionVid;
use dataflow::indexes::BorrowIndex;
use dataflow::move_paths::MoveData;
use rustc::mir::traversal;
use rustc::mir::visit::{
PlaceContext, Visitor, NonUseContext, MutatingUseContext, NonMutatingUseContext
};
use rustc::mir::{self, Location, Mir, Place, Local};
use rustc::ty::{Region, TyCtxt};
use rustc::ty::{RegionVid, TyCtxt};
use rustc::util::nodemap::{FxHashMap, FxHashSet};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::bit_set::BitSet;
Expand All @@ -42,7 +43,7 @@ crate struct BorrowSet<'tcx> {

/// Every borrow has a region; this maps each such regions back to
/// its borrow-indexes.
crate region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
crate region_map: FxHashMap<RegionVid, FxHashSet<BorrowIndex>>,

/// Map from local to all the borrows on that local
crate local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,
Expand Down Expand Up @@ -77,7 +78,7 @@ crate struct BorrowData<'tcx> {
/// What kind of borrow this is
crate kind: mir::BorrowKind,
/// The region for which this borrow is live
crate region: Region<'tcx>,
crate region: RegionVid,
/// Place from which we are borrowing
crate borrowed_place: mir::Place<'tcx>,
/// Place to which the borrow was stored
Expand All @@ -92,13 +93,7 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
mir::BorrowKind::Unique => "uniq ",
mir::BorrowKind::Mut { .. } => "mut ",
};
let region = self.region.to_string();
let separator = if !region.is_empty() {
" "
} else {
""
};
write!(w, "&{}{}{}{:?}", region, separator, kind, self.borrowed_place)
write!(w, "&{:?} {}{:?}", self.region, kind, self.borrowed_place)
}
}

Expand Down Expand Up @@ -189,7 +184,7 @@ struct GatherBorrows<'a, 'gcx: 'tcx, 'tcx: 'a> {
idx_vec: IndexVec<BorrowIndex, BorrowData<'tcx>>,
location_map: FxHashMap<Location, BorrowIndex>,
activation_map: FxHashMap<Location, Vec<BorrowIndex>>,
region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
region_map: FxHashMap<RegionVid, FxHashSet<BorrowIndex>>,
local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,

/// When we encounter a 2-phase borrow statement, it will always
Expand Down Expand Up @@ -219,6 +214,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
return;
}

let region = region.to_region_vid();

let borrow = BorrowData {
kind,
region,
Expand All @@ -230,7 +227,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
let idx = self.idx_vec.push(borrow);
self.location_map.insert(location, idx);

self.insert_as_pending_if_two_phase(location, &assigned_place, region, kind, idx);
self.insert_as_pending_if_two_phase(location, &assigned_place, kind, idx);

self.region_map.entry(region).or_default().insert(idx);
if let Some(local) = borrowed_place.root_local() {
Expand Down Expand Up @@ -314,7 +311,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
let borrow_data = &self.idx_vec[borrow_index];
assert_eq!(borrow_data.reserve_location, location);
assert_eq!(borrow_data.kind, kind);
assert_eq!(borrow_data.region, region);
assert_eq!(borrow_data.region, region.to_region_vid());
assert_eq!(borrow_data.borrowed_place, *place);
}

Expand Down Expand Up @@ -347,13 +344,12 @@ impl<'a, 'gcx, 'tcx> GatherBorrows<'a, 'gcx, 'tcx> {
&mut self,
start_location: Location,
assigned_place: &mir::Place<'tcx>,
region: Region<'tcx>,
kind: mir::BorrowKind,
borrow_index: BorrowIndex,
) {
debug!(
"Borrows::insert_as_pending_if_two_phase({:?}, {:?}, {:?}, {:?})",
start_location, assigned_place, region, borrow_index,
"Borrows::insert_as_pending_if_two_phase({:?}, {:?}, {:?})",
start_location, assigned_place, borrow_index,
);

if !self.allow_two_phase_borrow(kind) {
Expand Down
7 changes: 1 addition & 6 deletions src/librustc_mir/borrow_check/mod.rs
View file
Expand Up @@ -14,7 +14,6 @@ use borrow_check::nll::region_infer::RegionInferenceContext;
use rustc::hir;
use rustc::hir::Node;
use rustc::hir::def_id::DefId;
use rustc::hir::map::definitions::DefPathData;
use rustc::infer::InferCtxt;
use rustc::lint::builtin::UNUSED_MUT;
use rustc::middle::borrowck::SignalledError;
Expand Down Expand Up @@ -162,10 +161,6 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
move_data: move_data,
param_env: param_env,
};
let body_id = match tcx.def_key(def_id).disambiguated_data.data {
DefPathData::StructCtor | DefPathData::EnumVariant(_) => None,
_ => Some(tcx.hir.body_owned_by(id)),
};

let dead_unwinds = BitSet::new_empty(mir.basic_blocks().len());
let mut flow_inits = FlowAtLocation::new(do_dataflow(
Expand Down Expand Up @@ -212,7 +207,7 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
id,
&attributes,
&dead_unwinds,
Borrows::new(tcx, mir, regioncx.clone(), def_id, body_id, &borrow_set),
Borrows::new(tcx, mir, regioncx.clone(), &borrow_set),
|rs, i| DebugFormatted::new(&rs.location(i)),
));
let flow_uninits = FlowAtLocation::new(do_dataflow(
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
View file
Expand Up @@ -206,7 +206,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let mir = self.mir;
let tcx = self.infcx.tcx;

let borrow_region_vid = regioncx.to_region_vid(borrow.region);
let borrow_region_vid = borrow.region;
debug!(
"explain_why_borrow_contains_point: borrow_region_vid={:?}",
borrow_region_vid
Expand Down
91 changes: 9 additions & 82 deletions src/librustc_mir/dataflow/impls/borrows.rs
View file
Expand Up @@ -12,18 +12,13 @@ use borrow_check::borrow_set::{BorrowSet, BorrowData};
use borrow_check::place_ext::PlaceExt;

use rustc;
use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::middle::region;
use rustc::mir::{self, Location, Place, Mir};
use rustc::ty::TyCtxt;
use rustc::ty::{RegionKind, RegionVid};
use rustc::ty::RegionKind::ReScope;
use rustc::ty::RegionVid;

use rustc_data_structures::bit_set::{BitSet, BitSetOperator};
use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc_data_structures::sync::Lrc;

use dataflow::{BitDenotation, BlockSets, InitialFlow};
pub use dataflow::indexes::BorrowIndex;
Expand All @@ -42,8 +37,6 @@ use std::rc::Rc;
pub struct Borrows<'a, 'gcx: 'tcx, 'tcx: 'a> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
scope_tree: Lrc<region::ScopeTree>,
root_scope: Option<region::Scope>,

borrow_set: Rc<BorrowSet<'tcx>>,
borrows_out_of_scope_at_location: FxHashMap<Location, Vec<BorrowIndex>>,
Expand Down Expand Up @@ -150,18 +143,8 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
nonlexical_regioncx: Rc<RegionInferenceContext<'tcx>>,
def_id: DefId,
body_id: Option<hir::BodyId>,
borrow_set: &Rc<BorrowSet<'tcx>>,
) -> Self {
let scope_tree = tcx.region_scope_tree(def_id);
let root_scope = body_id.map(|body_id| {
region::Scope {
id: tcx.hir.body(body_id).value.hir_id.local_id,
data: region::ScopeData::CallSite
}
});

let mut borrows_out_of_scope_at_location = FxHashMap::default();
for (borrow_index, borrow_data) in borrow_set.borrows.iter_enumerated() {
let borrow_region = borrow_data.region.to_region_vid();
Expand All @@ -177,8 +160,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
mir: mir,
borrow_set: borrow_set.clone(),
borrows_out_of_scope_at_location,
scope_tree,
root_scope,
_nonlexical_regioncx: nonlexical_regioncx,
}
}
Expand Down Expand Up @@ -277,22 +258,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
panic!("could not find BorrowIndex for location {:?}", location);
});

if let RegionKind::ReEmpty = region {
// If the borrowed value dies before the borrow is used, the region for
// the borrow can be empty. Don't track the borrow in that case.
debug!("Borrows::statement_effect_on_borrows \
location: {:?} stmt: {:?} has empty region, killing {:?}",
location, stmt.kind, index);
sets.kill(*index);
return
} else {
debug!("Borrows::statement_effect_on_borrows location: {:?} stmt: {:?}",
location, stmt.kind);
}

assert!(self.borrow_set.region_map.get(region).unwrap_or_else(|| {
panic!("could not find BorrowIndexs for region {:?}", region);
}).contains(&index));
assert!(self.borrow_set.region_map
.get(&region.to_region_vid())
.unwrap_or_else(|| {
panic!("could not find BorrowIndexs for RegionVid {:?}", region);
})
.contains(&index)
);
sets.gen(*index);

// Issue #46746: Two-phase borrows handles
Expand Down Expand Up @@ -349,52 +321,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
self.kill_loans_out_of_scope_at_location(sets, location);
}

fn terminator_effect(&self, sets: &mut BlockSets<BorrowIndex>, location: Location) {
debug!("Borrows::terminator_effect sets: {:?} location: {:?}", sets, location);

let block = &self.mir.basic_blocks().get(location.block).unwrap_or_else(|| {
panic!("could not find block at location {:?}", location);
});

let term = block.terminator();
match term.kind {
mir::TerminatorKind::Resume |
mir::TerminatorKind::Return |
mir::TerminatorKind::GeneratorDrop => {
// When we return from the function, then all `ReScope`-style regions
// are guaranteed to have ended.
// Normally, there would be `EndRegion` statements that come before,
// and hence most of these loans will already be dead -- but, in some cases
// like unwind paths, we do not always emit `EndRegion` statements, so we
// add some kills here as a "backup" and to avoid spurious error messages.
for (borrow_index, borrow_data) in self.borrow_set.borrows.iter_enumerated() {
if let ReScope(scope) = borrow_data.region {
// Check that the scope is not actually a scope from a function that is
// a parent of our closure. Note that the CallSite scope itself is
// *outside* of the closure, for some weird reason.
if let Some(root_scope) = self.root_scope {
if *scope != root_scope &&
self.scope_tree.is_subscope_of(*scope, root_scope)
{
sets.kill(borrow_index);
}
}
}
}
}
mir::TerminatorKind::Abort |
mir::TerminatorKind::SwitchInt {..} |
mir::TerminatorKind::Drop {..} |
mir::TerminatorKind::DropAndReplace {..} |
mir::TerminatorKind::Call {..} |
mir::TerminatorKind::Assert {..} |
mir::TerminatorKind::Yield {..} |
mir::TerminatorKind::Goto {..} |
mir::TerminatorKind::FalseEdges {..} |
mir::TerminatorKind::FalseUnwind {..} |
mir::TerminatorKind::Unreachable => {}
}
}
fn terminator_effect(&self, _: &mut BlockSets<BorrowIndex>, _: Location) {}

fn propagate_call_return(&self,
_in_out: &mut BitSet<BorrowIndex>,
Expand Down

0 comments on commit 6027e08

Please sign in to comment.