Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MIR borrowck: implement union-and-array-compatible semantics #46268

Merged
merged 11 commits into from
Dec 6, 2017
6 changes: 4 additions & 2 deletions src/libcore/cell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1084,9 +1084,11 @@ impl<'b, T: ?Sized> RefMut<'b, T> {
pub fn map<U: ?Sized, F>(orig: RefMut<'b, T>, f: F) -> RefMut<'b, U>
where F: FnOnce(&mut T) -> &mut U
{
// FIXME(nll-rfc#40): fix borrow-check
let RefMut { value, borrow } = orig;
RefMut {
value: f(orig.value),
borrow: orig.borrow,
value: f(value),
borrow: borrow,
}
}
}
Expand Down
18 changes: 12 additions & 6 deletions src/libcore/iter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1776,12 +1776,18 @@ impl<I: Iterator> Iterator for Peekable<I> {

#[inline]
fn nth(&mut self, n: usize) -> Option<I::Item> {
match self.peeked.take() {
// the .take() below is just to avoid "move into pattern guard"
Some(ref mut v) if n == 0 => v.take(),
Some(None) => None,
Some(Some(_)) => self.iter.nth(n - 1),
None => self.iter.nth(n),
// FIXME(#6393): merge these when borrow-checking gets better.
if n == 0 {
match self.peeked.take() {
Some(v) => v,
None => self.iter.nth(n),
}
} else {
match self.peeked.take() {
Some(None) => None,
Some(Some(_)) => self.iter.nth(n - 1),
None => self.iter.nth(n),
}
}
}

Expand Down
513 changes: 428 additions & 85 deletions src/librustc_mir/borrow_check/mod.rs

Large diffs are not rendered by default.

11 changes: 11 additions & 0 deletions src/librustc_mir/build/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,17 @@ impl<'tcx> CFG<'tcx> {
source_info: SourceInfo,
region_scope: region::Scope) {
if tcx.sess.emit_end_regions() {
if let region::ScopeData::CallSite(_) = region_scope.data() {
// The CallSite scope (aka the root scope) is sort of weird, in that it is
// supposed to "separate" the "interior" and "exterior" of a closure. Being
// that, it is not really a part of the region hierarchy, but for some
// reason it *is* considered a part of it.
//
// It should die a hopefully painful death with NLL, so let's leave this hack
// for now so that nobody can complain about soundness.
return
}

self.push(block, Statement {
source_info,
kind: StatementKind::EndRegion(region_scope),
Expand Down
60 changes: 55 additions & 5 deletions src/librustc_mir/dataflow/impls/borrows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::middle::region;
use rustc::mir::{self, Location, Mir};
use rustc::mir::visit::Visitor;
use rustc::ty::{self, Region, TyCtxt};
Expand All @@ -27,16 +30,20 @@ use borrow_check::nll::ToRegionVid;
use syntax_pos::Span;

use std::fmt;
use std::rc::Rc;

// `Borrows` maps each dataflow bit to an `Rvalue::Ref`, which can be
// uniquely identified in the MIR by the `Location` of the assigment
// statement in which it appears on the right hand side.
pub struct Borrows<'a, 'gcx: 'tcx, 'tcx: 'a> {
tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
scope_tree: Rc<region::ScopeTree>,
root_scope: Option<region::Scope>,
borrows: IndexVec<BorrowIndex, BorrowData<'tcx>>,
location_map: FxHashMap<Location, BorrowIndex>,
region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,
region_span_map: FxHashMap<RegionKind, Span>,
nonlexical_regioncx: Option<RegionInferenceContext<'tcx>>,
}
Expand Down Expand Up @@ -69,22 +76,32 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> {
impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>,
mir: &'a Mir<'tcx>,
nonlexical_regioncx: Option<RegionInferenceContext<'tcx>>)
nonlexical_regioncx: Option<RegionInferenceContext<'tcx>>,
def_id: DefId,
body_id: Option<hir::BodyId>)
-> Self {
let scope_tree = tcx.region_scope_tree(def_id);
let root_scope = body_id.map(|body_id| {
region::Scope::CallSite(tcx.hir.body(body_id).value.hir_id.local_id)
});
let mut visitor = GatherBorrows {
tcx,
mir,
idx_vec: IndexVec::new(),
location_map: FxHashMap(),
region_map: FxHashMap(),
local_map: FxHashMap(),
region_span_map: FxHashMap()
};
visitor.visit_mir(mir);
return Borrows { tcx: tcx,
mir: mir,
borrows: visitor.idx_vec,
scope_tree,
root_scope,
location_map: visitor.location_map,
region_map: visitor.region_map,
local_map: visitor.local_map,
region_span_map: visitor.region_span_map,
nonlexical_regioncx };

Expand All @@ -94,13 +111,22 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
idx_vec: IndexVec<BorrowIndex, BorrowData<'tcx>>,
location_map: FxHashMap<Location, BorrowIndex>,
region_map: FxHashMap<Region<'tcx>, FxHashSet<BorrowIndex>>,
local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,
region_span_map: FxHashMap<RegionKind, Span>,
}

impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
fn visit_rvalue(&mut self,
rvalue: &mir::Rvalue<'tcx>,
location: mir::Location) {
fn root_local(mut p: &mir::Place<'_>) -> Option<mir::Local> {
loop { match p {
mir::Place::Projection(pi) => p = &pi.base,
mir::Place::Static(_) => return None,
mir::Place::Local(l) => return Some(*l)
}}
}

if let mir::Rvalue::Ref(region, kind, ref place) = *rvalue {
if is_unsafe_place(self.tcx, self.mir, place) { return; }

Expand All @@ -109,8 +135,14 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
};
let idx = self.idx_vec.push(borrow);
self.location_map.insert(location, idx);

let borrows = self.region_map.entry(region).or_insert(FxHashSet());
borrows.insert(idx);

if let Some(local) = root_local(place) {
let borrows = self.local_map.entry(local).or_insert(FxHashSet());
borrows.insert(idx);
}
}
}

Expand Down Expand Up @@ -199,7 +231,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
mir::StatementKind::EndRegion(region_scope) => {
if let Some(borrow_indexes) = self.region_map.get(&ReScope(region_scope)) {
assert!(self.nonlexical_regioncx.is_none());
for idx in borrow_indexes { sets.kill(&idx); }
sets.kill_all(borrow_indexes);
} else {
// (if there is no entry, then there are no borrows to be tracked)
}
Expand All @@ -224,10 +256,19 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
}
}

mir::StatementKind::StorageDead(local) => {
// Make sure there are no remaining borrows for locals that
// are gone out of scope.
//
// FIXME: expand this to variables that are assigned over.
if let Some(borrow_indexes) = self.local_map.get(&local) {
sets.kill_all(borrow_indexes);
}
}

mir::StatementKind::InlineAsm { .. } |
mir::StatementKind::SetDiscriminant { .. } |
mir::StatementKind::StorageLive(..) |
mir::StatementKind::StorageDead(..) |
mir::StatementKind::Validate(..) |
mir::StatementKind::Nop => {}

Expand All @@ -253,8 +294,17 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
// 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.borrows.iter_enumerated() {
if let ReScope(..) = borrow_data.region {
sets.kill(&borrow_index);
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);
}
}
}
}
}
Expand Down
92 changes: 39 additions & 53 deletions src/librustc_mir/dataflow/impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

