Skip to content

Commit

Permalink
Auto merge of #46268 - arielb1:union-borrow, r=nikomatsakis
Browse files Browse the repository at this point in the history
MIR borrowck: implement union-and-array-compatible semantics

Fixes #44831.
Fixes #44834.
Fixes #45537.
Fixes #45696 (by implementing DerefPure semantics, which is what we want going forward).

r? @nikomatsakis
  • Loading branch information
bors committed Dec 6, 2017
2 parents 833785b + 9d35587 commit cf30759
Show file tree
Hide file tree
Showing 17 changed files with 608 additions and 176 deletions.
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

0 comments on commit cf30759

Please sign in to comment.