Auto merge of #56649 - davidtwco:issue-46589, r=pnkfelix

MIR borrowck doesn't accept the example of iterating and updating a mutable reference

Fixes #46589.

r? @pnkfelix or @nikomatsakis

4 files changed, 106 insertions, 74 deletions

diff --git a/src/librustc_mir/dataflow/impls/borrowed_locals.rs b/src/librustc_mir/dataflow/impls/borrowed_locals.rs
index 1e279d8dd97..374f7071ffa 100644
--- a/src/librustc_mir/dataflow/impls/borrowed_locals.rs
+++ b/src/librustc_mir/dataflow/impls/borrowed_locals.rs
@@ -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 {
@@ -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
     }
 }
diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs
index 5e78ef03c2c..f6732b0a0b0 100644
--- a/src/librustc_mir/dataflow/impls/borrows.rs
+++ b/src/librustc_mir/dataflow/impls/borrows.rs
@@ -11,7 +11,6 @@
 use borrow_check::borrow_set::{BorrowSet, BorrowData};
 use borrow_check::place_ext::PlaceExt;
 
-use rustc;
 use rustc::mir::{self, Location, Place, Mir};
 use rustc::ty::TyCtxt;
 use rustc::ty::RegionVid;
@@ -24,6 +23,7 @@ use dataflow::{BitDenotation, BlockSets, InitialFlow};
 pub use dataflow::indexes::BorrowIndex;
 use borrow_check::nll::region_infer::RegionInferenceContext;
 use borrow_check::nll::ToRegionVid;
+use borrow_check::places_conflict;
 
 use std::rc::Rc;
 
@@ -191,17 +191,55 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
         }
     }
 
