Skip to content

Commit

Permalink
Rollup merge of rust-lang#56388 - matthewjasper:more-lexical-mir-clea…
Browse files Browse the repository at this point in the history 
…nup, r=nikomatsakis

More MIR borrow check cleanup

* Fix some rustc doc links
* Remove the `region_map` field from `BorrowSet`
*  Use `visit_local` to find 2PB activations

r? @nikomatsakis
  • Loading branch information
@GuillaumeGomez
GuillaumeGomez committed Dec 5, 2018
2 parents d6f80a2 + 6000c2e commit 4b6011b
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 81 deletions.
120 changes: 56 additions & 64 deletions src/librustc_mir/borrow_check/borrow_set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc::mir::traversal;
use rustc::mir::visit::{
PlaceContext, Visitor, NonUseContext, MutatingUseContext, NonMutatingUseContext
};
use rustc::mir::{self, Location, Mir, Place, Local};
use rustc::mir::{self, Location, Mir, Local};
use rustc::ty::{RegionVid, TyCtxt};
use rustc::util::nodemap::{FxHashMap, FxHashSet};
use rustc_data_structures::indexed_vec::IndexVec;
Expand All @@ -41,10 +41,6 @@ crate struct BorrowSet<'tcx> {
/// only need to store one borrow index
crate activation_map: FxHashMap<Location, Vec<BorrowIndex>>,

/// Every borrow has a region; this maps each such regions back to
/// its borrow-indexes.
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 @@ -149,7 +145,6 @@ impl<'tcx> BorrowSet<'tcx> {
idx_vec: IndexVec::new(),
location_map: Default::default(),
activation_map: Default::default(),
region_map: Default::default(),
local_map: Default::default(),
pending_activations: Default::default(),
locals_state_at_exit:
Expand All @@ -164,7 +159,6 @@ impl<'tcx> BorrowSet<'tcx> {
borrows: visitor.idx_vec,
location_map: visitor.location_map,
activation_map: visitor.activation_map,
region_map: visitor.region_map,
local_map: visitor.local_map,
locals_state_at_exit: visitor.locals_state_at_exit,
}
Expand All @@ -184,7 +178,6 @@ 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<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 @@ -229,7 +222,6 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {

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() {
self.local_map.entry(local).or_default().insert(idx);
}
Expand All @@ -238,68 +230,68 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
self.super_assign(block, assigned_place, rvalue, location)
}

fn visit_place(
fn visit_local(
&mut self,
place: &mir::Place<'tcx>,
temp: &Local,
context: PlaceContext<'tcx>,
location: Location,
) {
self.super_place(place, context, location);

// We found a use of some temporary TEMP...
if let Place::Local(temp) = place {
// ... check whether we (earlier) saw a 2-phase borrow like
//
// TMP = &mut place
if let Some(&borrow_index) = self.pending_activations.get(temp) {
let borrow_data = &mut self.idx_vec[borrow_index];

// Watch out: the use of TMP in the borrow itself
// doesn't count as an activation. =)
if borrow_data.reserve_location == location &&
context == PlaceContext::MutatingUse(MutatingUseContext::Store)
{
return;
}
if !context.is_use() {
return;
}

if let TwoPhaseActivation::ActivatedAt(other_location) =
borrow_data.activation_location {
span_bug!(
self.mir.source_info(location).span,
"found two uses for 2-phase borrow temporary {:?}: \
{:?} and {:?}",
temp,
location,
other_location,
);
}
// We found a use of some temporary TMP
// check whether we (earlier) saw a 2-phase borrow like
//
// TMP = &mut place
if let Some(&borrow_index) = self.pending_activations.get(temp) {
let borrow_data = &mut self.idx_vec[borrow_index];

// Otherwise, this is the unique later use
// that we expect.
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) =>
TwoPhaseActivation::NotActivated,
_ => {
// Double check: This borrow is indeed a two-phase borrow (that is,
// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
// we've not found any other activations (checked above).
assert_eq!(
borrow_data.activation_location,
TwoPhaseActivation::NotActivated,
"never found an activation for this borrow!",
);

self.activation_map
.entry(location)
.or_default()
.push(borrow_index);
TwoPhaseActivation::ActivatedAt(location)
}
};
// Watch out: the use of TMP in the borrow itself
// doesn't count as an activation. =)
if borrow_data.reserve_location == location &&
context == PlaceContext::MutatingUse(MutatingUseContext::Store)
{
return;
}

if let TwoPhaseActivation::ActivatedAt(other_location) =
borrow_data.activation_location {
span_bug!(
self.mir.source_info(location).span,
"found two uses for 2-phase borrow temporary {:?}: \
{:?} and {:?}",
temp,
location,
other_location,
);
}

// Otherwise, this is the unique later use
// that we expect.
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) =>
TwoPhaseActivation::NotActivated,
_ => {
// Double check: This borrow is indeed a two-phase borrow (that is,
// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
// we've not found any other activations (checked above).
assert_eq!(
borrow_data.activation_location,
TwoPhaseActivation::NotActivated,
"never found an activation for this borrow!",
);

self.activation_map
.entry(location)
.or_default()
.push(borrow_index);
TwoPhaseActivation::ActivatedAt(location)
}
};
}
}

