Skip to content

Commit

Permalink
Add required lifetime parameter to BitDenotation.
Browse files Browse the repository at this point in the history
This avoids all sorts of confusing issues with using both `dest_place`
and `self` in the `propagate_call_return` function in the
`BitDenotation` implementation for `Borrows`.
  • Loading branch information
davidtwco committed Dec 17, 2018
1 parent db635fc commit 7b628e1
Show file tree
Hide file tree
Showing 14 changed files with 138 additions and 132 deletions.
12 changes: 6 additions & 6 deletions src/librustc_mir/borrow_check/flows.rs
Expand Up @@ -33,19 +33,19 @@ use std::rc::Rc;

// (forced to be `pub` due to its use as an associated type below.)
crate struct Flows<'b, 'gcx: 'tcx, 'tcx: 'b> {
borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
pub uninits: FlowAtLocation<MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
pub ever_inits: FlowAtLocation<EverInitializedPlaces<'b, 'gcx, 'tcx>>,
borrows: FlowAtLocation<'tcx, Borrows<'b, 'gcx, 'tcx>>,
pub uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
pub ever_inits: FlowAtLocation<'tcx, EverInitializedPlaces<'b, 'gcx, 'tcx>>,

/// Polonius Output
pub polonius_output: Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex>>>,
}