-    fn kill_borrows_on_local(&self,
-                             sets: &mut BlockSets<BorrowIndex>,
-                             local: &rustc::mir::Local)
-    {
-        if let Some(borrow_indexes) = self.borrow_set.local_map.get(local) {
-            sets.kill_all(borrow_indexes);
+    /// Kill any borrows that conflict with `place`.
+    fn kill_borrows_on_place(
+        &self,
+        sets: &mut BlockSets<BorrowIndex>,
+        place: &Place<'tcx>
+    ) {
+        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_indices) = self.borrow_set.local_map.get(&local) {
+                debug!("kill_borrows_on_place: borrow_indices={:?}", borrow_indices);
+                sets.kill_all(borrow_indices);
+                return;
+            }
+        }
+
+        // Otherwise, look at all borrows that are live and if they conflict with the assignment
+        // into our place then we can kill them.
+        let mut borrows = sets.on_entry.clone();
+        let _ = borrows.union(sets.gen_set);
+        for borrow_index in borrows.iter() {
+            let borrow_data = &self.borrows()[borrow_index];
+            debug!(
+                "kill_borrows_on_place: borrow_index={:?} borrow_data={:?}",
+                borrow_index, borrow_data,
+            );
+
+            // By passing `PlaceConflictBias::NoOverlap`, we conservatively assume that any given
+            // pair of array indices are unequal, so that when `places_conflict` returns true, we
+            // will be assured that two places being compared definitely denotes the same sets of
+            // locations.
+            if places_conflict::places_conflict(
+                self.tcx,
+                self.mir,
+                place,
+                &borrow_data.borrowed_place,
+                places_conflict::PlaceConflictBias::NoOverlap,
+            ) {
+                debug!(
+                    "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 {
@@ -222,7 +260,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
     }
 
     fn statement_effect(&self, sets: &mut BlockSets<BorrowIndex>, location: Location) {
-        debug!("Borrows::statement_effect sets: {:?} location: {:?}", sets, location);
+        debug!("Borrows::statement_effect: sets={:?} location={:?}", sets, location);
 
         let block = &self.mir.basic_blocks().get(location.block).unwrap_or_else(|| {
             panic!("could not find block at location {:?}", location);
@@ -231,20 +269,12 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
             panic!("could not find statement at location {:?}");
         });
 
+        debug!("Borrows::statement_effect: stmt={:?}", stmt);
         match stmt.kind {
             mir::StatementKind::Assign(ref lhs, ref rhs) => {
                 // Make sure there are no remaining borrows for variables
                 // that are assigned over.
-                if let Place::Local(ref local) = *lhs {
-                    // FIXME: Handle the case in which we're assigning over
-                    // a projection (`foo.bar`).
-                    self.kill_borrows_on_local(sets, local);
-                }
-
-                // 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.
+                self.kill_borrows_on_place(sets, lhs);
 
                 if let mir::Rvalue::Ref(_, _, ref place) = **rhs {
                     if place.ignore_borrow(
@@ -279,19 +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_local(sets, &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 {
-                        // Make sure there are no remaining borrows for direct
-                        // output variables.
-                        if let Place::Local(ref local) = *output {
-                            // FIXME: Handle the case in which we're assigning over
-                            // a projection (`foo.bar`).
-                            self.kill_borrows_on_local(sets, local);
-                        }
+                        self.kill_borrows_on_place(sets, output);
                     }
                 }
             }
@@ -316,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>,
+    ) {
     }
 }
 
@@ -342,4 +363,3 @@ impl<'a, 'gcx, 'tcx> InitialFlow for Borrows<'a, 'gcx, 'tcx> {
         false // bottom = nothing is reserved or activated yet
     }
 }
-
diff --git a/src/librustc_mir/dataflow/impls/mod.rs b/src/librustc_mir/dataflow/impls/mod.rs
index c29a855b1d2..99e1eab8269 100644
--- a/src/librustc_mir/dataflow/impls/mod.rs
+++ b/src/librustc_mir/dataflow/impls/mod.rs
@@ -293,7 +293,7 @@ impl<'a, 'gcx, 'tcx> DefinitelyInitializedPlaces<'a, 'gcx, 'tcx> {
     }
 }
 
-impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedPlaces<'a, 'gcx, 'tcx> {
+impl<'a, 'gcx, 'tcx> BitDenotation<'tcx> for MaybeInitializedPlaces<'a, 'gcx, 'tcx> {
     type Idx = MovePathIndex;
     fn name() -> &'static str { "maybe_init" }
     fn bits_per_block(&self) -> usize {
@@ -331,11 +331,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedPlaces<'a, 'gcx, 'tcx> {
         )
     }
 
-    fn propagate_call_return(&self,
-                             in_out: &mut BitSet<MovePathIndex>,
-                             _call_bb: mir::BasicBlock,
-                             _dest_bb: mir::BasicBlock,
-                             dest_place: &mir::Place) {
+    fn propagate_call_return(
+        &self,
+        in_out: &mut BitSet<MovePathIndex>,
+        _call_bb: mir::BasicBlock,
+        _dest_bb: mir::BasicBlock,
+        dest_place: &mir::Place<'tcx>,
+    ) {
         // when a call returns successfully, that means we need to set
         // the bits for that dest_place to 1 (initialized).
         on_lookup_result_bits(self.tcx, self.mir, self.move_data(),
@@ -344,7 +346,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeInitializedPlaces<'a, 'gcx, 'tcx> {
     }
 }
 
-impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedPlaces<'a, 'gcx, 'tcx> {
+impl<'a, 'gcx, 'tcx> BitDenotation<'tcx> for MaybeUninitializedPlaces<'a, 'gcx, 'tcx> {
     type Idx = MovePathIndex;
     fn name() -> &'static str { "maybe_uninit" }
     fn bits_per_block(&self) -> usize {
@@ -387,11 +389,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedPlaces<'a, 'gcx, 'tcx>
         )
     }
 
-    fn propagate_call_return(&self,
-                             in_out: &mut BitSet<MovePathIndex>,
-                             _call_bb: mir::BasicBlock,
-                             _dest_bb: mir::BasicBlock,
-                             dest_place: &mir::Place) {
+    fn propagate_call_return(
+        &self,
+        in_out: &mut BitSet<MovePathIndex>,
+        _call_bb: mir::BasicBlock,
+        _dest_bb: mir::BasicBlock,
+        dest_place: &mir::Place<'tcx>,
+    ) {
         // when a call returns successfully, that means we need to set
         // the bits for that dest_place to 0 (initialized).
         on_lookup_result_bits(self.tcx, self.mir, self.move_data(),
@@ -400,7 +404,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for MaybeUninitializedPlaces<'a, 'gcx, 'tcx>
     }
 }
 
-impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedPlaces<'a, 'gcx, 'tcx> {
+impl<'a, 'gcx, 'tcx> BitDenotation<'tcx> for DefinitelyInitializedPlaces<'a, 'gcx, 'tcx> {
     type Idx = MovePathIndex;
     fn name() -> &'static str { "definite_init" }
     fn bits_per_block(&self) -> usize {
@@ -441,11 +445,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedPlaces<'a, 'gcx, 'tc
         )
     }
 
-    fn propagate_call_return(&self,
-                             in_out: &mut BitSet<MovePathIndex>,
-                             _call_bb: mir::BasicBlock,
-                             _dest_bb: mir::BasicBlock,
-                             dest_place: &mir::Place) {
+    fn propagate_call_return(
+        &self,
+        in_out: &mut BitSet<MovePathIndex>,
+        _call_bb: mir::BasicBlock,
+        _dest_bb: mir::BasicBlock,
+        dest_place: &mir::Place<'tcx>,
+    ) {
         // when a call returns successfully, that means we need to set
         // the bits for that dest_place to 1 (initialized).
         on_lookup_result_bits(self.tcx, self.mir, self.move_data(),
@@ -454,7 +460,7 @@ impl<'a, 'gcx, 'tcx> BitDenotation for DefinitelyInitializedPlaces<'a, 'gcx, 'tc
     }
 }
 
-impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedPlaces<'a, 'gcx, 'tcx> {
+impl<'a, 'gcx, 'tcx> BitDenotation<'tcx> for EverInitializedPlaces<'a, 'gcx, 'tcx> {
     type Idx = InitIndex;
     fn name() -> &'static str { "ever_init" }
     fn bits_per_block(&self) -> usize {
@@ -530,11 +536,13 @@ impl<'a, 'gcx, 'tcx> BitDenotation for EverInitializedPlaces<'a, 'gcx, 'tcx> {
         );
     }
 
-    fn propagate_call_return(&self,
-                             in_out: &mut BitSet<InitIndex>,
-                             call_bb: mir::BasicBlock,
-                             _dest_bb: mir::BasicBlock,
-                             _dest_place: &mir::Place) {
+    fn propagate_call_return(
+        &self,
+        in_out: &mut BitSet<InitIndex>,
+        call_bb: mir::BasicBlock,
+        _dest_bb: mir::BasicBlock,
+        _dest_place: &mir::Place<'tcx>,
+    ) {
         let move_data = self.move_data();
         let bits_per_block = self.bits_per_block();
         let init_loc_map = &move_data.init_loc_map;
diff --git a/src/librustc_mir/dataflow/impls/storage_liveness.rs b/src/librustc_mir/dataflow/impls/storage_liveness.rs
index c8faa34df8a..caf3d3f7ab6 100644
--- a/src/librustc_mir/dataflow/impls/storage_liveness.rs
+++ b/src/librustc_mir/dataflow/impls/storage_liveness.rs
@@ -29,7 +29,7 @@ impl<'a, 'tcx: 'a> MaybeStorageLive<'a, 'tcx> {
     }
 }
 
-impl<'a, 'tcx> BitDenotation for MaybeStorageLive<'a, 'tcx> {
+impl<'a, 'tcx> BitDenotation<'tcx> for MaybeStorageLive<'a, 'tcx> {
     type Idx = Local;
     fn name() -> &'static str { "maybe_storage_live" }
     fn bits_per_block(&self) -> usize {
@@ -58,11 +58,13 @@ impl<'a, 'tcx> BitDenotation for MaybeStorageLive<'a, 'tcx> {
         // Terminators have no effect
     }
 
-    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
     }
 }