Expand Down
9 changes: 1 addition & 8 deletions src/librustc_mir/dataflow/impls/borrows.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
// re-consider the current implementations of the
// propagate_call_return method.

if let mir::Rvalue::Ref(region, _, ref place) = **rhs {
if let mir::Rvalue::Ref(_, _, ref place) = **rhs {
if place.ignore_borrow(
self.tcx,
self.mir,
Expand All @@ -258,13 +258,6 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
panic!("could not find BorrowIndex for location {:?}", location);
});

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
26 changes: 17 additions & 9 deletions src/librustc_mir/transform/cleanup_post_borrowck.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,25 @@

//! This module provides two passes:
//!
//! - [CleanAscribeUserType], that replaces all
//! [StatementKind::AscribeUserType] statements with [StatementKind::Nop].
//! - [CleanFakeReadsAndBorrows], that replaces all [FakeRead] statements and
//! borrows that are read by [FakeReadCause::ForMatchGuard] fake reads with
//! [StatementKind::Nop].
//! - [`CleanAscribeUserType`], that replaces all [`AscribeUserType`]
//! statements with [`Nop`].
//! - [`CleanFakeReadsAndBorrows`], that replaces all [`FakeRead`] statements
//! and borrows that are read by [`ForMatchGuard`] fake reads with [`Nop`].
//!
//! The [CleanFakeReadsAndBorrows] "pass" is actually implemented as two
//! The `CleanFakeReadsAndBorrows` "pass" is actually implemented as two
//! traversals (aka visits) of the input MIR. The first traversal,
//! [DeleteAndRecordFakeReads], deletes the fake reads and finds the temporaries
//! read by [ForMatchGuard] reads, and [DeleteFakeBorrows] deletes the
//! initialization of those temporaries.
//! [`DeleteAndRecordFakeReads`], deletes the fake reads and finds the
//! temporaries read by [`ForMatchGuard`] reads, and [`DeleteFakeBorrows`]
//! deletes the initialization of those temporaries.
//!
//! [`CleanAscribeUserType`]: cleanup_post_borrowck::CleanAscribeUserType
//! [`CleanFakeReadsAndBorrows`]: cleanup_post_borrowck::CleanFakeReadsAndBorrows
//! [`DeleteAndRecordFakeReads`]: cleanup_post_borrowck::DeleteAndRecordFakeReads
//! [`DeleteFakeBorrows`]: cleanup_post_borrowck::DeleteFakeBorrows
//! [`AscribeUserType`]: rustc::mir::StatementKind::AscribeUserType
//! [`Nop`]: rustc::mir::StatementKind::Nop
//! [`FakeRead`]: rustc::mir::StatementKind::FakeRead
//! [`ForMatchGuard`]: rustc::mir::FakeReadCause::ForMatchGuard

use rustc_data_structures::fx::FxHashSet;

Expand Down

0 comments on commit 4b6011b

Please sign in to comment.