use rustc::ty::TyCtxt;
use rustc::mir::{self, Mir, Location};
use rustc_data_structures::bitslice::BitSlice; // adds set_bit/get_bit to &[usize] bitvector rep.
use rustc_data_structures::bitslice::{BitwiseOperator};
use rustc_data_structures::indexed_set::{IdxSet};
use rustc_data_structures::indexed_vec::Idx;
Expand Down Expand Up @@ -504,7 +503,6 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> {
let stmt = &mir[location.block].statements[location.statement_index];
let loc_map = &move_data.loc_map;
let path_map = &move_data.path_map;
let bits_per_block = self.bits_per_block();

match stmt.kind {
// this analysis only tries to find moves explicitly
Expand All @@ -515,21 +513,15 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> {
_ => {
debug!("stmt {:?} at loc {:?} moves out of move_indexes {:?}",
stmt, location, &loc_map[location]);
for move_index in &loc_map[location] {
// Every path deinitialized by a *particular move*
// has corresponding bit, "gen'ed" (i.e. set)
// here, in dataflow vector
zero_to_one(sets.gen_set.words_mut(), *move_index);
}
// Every path deinitialized by a *particular move*
// has corresponding bit, "gen'ed" (i.e. set)
// here, in dataflow vector
sets.gen_all_and_assert_dead(&loc_map[location]);
}
}

for_location_inits(tcx, mir, move_data, location,
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
sets.kill_set.add(&moi);
}
);
|mpi| sets.kill_all(&path_map[mpi]));
}

