From d92287a4e94a5ca729867ae8af146bfaed197542 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 30 Nov 2018 21:22:04 +0000 Subject: [PATCH 1/3] Fix some rustc doc links --- .../transform/cleanup_post_borrowck.rs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/librustc_mir/transform/cleanup_post_borrowck.rs b/src/librustc_mir/transform/cleanup_post_borrowck.rs index c0edd3926d32d..6d7fc404edbb1 100644 --- a/src/librustc_mir/transform/cleanup_post_borrowck.rs +++ b/src/librustc_mir/transform/cleanup_post_borrowck.rs @@ -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; From 4f5b8eace152601d20ac7d4aa3b9c9de0f45dd62 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 30 Nov 2018 21:48:31 +0000 Subject: [PATCH 2/3] Remove the `region_map` field from `BorrowSet` --- src/librustc_mir/borrow_check/borrow_set.rs | 10 +--------- src/librustc_mir/dataflow/impls/borrows.rs | 9 +-------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index fd7dc7fc4bd3a..a6af79ffbe922 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -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; @@ -41,10 +41,6 @@ crate struct BorrowSet<'tcx> { /// only need to store one borrow index crate activation_map: FxHashMap>, - /// Every borrow has a region; this maps each such regions back to - /// its borrow-indexes. - crate region_map: FxHashMap>, - /// Map from local to all the borrows on that local crate local_map: FxHashMap>, @@ -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: @@ -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, } @@ -184,7 +178,6 @@ struct GatherBorrows<'a, 'gcx: 'tcx, 'tcx: 'a> { idx_vec: IndexVec>, location_map: FxHashMap, activation_map: FxHashMap>, - region_map: FxHashMap>, local_map: FxHashMap>, /// When we encounter a 2-phase borrow statement, it will always @@ -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); } diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index 27bc28ac81d84..3a9e4fc9e4ab9 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -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, @@ -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(®ion.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 From 6000c2e0e16ee60078a3e2810ff3d26639bdeff5 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Fri, 30 Nov 2018 21:49:43 +0000 Subject: [PATCH 3/3] Use visit_local to find 2PB activations --- src/librustc_mir/borrow_check/borrow_set.rs | 110 ++++++++++---------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs index a6af79ffbe922..947c32df0f6a3 100644 --- a/src/librustc_mir/borrow_check/borrow_set.rs +++ b/src/librustc_mir/borrow_check/borrow_set.rs @@ -230,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) + } + }; } }