impl<'b, 'gcx, 'tcx> Flows<'b, 'gcx, 'tcx> {
crate fn new(
borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
uninits: FlowAtLocation<MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
ever_inits: FlowAtLocation<EverInitializedPlaces<'b, 'gcx, 'tcx>>,
borrows: FlowAtLocation<'tcx, Borrows<'b, 'gcx, 'tcx>>,
uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
ever_inits: FlowAtLocation<'tcx, EverInitializedPlaces<'b, 'gcx, 'tcx>>,
polonius_output: Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex>>>,
) -> Self {
Flows {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/nll/mod.rs
Expand Up @@ -85,7 +85,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
mir: &Mir<'tcx>,
location_table: &LocationTable,
param_env: ty::ParamEnv<'gcx>,
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'cx, 'gcx, 'tcx>>,
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'cx, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
borrow_set: &BorrowSet<'tcx>,
errors_buffer: &mut Vec<Diagnostic>,
Expand Down
Expand Up @@ -39,7 +39,7 @@ pub(super) fn generate<'gcx, 'tcx>(
typeck: &mut TypeChecker<'_, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
elements: &Rc<RegionValueElements>,
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
location_table: &LocationTable,
) {
Expand Down
Expand Up @@ -46,7 +46,7 @@ pub(super) fn trace(
typeck: &mut TypeChecker<'_, 'gcx, 'tcx>,
mir: &Mir<'tcx>,
elements: &Rc<RegionValueElements>,
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
liveness_map: &NllLivenessMap,
location_table: &LocationTable,
Expand Down Expand Up @@ -99,7 +99,7 @@ where

/// Results of dataflow tracking which variables (and paths) have been
/// initialized.
flow_inits: &'me mut FlowAtLocation<MaybeInitializedPlaces<'flow, 'gcx, 'tcx>>,
flow_inits: &'me mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'flow, 'gcx, 'tcx>>,

/// Index indicating where each variable is assigned, used, or
/// dropped.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/nll/type_check/mod.rs
Expand Up @@ -122,7 +122,7 @@ pub(crate) fn type_check<'gcx, 'tcx>(
location_table: &LocationTable,
borrow_set: &BorrowSet<'tcx>,
all_facts: &mut Option<AllFacts>,
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
move_data: &MoveData<'tcx>,
elements: &Rc<RegionValueElements>,
) -> MirTypeckResults<'tcx> {
Expand Down
20 changes: 10 additions & 10 deletions src/librustc_mir/dataflow/at_location.rs
Expand Up @@ -70,19 +70,19 @@ pub trait FlowsAtLocation {
/// (e.g. via `reconstruct_statement_effect` and
/// `reconstruct_terminator_effect`; don't forget to call
/// `apply_local_effect`).
pub struct FlowAtLocation<BD>
pub struct FlowAtLocation<'tcx, BD>
where
BD: BitDenotation,
BD: BitDenotation<'tcx>,
{
base_results: DataflowResults<BD>,
base_results: DataflowResults<'tcx, BD>,
curr_state: BitSet<BD::Idx>,
stmt_gen: HybridBitSet<BD::Idx>,
stmt_kill: HybridBitSet<BD::Idx>,
}

impl<BD> FlowAtLocation<BD>
impl<'tcx, BD> FlowAtLocation<'tcx, BD>
where
BD: BitDenotation,
BD: BitDenotation<'tcx>,
{
/// Iterate over each bit set in the current state.
pub fn each_state_bit<F>(&self, f: F)
Expand All @@ -102,7 +102,7 @@ where
self.stmt_gen.iter().for_each(f)
}

pub fn new(results: DataflowResults<BD>) -> Self {
pub fn new(results: DataflowResults<'tcx, BD>) -> Self {
let bits_per_block = results.sets().bits_per_block();
let curr_state = BitSet::new_empty(bits_per_block);
let stmt_gen = HybridBitSet::new_empty(bits_per_block);
Expand Down Expand Up @@ -143,8 +143,8 @@ where
}
}

impl<BD> FlowsAtLocation for FlowAtLocation<BD>
where BD: BitDenotation
impl<'tcx, BD> FlowsAtLocation for FlowAtLocation<'tcx, BD>
where BD: BitDenotation<'tcx>
{
fn reset_to_entry_of(&mut self, bb: BasicBlock) {
self.curr_state.overwrite(self.base_results.sets().on_entry_set_for(bb.index()));
Expand Down Expand Up @@ -213,9 +213,9 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD>
}


impl<'tcx, T> FlowAtLocation<T>
impl<'tcx, T> FlowAtLocation<'tcx, T>
where
T: HasMoveData<'tcx> + BitDenotation<Idx = MovePathIndex>,
T: HasMoveData<'tcx> + BitDenotation<'tcx, Idx = MovePathIndex>,
{
pub fn has_any_child_of(&self, mpi: T::Idx) -> Option<T::Idx> {
// We process `mpi` before the loop below, for two reasons:
Expand Down
16 changes: 8 additions & 8 deletions src/librustc_mir/dataflow/graphviz.rs
Expand Up @@ -25,19 +25,19 @@ use super::DataflowBuilder;
use super::DebugFormatted;

pub trait MirWithFlowState<'tcx> {
type BD: BitDenotation;
type BD: BitDenotation<'tcx>;
fn node_id(&self) -> NodeId;
fn mir(&self) -> &Mir<'tcx>;
fn flow_state(&self) -> &DataflowState<Self::BD>;
fn flow_state(&self) -> &DataflowState<'tcx, Self::BD>;
}

impl<'a, 'tcx, BD> MirWithFlowState<'tcx> for DataflowBuilder<'a, 'tcx, BD>
where BD: BitDenotation
where BD: BitDenotation<'tcx>
{
type BD = BD;
fn node_id(&self) -> NodeId { self.node_id }
fn mir(&self) -> &Mir<'tcx> { self.flow_state.mir() }
fn flow_state(&self) -> &DataflowState<Self::BD> { &self.flow_state.flow_state }
fn flow_state(&self) -> &DataflowState<'tcx, Self::BD> { &self.flow_state.flow_state }
}

struct Graph<'a, 'tcx, MWF:'a, P> where
Expand All @@ -53,8 +53,8 @@ pub(crate) fn print_borrowck_graph_to<'a, 'tcx, BD, P>(
path: &Path,
render_idx: P)
-> io::Result<()>
where BD: BitDenotation,
P: Fn(&BD, BD::Idx) -> DebugFormatted
where BD: BitDenotation<'tcx>,
P: Fn(&BD, BD::Idx) -> DebugFormatted,
{
let g = Graph { mbcx, phantom: PhantomData, render_idx };
let mut v = Vec::new();
Expand All @@ -76,7 +76,7 @@ fn outgoing(mir: &Mir, bb: BasicBlock) -> Vec<Edge> {

impl<'a, 'tcx, MWF, P> dot::Labeller<'a> for Graph<'a, 'tcx, MWF, P>
where MWF: MirWithFlowState<'tcx>,
P: Fn(&MWF::BD, <MWF::BD as BitDenotation>::Idx) -> DebugFormatted,
P: Fn(&MWF::BD, <MWF::BD as BitDenotation<'tcx>>::Idx) -> DebugFormatted,
{
type Node = Node;
type Edge = Edge;
Expand Down Expand Up @@ -128,7 +128,7 @@ impl<'a, 'tcx, MWF, P> dot::Labeller<'a> for Graph<'a, 'tcx, MWF, P>

impl<'a, 'tcx, MWF, P> Graph<'a, 'tcx, MWF, P>
where MWF: MirWithFlowState<'tcx>,
P: Fn(&MWF::BD, <MWF::BD as BitDenotation>::Idx) -> DebugFormatted,
P: Fn(&MWF::BD, <MWF::BD as BitDenotation<'tcx>>::Idx) -> DebugFormatted,
{
/// Generate the node label
fn node_label_internal<W: io::Write>(&self,
Expand Down
14 changes: 8 additions & 6 deletions src/librustc_mir/dataflow/impls/borrowed_locals.rs
Expand Up @@ -36,7 +36,7 @@ impl<'a, 'tcx: 'a> HaveBeenBorrowedLocals<'a, 'tcx> {
}
}

impl<'a, 'tcx> BitDenotation for HaveBeenBorrowedLocals<'a, 'tcx> {
impl<'a, 'tcx> BitDenotation<'tcx> for HaveBeenBorrowedLocals<'a, 'tcx> {
type Idx = Local;
fn name() -> &'static str { "has_been_borrowed_locals" }
fn bits_per_block(&self) -> usize {
Expand Down Expand Up @@ -71,11 +71,13 @@ impl<'a, 'tcx> BitDenotation for HaveBeenBorrowedLocals<'a, 'tcx> {
}.visit_terminator(loc.block, self.mir[loc.block].terminator(), loc);
}

fn propagate_call_return(&self,
_in_out: &mut BitSet<Local>,
_call_bb: mir::BasicBlock,
_dest_bb: mir::BasicBlock,
_dest_place: &mir::Place) {
fn propagate_call_return(
&self,
_in_out: &mut BitSet<Local>,
_call_bb: mir::BasicBlock,
_dest_bb: mir::BasicBlock,
_dest_place: &mir::Place<'tcx>,
) {
// Nothing to do when a call returns successfully
}
}
Expand Down
45 changes: 17 additions & 28 deletions src/librustc_mir/dataflow/impls/borrows.rs
Expand Up @@ -195,18 +195,14 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
fn kill_borrows_on_place(
&self,
sets: &mut BlockSets<BorrowIndex>,
location: Location,
place: &Place<'tcx>
) {
debug!("kill_borrows_on_place: location={:?} place={:?}", location, place);
debug!("kill_borrows_on_place: place={:?}", place);
// Handle the `Place::Local(..)` case first and exit early.
if let Place::Local(local) = place {
if let Some(borrow_indexes) = self.borrow_set.local_map.get(&local) {
debug!(
"kill_borrows_on_place: local={:?} borrow_indexes={:?}",
local, borrow_indexes,
);
sets.kill_all(borrow_indexes);
if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) {
debug!("kill_borrows_on_place: borrow_indices={:?}", borrow_indices);
sets.kill_all(borrow_indices);
return;
}
}
Expand Down Expand Up @@ -234,16 +230,16 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
places_conflict::PlaceConflictBias::NoOverlap,
) {
debug!(
"kill_borrows_on_place: (kill) place={:?} borrow_index={:?} borrow_data={:?}",
place, borrow_index, borrow_data,
"kill_borrows_on_place: (kill) borrow_index={:?} borrow_data={:?}",
borrow_index, borrow_data,
);
sets.kill(borrow_index);
}
}
}
}

impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
impl<'a, 'gcx, 'tcx> BitDenotation<'tcx> for Borrows<'a, 'gcx, 'tcx> {
type Idx = BorrowIndex;
fn name() -> &'static str { "borrows" }
fn bits_per_block(&self) -> usize {
Expand Down Expand Up @@ -278,12 +274,8 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
mir::StatementKind::Assign(ref lhs, ref rhs) => {
// Make sure there are no remaining borrows for variables
// that are assigned over.
self.kill_borrows_on_place(sets, location, lhs);
self.kill_borrows_on_place(sets, lhs);

// 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(_, _, ref place) = **rhs {
if place.ignore_borrow(
self.tcx,
Expand Down Expand Up @@ -317,13 +309,13 @@ 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.
self.kill_borrows_on_place(sets, location, &Place::Local(local));
self.kill_borrows_on_place(sets, &Place::Local(local));
}

mir::StatementKind::InlineAsm { ref outputs, ref asm, .. } => {
for (output, kind) in outputs.iter().zip(&asm.outputs) {
if !kind.is_indirect && !kind.is_rw {
self.kill_borrows_on_place(sets, location, output);
self.kill_borrows_on_place(sets, output);
}
}
}
Expand All @@ -348,16 +340,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {

fn terminator_effect(&self, _: &mut BlockSets<BorrowIndex>, _: Location) {}

fn propagate_call_return(&self,
_in_out: &mut BitSet<BorrowIndex>,
_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.
fn propagate_call_return(
&self,
_in_out: &mut BitSet<BorrowIndex>,
_call_bb: mir::BasicBlock,
_dest_bb: mir::BasicBlock,
_dest_place: &mir::Place<'tcx>,
) {
}
}

Expand Down

0 comments on commit 7b628e1

Please sign in to comment.