fn terminator_effect(&self,
Expand All @@ -543,18 +535,10 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MovingOutStatements<'a, 'gcx, 'tcx> {

debug!("terminator {:?} at loc {:?} moves out of move_indexes {:?}",
term, location, &loc_map[location]);
let bits_per_block = self.bits_per_block();
for move_index in &loc_map[location] {
assert!(move_index.index() < bits_per_block);
zero_to_one(sets.gen_set.words_mut(), *move_index);
}
sets.gen_all_and_assert_dead(&loc_map[location]);

for_location_inits(tcx, mir, move_data, location,
|mpi| for moi in &path_map[mpi] {
assert!(moi.index() < bits_per_block);
sets.kill_set.add(&moi);
}
);
|mpi| sets.kill_all(&path_map[mpi]));
}

fn propagate_call_return(&self,
Expand Down Expand Up @@ -585,11 +569,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> {
}

fn start_block_effect(&self, sets: &mut BlockSets<InitIndex>) {
let bits_per_block = self.bits_per_block();
for init_index in (0..self.mir.arg_count).map(InitIndex::new) {
assert!(init_index.index() < bits_per_block);
sets.gen_set.add(&init_index);
}
sets.gen_all((0..self.mir.arg_count).map(InitIndex::new));
}
fn statement_effect(&self,
sets: &mut BlockSets<InitIndex>,
Expand All @@ -599,26 +579,39 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> {
let init_path_map = &move_data.init_path_map;
let init_loc_map = &move_data.init_loc_map;
let rev_lookup = &move_data.rev_lookup;
let bits_per_block = self.bits_per_block();

debug!("statement {:?} at loc {:?} initializes move_indexes {:?}",
stmt, location, &init_loc_map[location]);
for init_index in &init_loc_map[location] {
assert!(init_index.index() < bits_per_block);
sets.gen_set.add(init_index);
}
sets.gen_all(&init_loc_map[location]);

match stmt.kind {
mir::StatementKind::StorageDead(local) => {
// End inits for StorageDead, so that an immutable variable can
// be reinitialized on the next iteration of the loop.
mir::StatementKind::StorageDead(local) |
mir::StatementKind::StorageLive(local) => {
// End inits for StorageDead and StorageLive, so that an immutable
// variable can be reinitialized on the next iteration of the loop.
//
// FIXME(#46525): We *need* to do this for StorageLive as well as
// StorageDead, because lifetimes of match bindings with guards are
// weird - i.e. this code
//
// ```
// fn main() {
// match 0 {
// a | a
// if { println!("a={}", a); false } => {}
// _ => {}
// }
// }
// ```
//
// runs the guard twice, using the same binding for `a`, and only
// storagedeads after everything ends, so if we don't regard the
// storagelive as killing storage, we would have a multiple assignment
// to immutable data error.
if let LookupResult::Exact(mpi) = rev_lookup.find(&mir::Place::Local(local)) {
debug!("stmt {:?} at loc {:?} clears the ever initialized status of {:?}",
stmt, location, &init_path_map[mpi]);
for ii in &init_path_map[mpi] {
assert!(ii.index() < bits_per_block);
sets.kill_set.add(&ii);
}
stmt, location, &init_path_map[mpi]);
sets.kill_all(&init_path_map[mpi]);
}
}
_ => {}
Expand All @@ -634,13 +627,11 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> {
let init_loc_map = &move_data.init_loc_map;
debug!("terminator {:?} at loc {:?} initializes move_indexes {:?}",
term, location, &init_loc_map[location]);
let bits_per_block = self.bits_per_block();
for init_index in &init_loc_map[location] {
if move_data.inits[*init_index].kind != InitKind::NonPanicPathOnly {
assert!(init_index.index() < bits_per_block);
sets.gen_set.add(init_index);
}
}
sets.gen_all(
init_loc_map[location].iter().filter(|init_index| {
move_data.inits[**init_index].kind != InitKind::NonPanicPathOnly
})
);
}

fn propagate_call_return(&self,
Expand All @@ -663,11 +654,6 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedLvals<'a, 'gcx, 'tcx> {
}
}

fn zero_to_one(bitvec: &mut [usize], move_index: MoveOutIndex) {
let retval = bitvec.set_bit(move_index.index());
assert!(retval);
}

impl<'a, 'gcx, 'tcx> BitwiseOperator for MaybeInitializedLvals<'a, 'gcx, 'tcx> {
#[inline]
fn join(&self, pred1: usize, pred2: usize) -> usize {
Expand Down
Loading