Complete re-implementation of 2-phase borrows

See rust-lang#48431 for discussion as to why this was necessary and what we hoped to
accomplish. A brief summary:
   - the first implementation of 2-phase borrows was hard to limit in the way we
   wanted. That is, it was too good at accepting all 2-phase borrows rather than
   just autorefs =)
   - Numerous diagnostic regressions were introduced by 2-phase borrow support
   which were difficult to fix

Author: sapphire-arches

src/librustc_mir/dataflow/impls/borrows.rs

@@ -450,8 +450,10 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
 
                 // Perform the DFS.
                 // `stack` is the stack of locations still under consideration
+                // `visited` is the set of points we have already visited
                 // `found_use` is an Option that becomes Some when we find a use
                 let mut stack = vec![start_location];
+                let mut visited = FxHashSet();
                 let mut found_use = None;
                 while let Some(curr_loc) = stack.pop() {
                     let block_data = &self.mir.basic_blocks()
@@ -467,6 +469,11 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
                         continue;
                     }
 
+                    if !visited.insert(curr_loc) {
+                        debug!("  Already visited {:?}", curr_loc);
+                        continue;
+                    }
+
                     if self.location_contains_use(curr_loc, assigned_place) {
                         // TODO: Handle this case a little more gracefully. Perhaps collect
                         // all uses in a vector, and find the point in the CFG that dominates
@@ -529,19 +536,6 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
     fn kill_loans_out_of_scope_at_location(&self,
                                            sets: &mut BlockSets<ReserveOrActivateIndex>,
                                            location: Location) {
-        /*
-         XXX: bob_twinkles  reintroduce this
-        let block_data = &self.mir[location.block];
-        if location.statement_index != block_data.statements.len() {
-            let statement = &block_data.statements[location.statement_index];
-            if let mir::StatementKind::EndRegion(region_scope) = statement.kind {
-                for &borrow_index in &self.region_map[&ReScope(region_scope)] {
-                    sets.kill(&ReserveOrActivateIndex::reserved(borrow_index));
-                    sets.kill(&ReserveOrActivateIndex::active(borrow_index));
-                }
-            }
-        }
-        */
         if let Some(ref regioncx) = self.nonlexical_regioncx {
             // NOTE: The state associated with a given `location`
             // reflects the dataflow on entry to the statement. If it
@@ -575,6 +569,20 @@ impl<'a, 'gcx, 'tcx> Borrows<'a, 'gcx, 'tcx> {
                           .map(|b| ReserveOrActivateIndex::active(*b)));
         }
     }
+
+    /// Performs the activations for a given location
+    fn perform_activations_at_location(&self,
+                                       sets: &mut BlockSets<ReserveOrActivateIndex>,
+                                       location: Location) {
+        // Handle activations
+        match self.activation_map.get(&location) {
+            Some(&activated) => {
+                debug!("activating borrow {:?}", activated);
+                sets.gen(&ReserveOrActivateIndex::active(activated))
+            }
+            None => {}
+        }
+    }
 }
 
 impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
@@ -605,13 +613,8 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
             panic!("could not find statement at location {:?}");
         });
 
-        // Handle activations
-        match self.activation_map.get(&location) {
-            Some(&activated) => {
-                sets.gen(&ReserveOrActivateIndex::active(activated))
-            }
-            None => {}
-        }
+        self.perform_activations_at_location(sets, location);
+        self.kill_loans_out_of_scope_at_location(sets, location);
 
         match stmt.kind {
             // EndRegion kills any borrows (reservations and active borrows both)
@@ -643,15 +646,17 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
 
                 if let mir::Rvalue::Ref(region, _, ref place) = *rhs {
                     if is_unsafe_place(self.tcx, self.mir, place) { return; }
+                    let index = self.location_map.get(&location).unwrap_or_else(|| {
+                        panic!("could not find BorrowIndex for location {:?}", location);
+                    });
+
                     if let RegionKind::ReEmpty = region {
-                        // If the borrowed value is dead, the region for it
-                        // can be empty. Don't track the borrow in that case.
+                        // If the borrowed value dies before the borrow is used, the region for
+                        // the borrow can be empty. Don't track the borrow in that case.
+                        sets.kill(&ReserveOrActivateIndex::active(*index));
                         return
                     }
 
-                    let index = self.location_map.get(&location).unwrap_or_else(|| {
-                        panic!("could not find BorrowIndex for location {:?}", location);
-                    });
                     assert!(self.region_map.get(region).unwrap_or_else(|| {
                         panic!("could not find BorrowIndexs for region {:?}", region);
                     }).contains(&index));
@@ -714,14 +719,9 @@ impl<'a, 'gcx, 'tcx> BitDenotation for Borrows<'a, 'gcx, 'tcx> {
         });
 
         let term = block.terminator();
+        self.perform_activations_at_location(sets, location);
+        self.kill_loans_out_of_scope_at_location(sets, location);
 
-        // Handle activations
-        match self.activation_map.get(&location) {
-            Some(&activated) => {
-                sets.gen(&ReserveOrActivateIndex::active(activated))
-            }
-            None => {}
-        }
 
         match term.kind {
             mir::TerminatorKind::Resume |

src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs

@@ -53,7 +53,6 @@ fn not_ok() {
     *y += 1;
     //[lxl_beyond]~^   ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
     //[nll_beyond]~^^  ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
-    //[nll_target]~^^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
     read(z);
 }