diff options
| -rw-r--r-- | src/librustc_mir/borrow_check/error_reporting.rs | 6 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/flows.rs | 12 | ||||
| -rw-r--r-- | src/librustc_mir/borrow_check/mod.rs | 76 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/at_location.rs | 6 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/impls/borrows.rs | 458 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/mod.rs | 142 | ||||
| -rw-r--r-- | src/librustc_mir/dataflow/move_paths/mod.rs | 3 |
7 files changed, 552 insertions, 151 deletions
diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 1d5e09b164a..8f07c73b5ee 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -19,7 +19,7 @@ use std::rc::Rc; use super::{MirBorrowckCtxt, Context}; use super::{InitializationRequiringAction, PrefixSet}; -use dataflow::{BorrowData, Borrows, FlowAtLocation, MovingOutStatements}; +use dataflow::{ActiveBorrows, BorrowData, FlowAtLocation, MovingOutStatements}; use dataflow::move_paths::MovePathIndex; use util::borrowck_errors::{BorrowckErrors, Origin}; @@ -324,10 +324,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { _: Context, borrow: &BorrowData<'tcx>, drop_span: Span, - borrows: &Borrows<'cx, 'gcx, 'tcx> + borrows: &ActiveBorrows<'cx, 'gcx, 'tcx> ) { let end_span = borrows.opt_region_end_span(&borrow.region); - let scope_tree = borrows.scope_tree(); + let scope_tree = borrows.0.scope_tree(); let root_place = self.prefixes(&borrow.borrowed_place, PrefixSet::All).last().unwrap(); match root_place { diff --git a/src/librustc_mir/borrow_check/flows.rs b/src/librustc_mir/borrow_check/flows.rs index 449062989e0..69a08c7a30d 100644 --- a/src/librustc_mir/borrow_check/flows.rs +++ b/src/librustc_mir/borrow_check/flows.rs @@ -17,13 +17,13 @@ use rustc::mir::{BasicBlock, Location}; use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals}; use dataflow::{EverInitializedLvals, MovingOutStatements}; -use dataflow::{Borrows, FlowAtLocation, FlowsAtLocation}; +use dataflow::{ActiveBorrows, FlowAtLocation, FlowsAtLocation}; use dataflow::move_paths::HasMoveData; use std::fmt; // (forced to be `pub` due to its use as an associated type below.) -pub struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> { - pub borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>, +pub(crate) struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> { + pub borrows: FlowAtLocation<ActiveBorrows<'b, 'gcx, 'tcx>>, pub inits: FlowAtLocation<MaybeInitializedLvals<'b, 'gcx, 'tcx>>, pub uninits: FlowAtLocation<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>, pub move_outs: FlowAtLocation<MovingOutStatements<'b, 'gcx, 'tcx>>, @@ -32,7 +32,7 @@ pub struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> { impl<'b, 'gcx, 'tcx> Flows<'b, 'gcx, 'tcx> { pub fn new( - borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>, + borrows: FlowAtLocation<ActiveBorrows<'b, 'gcx, 'tcx>>, inits: FlowAtLocation<MaybeInitializedLvals<'b, 'gcx, 'tcx>>, uninits: FlowAtLocation<MaybeUninitializedLvals<'b, 'gcx, 'tcx>>, move_outs: FlowAtLocation<MovingOutStatements<'b, 'gcx, 'tcx>>, @@ -87,7 +87,7 @@ impl<'b, 'gcx, 'tcx> fmt::Display for Flows<'b, 'gcx, 'tcx> { s.push_str(", "); }; saw_one = true; - let borrow_data = &self.borrows.operator().borrows()[borrow]; + let borrow_data = &self.borrows.operator().borrows()[borrow.borrow_index()]; s.push_str(&format!("{}", borrow_data)); }); s.push_str("] "); @@ -99,7 +99,7 @@ impl<'b, 'gcx, 'tcx> fmt::Display for Flows<'b, 'gcx, 'tcx> { s.push_str(", "); }; saw_one = true; - let borrow_data = &self.borrows.operator().borrows()[borrow]; + let borrow_data = &self.borrows.operator().borrows()[borrow.borrow_index()]; s.push_str(&format!("{}", borrow_data)); }); s.push_str("] "); diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 98f974aba50..0be250eb96c 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -30,11 +30,12 @@ use syntax_pos::Span; use dataflow::{do_dataflow, DebugFormatted}; use dataflow::MoveDataParamEnv; -use dataflow::DataflowResultsConsumer; +use dataflow::{DataflowAnalysis, DataflowResultsConsumer}; use dataflow::{FlowAtLocation, FlowsAtLocation}; use dataflow::{MaybeInitializedLvals, MaybeUninitializedLvals}; use dataflow::{EverInitializedLvals, MovingOutStatements}; -use dataflow::{BorrowData, BorrowIndex, Borrows}; +use dataflow::{Borrows, BorrowData, ReserveOrActivateIndex}; +use dataflow::{ActiveBorrows, Reservations}; use dataflow::move_paths::{IllegalMoveOriginKind, MoveError}; use dataflow::move_paths::{HasMoveData, LookupResult, MoveData, MovePathIndex}; use util::borrowck_errors::{BorrowckErrors, Origin}; @@ -48,6 +49,9 @@ use self::MutateMode::{JustWrite, WriteAndRead}; mod error_reporting; mod flows; mod prefixes; + +use std::borrow::Cow; + pub(crate) mod nll; pub fn provide(providers: &mut Providers) { @@ -205,23 +209,6 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( }; let flow_inits = flow_inits; // remove mut - let flow_borrows = FlowAtLocation::new(do_dataflow( - tcx, - mir, - id, - &attributes, - &dead_unwinds, - Borrows::new(tcx, mir, opt_regioncx, def_id, body_id), - |bd, i| DebugFormatted::new(bd.location(i)), - )); - - let mut state = Flows::new( - flow_borrows, - flow_inits, - flow_uninits, - flow_move_outs, - flow_ever_inits, - ); let mut mbcx = MirBorrowckCtxt { tcx: tcx, mir: mir, @@ -237,6 +224,44 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>( storage_dead_or_drop_error_reported_s: FxHashSet(), }; + let borrows = Borrows::new(tcx, mir, opt_regioncx, def_id, body_id); + let flow_reservations = do_dataflow( + tcx, + mir, + id, + &attributes, + &dead_unwinds, + Reservations::new(borrows), + |rs, i| { + // In principle we could make the dataflow ensure that + // only reservation bits show up, and assert so here. + // + // In practice it is easier to be looser; in particular, + // it is okay for the kill-sets to hold activation bits. + DebugFormatted::new(&(i.kind(), rs.location(i))) + }); + let flow_active_borrows = { + let reservations_on_entry = flow_reservations.0.sets.entry_set_state(); + let reservations = flow_reservations.0.operator; + let a = DataflowAnalysis::new_with_entry_sets(mir, + &dead_unwinds, + Cow::Borrowed(reservations_on_entry), + ActiveBorrows::new(reservations)); + let results = a.run(tcx, + id, + &attributes, + |ab, i| DebugFormatted::new(&(i.kind(), ab.location(i)))); + FlowAtLocation::new(results) + }; + + let mut state = Flows::new( + flow_active_borrows, + flow_inits, + flow_uninits, + flow_move_outs, + flow_ever_inits, + ); + mbcx.analyze_results(&mut state); // entry point for DataflowResultsConsumer opt_closure_req @@ -504,9 +529,8 @@ impl<'cx, 'gcx, 'tcx> DataflowResultsConsumer<'cx, 'tcx> for MirBorrowckCtxt<'cx let data = domain.borrows(); flow_state.borrows.with_elems_outgoing(|borrows| { for i in borrows { - let borrow = &data[i]; + let borrow = &data[i.borrow_index()]; let context = ContextKind::StorageDead.new(loc); - self.check_for_invalidation_at_exit(context, borrow, span, flow_state); } }); @@ -721,7 +745,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { WriteKind::StorageDeadOrDrop => { error_reported = true; this.report_borrowed_value_does_not_live_long_enough( - context, borrow, place_span.1, flow_state.borrows.operator()); + context, borrow, place_span.1, + flow_state.borrows.operator()); } WriteKind::Mutate => { error_reported = true; @@ -1778,7 +1803,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { flow_state: &Flows<'cx, 'gcx, 'tcx>, mut op: F, ) where - F: FnMut(&mut Self, BorrowIndex, &BorrowData<'tcx>) -> Control, + F: FnMut(&mut Self, ReserveOrActivateIndex, &BorrowData<'tcx>) -> Control, { let (access, place) = access_place; @@ -1790,7 +1815,10 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // check for loan restricting path P being used. Accounts for // borrows of P, P.a.b, etc. for i in flow_state.borrows.elems_incoming() { - let borrowed = &data[i]; + // FIXME for now, just skip the activation state. + if i.is_activation() { continue } + + let borrowed = &data[i.borrow_index()]; if self.places_conflict(&borrowed.borrowed_place, place, access) { let ctrl = op(self, i, borrowed); diff --git a/src/librustc_mir/dataflow/at_location.rs b/src/librustc_mir/dataflow/at_location.rs index 90ebb45123e..fefb378fc75 100644 --- a/src/librustc_mir/dataflow/at_location.rs +++ b/src/librustc_mir/dataflow/at_location.rs @@ -121,9 +121,8 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD> fn reconstruct_statement_effect(&mut self, loc: Location) { self.stmt_gen.reset_to_empty(); self.stmt_kill.reset_to_empty(); - let mut ignored = IdxSetBuf::new_empty(0); let mut sets = BlockSets { - on_entry: &mut ignored, + on_entry: &mut self.curr_state, gen_set: &mut self.stmt_gen, kill_set: &mut self.stmt_kill, }; @@ -135,9 +134,8 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD> fn reconstruct_terminator_effect(&mut self, loc: Location) { self.stmt_gen.reset_to_empty(); self.stmt_kill.reset_to_empty(); - let mut ignored = IdxSetBuf::new_empty(0); let mut sets = BlockSets { - on_entry: &mut ignored, + on_entry: &mut self.curr_state, gen_set: &mut self.stmt_gen, kill_set: &mut self.stmt_kill, }; diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index e6a682e1be9..abfc3159cff 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -11,8 +11,8 @@ 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::mir::{self, Location, Place, Mir}; +use rustc::mir::visit::{PlaceContext, Visitor}; use rustc::ty::{self, Region, TyCtxt}; use rustc::ty::RegionKind; use rustc::ty::RegionKind::ReScope; @@ -20,16 +20,17 @@ use rustc::util::nodemap::{FxHashMap, FxHashSet}; use rustc_data_structures::bitslice::{BitwiseOperator}; use rustc_data_structures::indexed_set::{IdxSet}; -use rustc_data_structures::indexed_vec::{IndexVec}; +use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use dataflow::{BitDenotation, BlockSets, InitialFlow}; -pub use dataflow::indexes::BorrowIndex; +pub use dataflow::indexes::{BorrowIndex, ReserveOrActivateIndex}; use borrow_check::nll::region_infer::RegionInferenceContext; use borrow_check::nll::ToRegionVid; use syntax_pos::Span; use std::fmt; +use std::hash::Hash; use std::rc::Rc; // `Borrows` maps each dataflow bit to an `Rvalue::Ref`, which can be @@ -42,12 +43,47 @@ pub struct Borrows<'a, 'gcx: 'tcx, 'tcx: 'a> { root_scope: Option<region::Scope>, borrows: IndexVec<BorrowIndex, BorrowData<'tcx>>, location_map: FxHashMap<Location, BorrowIndex>, + assigned_map: FxHashMap<Place<'tcx>, FxHashSet<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>>, } +// Two-phase borrows actually requires two flow analyses; they need +// to be separate because the final results of the first are used to +// construct the gen+kill sets for the second. (The dataflow system +// is not designed to allow the gen/kill sets to change during the +// fixed-point iteration.) + +/// The `Reservations` analysis is the first of the two flow analyses +/// tracking (phased) borrows. It computes where a borrow is reserved; +/// i.e. where it can reach in the control flow starting from its +/// initial `assigned = &'rgn borrowed` statement, and ending +/// whereever `'rgn` itself ends. +pub(crate) struct Reservations<'a, 'gcx: 'tcx, 'tcx: 'a>(pub(crate) Borrows<'a, 'gcx, 'tcx>); + +/// The `ActiveBorrows` analysis is the second of the two flow +/// analyses tracking (phased) borrows. It computes where any given +/// borrow `&assigned = &'rgn borrowed` is *active*, which starts at +/// the first use of `assigned` after the reservation has started, and +/// ends whereever `'rgn` itself ends. +pub(crate) struct ActiveBorrows<'a, 'gcx: 'tcx, 'tcx: 'a>(pub(crate) Borrows<'a, 'gcx, 'tcx>); + +impl<'a, 'gcx, 'tcx> Reservations<'a, 'gcx, 'tcx> { + pub(crate) fn new(b: Borrows<'a, 'gcx, 'tcx>) -> Self { Reservations(b) } + pub(crate) fn location(&self, idx: ReserveOrActivateIndex) -> &Location { + self.0.location(idx.borrow_index()) + } +} + +impl<'a, 'gcx, 'tcx> ActiveBorrows<'a, 'gcx, 'tcx> { + pub(crate) fn new(r: Reservations<'a, 'gcx, 'tcx>) -> Self { ActiveBorrows(r.0) } + pub(crate) fn location(&self, idx: ReserveOrActivateIndex) -> &Location { + self.0.location(idx.borrow_index()) + } +} + // temporarily allow some dead fields: `kind` and `region` will be // needed by borrowck; `borrowed_place` will probably be a MovePathIndex when // that is extended to include borrowed data paths. @@ -58,6 +94,7 @@ pub struct BorrowData<'tcx> { pub(crate) kind: mir::BorrowKind, pub(crate) region: Region<'tcx>, pub(crate) borrowed_place: mir::Place<'tcx>, + pub(crate) assigned_place: mir::Place<'tcx>, } impl<'tcx> fmt::Display for BorrowData<'tcx> { @@ -73,6 +110,21 @@ impl<'tcx> fmt::Display for BorrowData<'tcx> { } } +impl ReserveOrActivateIndex { + fn reserved(i: BorrowIndex) -> Self { ReserveOrActivateIndex::new((i.index() * 2)) } + fn active(i: BorrowIndex) -> Self { ReserveOrActivateIndex::new((i.index() * 2) + 1) } + + pub(crate) fn is_reservation(self) -> bool { self.index() % 2 == 0 } + pub(crate) fn is_activation(self) -> bool { self.index() % 2 == 1} + + pub(crate) fn kind(self) -> &'static str { + if self.is_reservation() { "reserved" } else { "active" } + } + pub(crate) fn borrow_index(self) -> BorrowIndex { + BorrowIndex::new(self.index() / 2) + } +} + impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>, mir: &'a Mir<'tcx>, @@ -89,6 +141,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { mir, idx_vec: IndexVec::new(), location_map: FxHashMap(), + assigned_map: FxHashMap(), region_map: FxHashMap(), local_map: FxHashMap(), region_span_map: FxHashMap() @@ -100,6 +153,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { scope_tree, root_scope, location_map: visitor.location_map, + assigned_map: visitor.assigned_map, region_map: visitor.region_map, local_map: visitor.local_map, region_span_map: visitor.region_span_map, @@ -110,13 +164,16 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { mir: &'a Mir<'tcx>, idx_vec: IndexVec<BorrowIndex, BorrowData<'tcx>>, location_map: FxHashMap<Location, BorrowIndex>, + assigned_map: FxHashMap<Place<'tcx>, FxHashSet<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, + fn visit_assign(&mut self, + block: mir::BasicBlock, + assigned_place: &mir::Place<'tcx>, rvalue: &mir::Rvalue<'tcx>, location: mir::Location) { fn root_local(mut p: &mir::Place<'_>) -> Option<mir::Local> { @@ -127,23 +184,59 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { }} } - if let mir::Rvalue::Ref(region, kind, ref place) = *rvalue { - if is_unsafe_place(self.tcx, self.mir, place) { return; } + if let mir::Rvalue::Ref(region, kind, ref borrowed_place) = *rvalue { + if is_unsafe_place(self.tcx, self.mir, borrowed_place) { return; } let borrow = BorrowData { - location, kind, region, borrowed_place: place.clone(), + location, kind, region, + borrowed_place: borrowed_place.clone(), + assigned_place: assigned_place.clone(), }; 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); + insert(&mut self.assigned_map, assigned_place, idx); + insert(&mut self.region_map, ®ion, idx); + if let Some(local) = root_local(borrowed_place) { + insert(&mut self.local_map, &local, idx); + } + } + + return self.super_assign(block, assigned_place, rvalue, location); - if let Some(local) = root_local(place) { - let borrows = self.local_map.entry(local).or_insert(FxHashSet()); - borrows.insert(idx); + fn insert<'a, K, V>(map: &'a mut FxHashMap<K, FxHashSet<V>>, + k: &K, + v: V) + where K: Clone+Eq+Hash, V: Eq+Hash + { + map.entry(k.clone()) + .or_insert(FxHashSet()) + .insert(v); + } + } + + fn visit_rvalue(&mut self, + rvalue: &mir::Rvalue<'tcx>, + location: mir::Location) { + if let mir::Rvalue::Ref(region, kind, ref place) = *rvalue { + // double-check that we already registered a BorrowData for this + + let mut found_it = false; + for idx in &self.region_map[region] { + let bd = &self.idx_vec[*idx]; + if bd.location == location && + bd.kind == kind && + bd.region == region && + bd.borrowed_place == *place + { + found_it = true; + break; + } } + assert!(found_it, "Ref {:?} at {:?} missing BorrowData", rvalue, location); } + + return self.super_rvalue(rvalue, location); } fn visit_statement(&mut self, @@ -153,7 +246,7 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { if let mir::StatementKind::EndRegion(region_scope) = statement.kind { self.region_span_map.insert(ReScope(region_scope), statement.source_info.span); } - self.super_statement(block, statement, location); + return self.super_statement(block, statement, location); } } } @@ -166,26 +259,19 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { &self.borrows[idx].location } - /// Returns the span for the "end point" given region. This will - /// return `None` if NLL is enabled, since that concept has no - /// meaning there. Otherwise, return region span if it exists and - /// span for end of the function if it doesn't exist. - pub fn opt_region_end_span(&self, region: &Region) -> Option<Span> { - match self.nonlexical_regioncx { - Some(_) => None, - None => { - match self.region_span_map.get(region) { - Some(span) => Some(span.end_point()), - None => Some(self.mir.span.end_point()) - } - } - } - } - /// Add all borrows to the kill set, if those borrows are out of scope at `location`. + /// + /// `is_activations` tracks whether we are in the Reservations or + /// the ActiveBorrows flow analysis, and does not set the + /// activation kill bits in the former case. (Technically, we + /// could set those kill bits without such a guard, since they are + /// never gen'ed by Reservations in the first place. But it makes + /// the instrumentation and graph renderings nicer to leave + /// activations out when of the Reservations kill sets.) fn kill_loans_out_of_scope_at_location(&self, - sets: &mut BlockSets<BorrowIndex>, - location: Location) { + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location, + is_activations: bool) { if let Some(ref regioncx) = self.nonlexical_regioncx { for (borrow_index, borrow_data) in self.borrows.iter_enumerated() { let borrow_region = borrow_data.region.to_region_vid(); @@ -201,45 +287,74 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> { // mismatch here by not generating a kill for the // location on the borrow itself. if location != borrow_data.location { - sets.kill(&borrow_index); + sets.kill(&ReserveOrActivateIndex::reserved(borrow_index)); + } + + // FIXME: the logic used to justify the above + // "accounting for mismatch" does not generalize + // to activations, so we set the kill-bits without + // that same location check here. + // + // But... can we get into a situation where the + // gen/kill bits are both sets in this case, in + // which case we *do* need an analogous guard of + // some kind? + if is_activations { + sets.kill(&ReserveOrActivateIndex::active(borrow_index)); } } } } } -} -impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { - type Idx = BorrowIndex; - fn name() -> &'static str { "borrows" } - fn bits_per_block(&self) -> usize { - self.borrows.len() - } - fn start_block_effect(&self, _sets: &mut IdxSet<BorrowIndex>) { - // no borrows of code region_scopes have been taken prior to - // function execution, so this method has no effect on - // `_sets`. - } - fn statement_effect(&self, - sets: &mut BlockSets<BorrowIndex>, - location: Location) { + /// Models statement effect in Reservations and ActiveBorrows flow + /// analyses; `is activations` tells us if we are in the latter + /// case. + fn statement_effect_on_borrows(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location, + is_activations: bool) { let block = &self.mir.basic_blocks().get(location.block).unwrap_or_else(|| { panic!("could not find block at location {:?}", location); }); let stmt = block.statements.get(location.statement_index).unwrap_or_else(|| { panic!("could not find statement at location {:?}"); }); + + if is_activations { + // INVARIANT: At this point, `sets.on_entry` should + // correctly reflect the reservations as we enter the + // statement (because accumulates_intrablock_state is + // overridden) + // + // Now compute effect of the statement on the activations + // themselves in the ActiveBorrows state. + let mut find = FindPlaceUses { sets, assigned_map: &self.assigned_map }; + find.visit_statement(location.block, stmt, location); + } + match stmt.kind { + // EndRegion kills any borrows (reservations and active borrows both) mir::StatementKind::EndRegion(region_scope) => { if let Some(borrow_indexes) = self.region_map.get(&ReScope(region_scope)) { assert!(self.nonlexical_regioncx.is_none()); - sets.kill_all(borrow_indexes); + for idx in borrow_indexes { + sets.kill(&ReserveOrActivateIndex::reserved(*idx)); + if is_activations { + sets.kill(&ReserveOrActivateIndex::active(*idx)); + } + } } else { // (if there is no entry, then there are no borrows to be tracked) } } mir::StatementKind::Assign(_, ref rhs) => { + // NOTE: if/when the Assign case is revised to inspect + // the assigned_place here, make sure to also + // re-consider the current implementations of the + // propagate_call_return method. + if let mir::Rvalue::Ref(region, _, ref place) = *rhs { if is_unsafe_place(self.tcx, self.mir, place) { return; } if let RegionKind::ReEmpty = region { @@ -254,7 +369,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { assert!(self.region_map.get(region).unwrap_or_else(|| { panic!("could not find BorrowIndexs for region {:?}", region); }).contains(&index)); - sets.gen(&index); + sets.gen(&ReserveOrActivateIndex::reserved(*index)); } } @@ -264,7 +379,12 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { // // FIXME: expand this to variables that are assigned over. if let Some(borrow_indexes) = self.local_map.get(&local) { - sets.kill_all(borrow_indexes); + sets.kill_all(borrow_indexes.iter() + .map(|b| ReserveOrActivateIndex::reserved(*b))); + if is_activations { + sets.kill_all(borrow_indexes.iter() + .map(|b| ReserveOrActivateIndex::active(*b))); + } } } @@ -276,16 +396,28 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { } - self.kill_loans_out_of_scope_at_location(sets, location); + self.kill_loans_out_of_scope_at_location(sets, location, is_activations); } - fn terminator_effect(&self, - sets: &mut BlockSets<BorrowIndex>, - location: Location) { + /// Models terminator effect in Reservations and ActiveBorrows + /// flow analyses; `is activations` tells us if we are in the + /// latter case. + fn terminator_effect_on_borrows(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location, + is_activations: bool) { let block = &self.mir.basic_blocks().get(location.block).unwrap_or_else(|| { panic!("could not find block at location {:?}", location); }); - match block.terminator().kind { + + let term = block.terminator(); + if is_activations { + // Any uses of reserved Places in the statement are now activated. + let mut find = FindPlaceUses { sets, assigned_map: &self.assigned_map }; + find.visit_terminator(location.block, term, location); + } + + match term.kind { mir::TerminatorKind::Resume | mir::TerminatorKind::Return | mir::TerminatorKind::GeneratorDrop => { @@ -304,7 +436,10 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { if *scope != root_scope && self.scope_tree.is_subscope_of(*scope, root_scope) { - sets.kill(&borrow_index); + sets.kill(&ReserveOrActivateIndex::reserved(borrow_index)); + if is_activations { + sets.kill(&ReserveOrActivateIndex::active(borrow_index)); + } } } } @@ -320,29 +455,224 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> { mir::TerminatorKind::FalseEdges {..} | mir::TerminatorKind::Unreachable => {} } - self.kill_loans_out_of_scope_at_location(sets, location); + self.kill_loans_out_of_scope_at_location(sets, location, is_activations); + } +} + +impl<'a, 'gcx, 'tcx> ActiveBorrows<'a, 'gcx, 'tcx> { + pub(crate) fn borrows(&self) -> &IndexVec<BorrowIndex, BorrowData<'tcx>> { + self.0.borrows() + } + + /// Returns the span for the "end point" given region. This will + /// return `None` if NLL is enabled, since that concept has no + /// meaning there. Otherwise, return region span if it exists and + /// span for end of the function if it doesn't exist. + pub(crate) fn opt_region_end_span(&self, region: &Region) -> Option<Span> { + match self.0.nonlexical_regioncx { + Some(_) => None, + None => { + match self.0.region_span_map.get(region) { + Some(span) => Some(span.end_point()), + None => Some(self.0.mir.span.end_point()) + } + } + } + } +} + +/// `FindPlaceUses` is a MIR visitor that updates `self.sets` for all +/// of the borrows activated by a given statement or terminator. +/// +/// ---- +/// +/// The `ActiveBorrows` flow analysis, when inspecting any given +/// statement or terminator, needs to "generate" (i.e. set to 1) all +/// of the bits for the borrows that are activated by that +/// statement/terminator. +/// +/// This struct will seek out all places that are assignment-targets +/// for borrows (gathered in `self.assigned_map`; see also the +/// `assigned_map` in `struct Borrows`), and set the corresponding +/// gen-bits for activations of those borrows in `self.sets` +struct FindPlaceUses<'a, 'b: 'a, 'tcx: 'a> { + assigned_map: &'a FxHashMap<Place<'tcx>, FxHashSet<BorrowIndex>>, + sets: &'a mut BlockSets<'b, ReserveOrActivateIndex>, +} + +impl<'a, 'b, 'tcx> FindPlaceUses<'a, 'b, 'tcx> { + fn has_been_reserved(&self, b: &BorrowIndex) -> bool { + self.sets.on_entry.contains(&ReserveOrActivateIndex::reserved(*b)) + } + + /// return whether `context` should be considered a "use" of a + /// place found in that context. "Uses" activate associated + /// borrows (at least when such uses occur while the borrow also + /// has a reservation at the time). + fn is_potential_use(context: PlaceContext) -> bool { + match context { + // storage effects on an place do not activate it + PlaceContext::StorageLive | PlaceContext::StorageDead => false, + + // validation effects do not activate an place + // + // FIXME: Should they? Is it just another read? Or can we + // guarantee it won't dereference the stored address? How + // "deep" does validation go? + PlaceContext::Validate => false, + + // pure overwrites of an place do not activate it. (note + // PlaceContext::Call is solely about dest place) + PlaceContext::Store | PlaceContext::Call => false, + + // reads of an place *do* activate it + PlaceContext::Move | + PlaceContext::Copy | + PlaceContext::Drop | + PlaceContext::Inspect | + PlaceContext::Borrow { .. } | + PlaceContext::Projection(..) => true, + } + } +} + +impl<'a, 'b, 'tcx> Visitor<'tcx> for FindPlaceUses<'a, 'b, 'tcx> { + fn visit_place(&mut self, + place: &mir::Place<'tcx>, + context: PlaceContext<'tcx>, + location: Location) { + debug!("FindPlaceUses place: {:?} assigned from borrows: {:?} \ + used in context: {:?} at location: {:?}", + place, self.assigned_map.get(place), context, location); + if Self::is_potential_use(context) { + if let Some(borrows) = self.assigned_map.get(place) { + for borrow_idx in borrows { + debug!("checking if index {:?} for {:?} is reserved ({}) \ + and thus needs active gen-bit set in sets {:?}", + borrow_idx, place, self.has_been_reserved(&borrow_idx), self.sets); + if self.has_been_reserved(&borrow_idx) { + self.sets.gen(&ReserveOrActivateIndex::active(*borrow_idx)); + } else { + // (This can certainly happen in valid code. I + // just want to know about it in the short + // term.) + debug!("encountered use of Place {:?} of borrow_idx {:?} \ + at location {:?} outside of reservation", + place, borrow_idx, location); + } + } + } + } + + self.super_place(place, context, location); + } +} + + +impl<'a, 'gcx, 'tcx> BitDenotation for Reservations<'a, 'gcx, 'tcx> { + type Idx = ReserveOrActivateIndex; + fn name() -> &'static str { "reservations" } + fn bits_per_block(&self) -> usize { + self.0.borrows.len() * 2 + } + fn start_block_effect(&self, _entry_set: &mut IdxSet<ReserveOrActivateIndex>) { + // no borrows of code region_scopes have been taken prior to + // function execution, so this method has no effect on + // `_sets`. + } + + fn statement_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("Reservations::statement_effect sets: {:?} location: {:?}", sets, location); + self.0.statement_effect_on_borrows(sets, location, false); + } + + fn terminator_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("Reservations::terminator_effect sets: {:?} location: {:?}", sets, location); + self.0.terminator_effect_on_borrows(sets, location, false); } fn propagate_call_return(&self, - _in_out: &mut IdxSet<BorrowIndex>, + _in_out: &mut IdxSet<ReserveOrActivateIndex>, _call_bb: mir::BasicBlock, _dest_bb: mir::BasicBlock, _dest_place: &mir::Place) { - // there are no effects on the region scopes from method calls. + // there are no effects on borrows from method call return... + // + // ... but if overwriting a place can affect flow state, then + // latter is not true; see NOTE on Assign case in + // statement_effect_on_borrows. + } +} + +impl<'a, 'gcx, 'tcx> BitDenotation for ActiveBorrows<'a, 'gcx, 'tcx> { + type Idx = ReserveOrActivateIndex; + fn name() -> &'static str { "active_borrows" } + + /// Overriding this method; `ActiveBorrows` uses the intrablock + /// state in `on_entry` to track the current reservations (which + /// then affect the construction of the gen/kill sets for + /// activations). + fn accumulates_intrablock_state() -> bool { true } + + fn bits_per_block(&self) -> usize { + self.0.borrows.len() * 2 + } + + fn start_block_effect(&self, _entry_sets: &mut IdxSet<ReserveOrActivateIndex>) { + // no borrows of code region_scopes have been taken prior to + // function execution, so this method has no effect on + // `_sets`. + } + + fn statement_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("ActiveBorrows::statement_effect sets: {:?} location: {:?}", sets, location); + self.0.statement_effect_on_borrows(sets, location, true); + } + + fn terminator_effect(&self, + sets: &mut BlockSets<ReserveOrActivateIndex>, + location: Location) { + debug!("ActiveBorrows::terminator_effect sets: {:?} location: {:?}", sets, location); + self.0.terminator_effect_on_borrows(sets, location, true); + } + + fn propagate_call_return(&self, + _in_out: &mut IdxSet<ReserveOrActivateIndex>, + _call_bb: mir::BasicBlock, + _dest_bb: mir::BasicBlock, + _dest_place: &mir::Place) { + // there are no effects on borrows from method call return... + // + // ... but If overwriting a place can affect flow state, then + // latter is not true; see NOTE on Assign case in + // statement_effect_on_borrows. + } +} + +impl<'a, 'gcx, 'tcx> BitwiseOperator for Reservations<'a, 'gcx, 'tcx> { + #[inline] + fn join(&self, pred1: usize, pred2: usize) -> usize { + pred1 | pred2 // union effects of preds when computing reservations } } -impl<'a, 'gcx, 'tcx> BitwiseOperator for Borrows<'a, 'gcx, 'tcx> { +impl<'a, 'gcx, 'tcx> BitwiseOperator for ActiveBorrows<'a, 'gcx, 'tcx> { #[inline] fn join(&self, pred1: usize, pred2: usize) -> usize { - pred1 | pred2 // union effects of preds when computing borrows + pred1 | pred2 // union effects of preds when computing activations } } -impl<'a, 'gcx, 'tcx> InitialFlow for Borrows<'a, 'gcx, 'tcx> { +impl<'a, 'gcx, 'tcx> InitialFlow for Reservations<'a, 'gcx, 'tcx> { #[inline] fn bottom_value() -> bool { - false // bottom = no Rvalue::Refs are active by default + false // bottom = no Rvalue::Refs are reserved by default } } diff --git a/src/librustc_mir/dataflow/mod.rs b/src/librustc_mir/dataflow/mod.rs index f7efd7fe7da..83c46e0199e 100644 --- a/src/librustc_mir/dataflow/mod.rs +++ b/src/librustc_mir/dataflow/mod.rs @@ -18,7 +18,7 @@ use rustc::ty::{self, TyCtxt}; use rustc::mir::{self, Mir, BasicBlock, BasicBlockData, Location, Statement, Terminator}; use rustc::session::Session; -use std::borrow::Borrow; +use std::borrow::{Borrow, Cow}; use std::fmt; use std::io; use std::mem; @@ -29,7 +29,8 @@ pub use self::impls::{MaybeStorageLive}; pub use self::impls::{MaybeInitializedLvals, MaybeUninitializedLvals}; pub use self::impls::{DefinitelyInitializedLvals, MovingOutStatements}; pub use self::impls::EverInitializedLvals; -pub use self::impls::borrows::{Borrows, BorrowData, BorrowIndex}; +pub use self::impls::borrows::{Borrows, BorrowData}; +pub(crate) use self::impls::borrows::{ActiveBorrows, Reservations, ReserveOrActivateIndex}; pub use self::at_location::{FlowAtLocation, FlowsAtLocation}; pub(crate) use self::drop_flag_effects::*; @@ -120,7 +121,7 @@ pub struct MoveDataParamEnv<'gcx, 'tcx> { } pub(crate) fn do_dataflow<'a, 'gcx, 'tcx, BD, P>(tcx: TyCtxt<'a, 'gcx, 'tcx>, - mir: &Mir<'tcx>, + mir: &'a Mir<'tcx>, node_id: ast::NodeId, attributes: &[ast::Attribute], dead_unwinds: &IdxSet<BasicBlock>, @@ -130,34 +131,46 @@ pub(crate) fn do_dataflow<'a, 'gcx, 'tcx, BD, P>(tcx: TyCtxt<'a, 'gcx, 'tcx>, where BD: BitDenotation + InitialFlow, P: Fn(&BD, BD::Idx) -> DebugFormatted { - let name_found = |sess: &Session, attrs: &[ast::Attribute], name| -> Option<String> { - if let Some(item) = has_rustc_mir_with(attrs, name) { - if let Some(s) = item.value_str() { - return Some(s.to_string()) - } else { - sess.span_err( - item.span, - &format!("{} attribute requires a path", item.name())); - return None; + let flow_state = DataflowAnalysis::new(mir, dead_unwinds, bd); + flow_state.run(tcx, node_id, attributes, p) +} + +impl<'a, 'gcx: 'tcx, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation +{ + pub(crate) fn run<P>(self, + tcx: TyCtxt<'a, 'gcx, 'tcx>, + node_id: ast::NodeId, + attributes: &[ast::Attribute], + p: P) -> DataflowResults<BD> + where P: Fn(&BD, BD::Idx) -> DebugFormatted + { + let name_found = |sess: &Session, attrs: &[ast::Attribute], name| -> Option<String> { + if let Some(item) = has_rustc_mir_with(attrs, name) { + if let Some(s) = item.value_str() { + return Some(s.to_string()) + } else { + sess.span_err( + item.span, + &format!("{} attribute requires a path", item.name())); + return None; + } } - } - return None; - }; + return None; + }; - let print_preflow_to = - name_found(tcx.sess, attributes, "borrowck_graphviz_preflow"); - let print_postflow_to = - name_found(tcx.sess, attributes, "borrowck_graphviz_postflow"); + let print_preflow_to = + name_found(tcx.sess, attributes, "borrowck_graphviz_preflow"); + let print_postflow_to = + name_found(tcx.sess, attributes, "borrowck_graphviz_postflow"); - let mut mbcx = DataflowBuilder { - node_id, - print_preflow_to, - print_postflow_to, - flow_state: DataflowAnalysis::new(tcx, mir, dead_unwinds, bd), - }; + let mut mbcx = DataflowBuilder { + node_id, + print_preflow_to, print_postflow_to, flow_state: self, + }; - mbcx.dataflow(p); - mbcx.flow_state.results() + mbcx.dataflow(p); + mbcx.flow_state.results() + } } struct PropagationContext<'b, 'a: 'b, 'tcx: 'a, O> where O: 'b + BitDenotation @@ -181,11 +194,7 @@ impl<'a, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation } fn build_sets(&mut self) { - // First we need to build the entry-, gen- and kill-sets. The - // gather_moves information provides a high-level mapping from - // mir-locations to the MoveOuts (and those correspond - // directly to gen-sets here). But we still need to figure out - // the kill-sets. + // First we need to build the entry-, gen- and kill-sets. { let sets = &mut self.flow_state.sets.for_block(mir::START_BLOCK.index()); @@ -197,18 +206,26 @@ impl<'a, 'tcx: 'a, BD> DataflowAnalysis<'a, 'tcx, BD> where BD: BitDenotation let mut interim_state; let sets = &mut self.flow_state.sets.for_block(bb.index()); - if BD::accumulates_intrablock_state() { + let track_intrablock = BD::accumulates_intrablock_state(); + if track_intrablock { + debug!("swapping in mutable on_entry, initially {:?}", sets.on_entry); interim_state = sets.on_entry.to_owned(); sets.on_entry = &mut interim_state; } for j_stmt in 0..statements.len() { let location = Location { block: bb, statement_index: j_stmt }; self.flow_state.operator.statement_effect(sets, location); + if track_intrablock { + sets.apply_local_effect(); + } } if terminator.is_some() { let location = Location { block: bb, statement_index: statements.len() }; self.flow_state.operator.terminator_effect(sets, location); + if track_intrablock { + sets.apply_local_effect(); + } } } } @@ -271,7 +288,7 @@ impl<'a, 'tcx: 'a, BD> DataflowBuilder<'a, 'tcx, BD> where BD: BitDenotation /// Maps each block to a set of bits #[derive(Debug)] -struct Bits<E:Idx> { +pub(crate) struct Bits<E:Idx> { bits: IdxSetBuf<E>, } @@ -292,7 +309,7 @@ impl<E:Idx> Bits<E> { /// underlying flow analysis results, because it needs to handle cases /// where we are combining the results of *multiple* flow analyses /// (e.g. borrows + inits + uninits). -pub trait DataflowResultsConsumer<'a, 'tcx: 'a> { +pub(crate) trait DataflowResultsConsumer<'a, 'tcx: 'a> { type FlowState: FlowsAtLocation; // Observation Hooks: override (at least one of) these to get analysis feedback. @@ -471,6 +488,7 @@ pub struct AllSets<E: Idx> { /// killed during the iteration. (This is such a good idea that the /// `fn gen` and `fn kill` methods that set their state enforce this /// for you.) +#[derive(Debug)] pub struct BlockSets<'a, E: Idx> { /// Dataflow state immediately before control flow enters the given block. pub(crate) on_entry: &'a mut IdxSet<E>, @@ -512,6 +530,7 @@ impl<'a, E:Idx> BlockSets<'a, E> { self.gen_set.remove(e); self.kill_set.add(e); } + fn kill_all<I>(&mut self, i: I) where I: IntoIterator, I::Item: Borrow<E> @@ -520,6 +539,11 @@ impl<'a, E:Idx> BlockSets<'a, E> { self.kill(j.borrow()); } } + + fn apply_local_effect(&mut self) { + self.on_entry.union(&self.gen_set); + self.on_entry.subtract(&self.kill_set); + } } impl<E:Idx> AllSets<E> { @@ -548,6 +572,9 @@ impl<E:Idx> AllSets<E> { pub fn on_entry_set_for(&self, block_idx: usize) -> &IdxSet<E> { self.lookup_set_for(&self.on_entry_sets, block_idx) } + pub(crate) fn entry_set_state(&self) -> &Bits<E> { + &self.on_entry_sets + } } /// Parameterization for the precise form of data flow that is used. @@ -664,29 +691,33 @@ pub trait BitDenotation: BitwiseOperator { dest_place: &mir::Place); } -impl<'a, 'gcx, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation +impl<'a, 'tcx, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation { - pub fn new(_tcx: TyCtxt<'a, 'gcx, 'tcx>, - mir: &'a Mir<'tcx>, + pub fn new(mir: &'a Mir<'tcx>, dead_unwinds: &'a IdxSet<mir::BasicBlock>, denotation: D) -> Self where D: InitialFlow { let bits_per_block = denotation.bits_per_block(); - let usize_bits = mem::size_of::<usize>() * 8; - let words_per_block = (bits_per_block + usize_bits - 1) / usize_bits; - - // (now rounded up to multiple of word size) - let bits_per_block = words_per_block * usize_bits; - - let num_blocks = mir.basic_blocks().len(); - let num_overall = num_blocks * bits_per_block; - - let zeroes = Bits::new(IdxSetBuf::new_empty(num_overall)); + let num_overall = Self::num_bits_overall(mir, bits_per_block); let on_entry = Bits::new(if D::bottom_value() { IdxSetBuf::new_filled(num_overall) } else { IdxSetBuf::new_empty(num_overall) }); + Self::new_with_entry_sets(mir, dead_unwinds, Cow::Owned(on_entry), denotation) + } + + pub(crate) fn new_with_entry_sets(mir: &'a Mir<'tcx>, + dead_unwinds: &'a IdxSet<mir::BasicBlock>, + on_entry: Cow<Bits<D::Idx>>, + denotation: D) + -> Self { + let bits_per_block = denotation.bits_per_block(); + let usize_bits = mem::size_of::<usize>() * 8; + let words_per_block = (bits_per_block + usize_bits - 1) / usize_bits; + let num_overall = Self::num_bits_overall(mir, bits_per_block); + assert_eq!(num_overall, on_entry.bits.words().len() * usize_bits); + let zeroes = Bits::new(IdxSetBuf::new_empty(num_overall)); DataflowAnalysis { mir, dead_unwinds, @@ -696,12 +727,23 @@ impl<'a, 'gcx, 'tcx: 'a, D> DataflowAnalysis<'a, 'tcx, D> where D: BitDenotation words_per_block, gen_sets: zeroes.clone(), kill_sets: zeroes, - on_entry_sets: on_entry, + on_entry_sets: on_entry.into_owned(), }, operator: denotation, - }, + } } + } + + fn num_bits_overall(mir: &Mir, bits_per_block: usize) -> usize { + let usize_bits = mem::size_of::<usize>() * 8; + let words_per_block = (bits_per_block + usize_bits - 1) / usize_bits; + // (now rounded up to multiple of word size) + let bits_per_block = words_per_block * usize_bits; + + let num_blocks = mir.basic_blocks().len(); + let num_overall = num_blocks * bits_per_block; + num_overall } } diff --git a/src/librustc_mir/dataflow/move_paths/mod.rs b/src/librustc_mir/dataflow/move_paths/mod.rs index 9d91e1344dc..bcf4662211e 100644 --- a/src/librustc_mir/dataflow/move_paths/mod.rs +++ b/src/librustc_mir/dataflow/move_paths/mod.rs @@ -65,6 +65,9 @@ pub(crate) mod indexes { /// Index into Borrows.locations new_index!(BorrowIndex, "bw"); + + /// Index into Reservations/Activations bitvector + new_index!(ReserveOrActivateIndex, "ra"); } pub use self::indexes::MovePathIndex; |