Skip to content

Commit 7b628e1

Browse files
committed
Add required lifetime parameter to BitDenotation.
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`.
1 parent db635fc commit 7b628e1

File tree

14 files changed

+138
-132
lines changed

14 files changed

+138
-132
lines changed

src/librustc_mir/borrow_check/flows.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -33,19 +33,19 @@ use std::rc::Rc;
3333

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

4040
/// Polonius Output
4141
pub polonius_output: Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex>>>,
4242
}
4343

4444
impl<'b, 'gcx, 'tcx> Flows<'b, 'gcx, 'tcx> {
4545
crate fn new(
46-
borrows: FlowAtLocation<Borrows<'b, 'gcx, 'tcx>>,
47-
uninits: FlowAtLocation<MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
48-
ever_inits: FlowAtLocation<EverInitializedPlaces<'b, 'gcx, 'tcx>>,
46+
borrows: FlowAtLocation<'tcx, Borrows<'b, 'gcx, 'tcx>>,
47+
uninits: FlowAtLocation<'tcx, MaybeUninitializedPlaces<'b, 'gcx, 'tcx>>,
48+
ever_inits: FlowAtLocation<'tcx, EverInitializedPlaces<'b, 'gcx, 'tcx>>,
4949
polonius_output: Option<Rc<Output<RegionVid, BorrowIndex, LocationIndex>>>,
5050
) -> Self {
5151
Flows {

src/librustc_mir/borrow_check/nll/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
8585
mir: &Mir<'tcx>,
8686
location_table: &LocationTable,
8787
param_env: ty::ParamEnv<'gcx>,
88-
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'cx, 'gcx, 'tcx>>,
88+
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'cx, 'gcx, 'tcx>>,
8989
move_data: &MoveData<'tcx>,
9090
borrow_set: &BorrowSet<'tcx>,
9191
errors_buffer: &mut Vec<Diagnostic>,

src/librustc_mir/borrow_check/nll/type_check/liveness/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ pub(super) fn generate<'gcx, 'tcx>(
3939
typeck: &mut TypeChecker<'_, 'gcx, 'tcx>,
4040
mir: &Mir<'tcx>,
4141
elements: &Rc<RegionValueElements>,
42-
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
42+
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
4343
move_data: &MoveData<'tcx>,
4444
location_table: &LocationTable,
4545
) {

src/librustc_mir/borrow_check/nll/type_check/liveness/trace.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ pub(super) fn trace(
4646
typeck: &mut TypeChecker<'_, 'gcx, 'tcx>,
4747
mir: &Mir<'tcx>,
4848
elements: &Rc<RegionValueElements>,
49-
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
49+
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
5050
move_data: &MoveData<'tcx>,
5151
liveness_map: &NllLivenessMap,
5252
location_table: &LocationTable,
@@ -99,7 +99,7 @@ where
9999

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

104104
/// Index indicating where each variable is assigned, used, or
105105
/// dropped.

src/librustc_mir/borrow_check/nll/type_check/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ pub(crate) fn type_check<'gcx, 'tcx>(
122122
location_table: &LocationTable,
123123
borrow_set: &BorrowSet<'tcx>,
124124
all_facts: &mut Option<AllFacts>,
125-
flow_inits: &mut FlowAtLocation<MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
125+
flow_inits: &mut FlowAtLocation<'tcx, MaybeInitializedPlaces<'_, 'gcx, 'tcx>>,
126126
move_data: &MoveData<'tcx>,
127127
elements: &Rc<RegionValueElements>,
128128
) -> MirTypeckResults<'tcx> {

src/librustc_mir/dataflow/at_location.rs

+10-10
Original file line numberDiff line numberDiff line change
@@ -70,19 +70,19 @@ pub trait FlowsAtLocation {
7070
/// (e.g. via `reconstruct_statement_effect` and
7171
/// `reconstruct_terminator_effect`; don't forget to call
7272
/// `apply_local_effect`).
73-
pub struct FlowAtLocation<BD>
73+
pub struct FlowAtLocation<'tcx, BD>
7474
where
75-
BD: BitDenotation,
75+
BD: BitDenotation<'tcx>,
7676
{
77-
base_results: DataflowResults<BD>,
77+
base_results: DataflowResults<'tcx, BD>,
7878
curr_state: BitSet<BD::Idx>,
7979
stmt_gen: HybridBitSet<BD::Idx>,
8080
stmt_kill: HybridBitSet<BD::Idx>,
8181
}
8282

83-
impl<BD> FlowAtLocation<BD>
83+
impl<'tcx, BD> FlowAtLocation<'tcx, BD>
8484
where
85-
BD: BitDenotation,
85+
BD: BitDenotation<'tcx>,
8686
{
8787
/// Iterate over each bit set in the current state.
8888
pub fn each_state_bit<F>(&self, f: F)
@@ -102,7 +102,7 @@ where
102102
self.stmt_gen.iter().for_each(f)
103103
}
104104

105-
pub fn new(results: DataflowResults<BD>) -> Self {
105+
pub fn new(results: DataflowResults<'tcx, BD>) -> Self {
106106
let bits_per_block = results.sets().bits_per_block();
107107
let curr_state = BitSet::new_empty(bits_per_block);
108108
let stmt_gen = HybridBitSet::new_empty(bits_per_block);
@@ -143,8 +143,8 @@ where
143143
}
144144
}
145145

146-
impl<BD> FlowsAtLocation for FlowAtLocation<BD>
147-
where BD: BitDenotation
146+
impl<'tcx, BD> FlowsAtLocation for FlowAtLocation<'tcx, BD>
147+
where BD: BitDenotation<'tcx>
148148
{
149149
fn reset_to_entry_of(&mut self, bb: BasicBlock) {
150150
self.curr_state.overwrite(self.base_results.sets().on_entry_set_for(bb.index()));
@@ -213,9 +213,9 @@ impl<BD> FlowsAtLocation for FlowAtLocation<BD>
213213
}
214214

215215

216-
impl<'tcx, T> FlowAtLocation<T>
216+
impl<'tcx, T> FlowAtLocation<'tcx, T>
217217
where
218-
T: HasMoveData<'tcx> + BitDenotation<Idx = MovePathIndex>,
218+
T: HasMoveData<'tcx> + BitDenotation<'tcx, Idx = MovePathIndex>,
219219
{
220220
pub fn has_any_child_of(&self, mpi: T::Idx) -> Option<T::Idx> {
221221
// We process `mpi` before the loop below, for two reasons:

src/librustc_mir/dataflow/graphviz.rs

+8-8
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ use super::DataflowBuilder;
2525
use super::DebugFormatted;
2626

2727
pub trait MirWithFlowState<'tcx> {
28-
type BD: BitDenotation;
28+
type BD: BitDenotation<'tcx>;
2929
fn node_id(&self) -> NodeId;
3030
fn mir(&self) -> &Mir<'tcx>;
31-
fn flow_state(&self) -> &DataflowState<Self::BD>;
31+
fn flow_state(&self) -> &DataflowState<'tcx, Self::BD>;
3232
}
3333

3434
impl<'a, 'tcx, BD> MirWithFlowState<'tcx> for DataflowBuilder<'a, 'tcx, BD>
35-
where BD: BitDenotation
35+
where BD: BitDenotation<'tcx>
3636
{
3737
type BD = BD;
3838
fn node_id(&self) -> NodeId { self.node_id }
3939
fn mir(&self) -> &Mir<'tcx> { self.flow_state.mir() }
40-
fn flow_state(&self) -> &DataflowState<Self::BD> { &self.flow_state.flow_state }
40+
fn flow_state(&self) -> &DataflowState<'tcx, Self::BD> { &self.flow_state.flow_state }
4141
}
4242

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

7777
impl<'a, 'tcx, MWF, P> dot::Labeller<'a> for Graph<'a, 'tcx, MWF, P>
7878
where MWF: MirWithFlowState<'tcx>,
79-
P: Fn(&MWF::BD, <MWF::BD as BitDenotation>::Idx) -> DebugFormatted,
79+
P: Fn(&MWF::BD, <MWF::BD as BitDenotation<'tcx>>::Idx) -> DebugFormatted,
8080
{
8181
type Node = Node;
8282
type Edge = Edge;
@@ -128,7 +128,7 @@ impl<'a, 'tcx, MWF, P> dot::Labeller<'a> for Graph<'a, 'tcx, MWF, P>
128128

129129
impl<'a, 'tcx, MWF, P> Graph<'a, 'tcx, MWF, P>
130130
where MWF: MirWithFlowState<'tcx>,
131-
P: Fn(&MWF::BD, <MWF::BD as BitDenotation>::Idx) -> DebugFormatted,
131+
P: Fn(&MWF::BD, <MWF::BD as BitDenotation<'tcx>>::Idx) -> DebugFormatted,
132132
{
133133
/// Generate the node label
134134
fn node_label_internal<W: io::Write>(&self,

src/librustc_mir/dataflow/impls/borrowed_locals.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ impl<'a, 'tcx: 'a> HaveBeenBorrowedLocals<'a, 'tcx> {
3636
}
3737
}
3838

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

74-
fn propagate_call_return(&self,
75-
_in_out: &mut BitSet<Local>,
76-
_call_bb: mir::BasicBlock,
77-
_dest_bb: mir::BasicBlock,
78-
_dest_place: &mir::Place) {
74+
fn propagate_call_return(
75+
&self,
76+
_in_out: &mut BitSet<Local>,
77+
_call_bb: mir::BasicBlock,
78+
_dest_bb: mir::BasicBlock,
79+
_dest_place: &mir::Place<'tcx>,
80+
) {
7981
// Nothing to do when a call returns successfully
8082
}
8183
}

src/librustc_mir/dataflow/impls/borrows.rs

+17-28
Original file line numberDiff line numberDiff line change
@@ -195,18 +195,14 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
195195
fn kill_borrows_on_place(
196196
&self,
197197
sets: &mut BlockSets<BorrowIndex>,
198-
location: Location,
199198
place: &Place<'tcx>
200199
) {
201-
debug!("kill_borrows_on_place: location={:?} place={:?}", location, place);
200+
debug!("kill_borrows_on_place: place={:?}", place);
202201
// Handle the `Place::Local(..)` case first and exit early.
203202
if let Place::Local(local) = place {
204-
if let Some(borrow_indexes) = self.borrow_set.local_map.get(&local) {
205-
debug!(
206-
"kill_borrows_on_place: local={:?} borrow_indexes={:?}",
207-
local, borrow_indexes,
208-
);
209-
sets.kill_all(borrow_indexes);
203+
if let Some(borrow_indices) = self.borrow_set.local_map.get(&local) {
204+
debug!("kill_borrows_on_place: borrow_indices={:?}", borrow_indices);
205+
sets.kill_all(borrow_indices);
210206
return;
211207
}
212208
}
@@ -234,16 +230,16 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
234230
places_conflict::PlaceConflictBias::NoOverlap,
235231
) {
236232
debug!(
237-
"kill_borrows_on_place: (kill) place={:?} borrow_index={:?} borrow_data={:?}",
238-
place, borrow_index, borrow_data,
233+
"kill_borrows_on_place: (kill) borrow_index={:?} borrow_data={:?}",
234+
borrow_index, borrow_data,
239235
);
240236
sets.kill(borrow_index);
241237
}
242238
}
243239
}
244240
}
245241

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

283-
// NOTE: if/when the Assign case is revised to inspect
284-
// the assigned_place here, make sure to also
285-
// re-consider the current implementations of the
286-
// propagate_call_return method.
287279
if let mir::Rvalue::Ref(_, _, ref place) = **rhs {
288280
if place.ignore_borrow(
289281
self.tcx,
@@ -317,13 +309,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
317309
mir::StatementKind::StorageDead(local) => {
318310
// Make sure there are no remaining borrows for locals that
319311
// are gone out of scope.
320-
self.kill_borrows_on_place(sets, location, &Place::Local(local));
312+
self.kill_borrows_on_place(sets, &Place::Local(local));
321313
}
322314

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

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

351-
fn propagate_call_return(&self,
352-
_in_out: &mut BitSet<BorrowIndex>,
353-
_call_bb: mir::BasicBlock,
354-
_dest_bb: mir::BasicBlock,
355-
_dest_place: &mir::Place) {
356-
// there are no effects on borrows from method call return...
357-
//
358-
// ... but if overwriting a place can affect flow state, then
359-
// latter is not true; see NOTE on Assign case in
360-
// statement_effect_on_borrows.
343+
fn propagate_call_return(
344+
&self,
345+
_in_out: &mut BitSet<BorrowIndex>,
346+
_call_bb: mir::BasicBlock,
347+
_dest_bb: mir::BasicBlock,
348+
_dest_place: &mir::Place<'tcx>,
349+
) {
361350
}
362351
}
363352

0 commit comments

Comments
 (0)