From f045008f9430b238491eda3f218361190b5083a0 Mon Sep 17 00:00:00 2001
From: Dylan MacKenzie <ecstaticmorse@gmail.com>
Date: Thu, 20 Jun 2019 15:31:58 -0700
Subject: [PATCH] Kill conflicting borrows of places with projections.

Resolves #62007.

Due to a bug, the previous version of this check did not actually kill
any conflicting borrows unless the borrowed place had no projections.
Specifically, `entry_set` will always be empty when `statement_effect`
is called. It does not contain the set of borrows which are live at this
point in the program.
---
 src/librustc_mir/dataflow/impls/borrows.rs    | 55 +++++++++----------
 ...jection-kills-other-borrows-issue-62007.rs | 21 +++++++
 2 files changed, 46 insertions(+), 30 deletions(-)
 create mode 100644 src/test/run-pass/borrowck/borrowck-borrow-of-projection-kills-other-borrows-issue-62007.rs

diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs
index 899765a1d2daa..7617d3b997d08 100644
--- a/src/librustc_mir/dataflow/impls/borrows.rs
+++ b/src/librustc_mir/dataflow/impls/borrows.rs
@@ -193,43 +193,38 @@ impl<'a, 'tcx> Borrows<'a, 'tcx> {
         place: &Place<'tcx>
     ) {
         debug!("kill_borrows_on_place: place={:?}", place);
-        // Handle the `Place::Local(..)` case first and exit early.
-        if let Place::Base(PlaceBase::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);
+
+        if let Some(local) = place.base_local() {
+            let other_borrows_of_local = self
+                .borrow_set
+                .local_map
+                .get(&local)
+                .into_iter()
+                .flat_map(|bs| bs.into_iter());
+
+            // If the borrowed place is a local with no projections, all other borrows of this
+            // local must conflict. This is purely an optimization so we don't have to call
+            // `places_conflict` for every borrow.
+            if let Place::Base(PlaceBase::Local(_)) = place {
+                sets.kill_all(other_borrows_of_local);
                 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.body,
-                &borrow_data.borrowed_place,
-                place,
-                places_conflict::PlaceConflictBias::NoOverlap,
-            ) {
-                debug!(
-                    "kill_borrows_on_place: (kill) borrow_index={:?} borrow_data={:?}",
-                    borrow_index, borrow_data,
-                );
-                sets.kill(borrow_index);
-            }
+            let definitely_conflicting_borrows = other_borrows_of_local
+                .filter(|&&i| {
+                    places_conflict::places_conflict(
+                        self.tcx,
+                        self.body,
+                        &self.borrow_set.borrows[i].borrowed_place,
+                        place,
+                        places_conflict::PlaceConflictBias::NoOverlap)
+                });
+
+            sets.kill_all(definitely_conflicting_borrows);
         }
     }
 }
diff --git a/src/test/run-pass/borrowck/borrowck-borrow-of-projection-kills-other-borrows-issue-62007.rs b/src/test/run-pass/borrowck/borrowck-borrow-of-projection-kills-other-borrows-issue-62007.rs
new file mode 100644
index 0000000000000..2ab0e6cf35520
--- /dev/null
+++ b/src/test/run-pass/borrowck/borrowck-borrow-of-projection-kills-other-borrows-issue-62007.rs
@@ -0,0 +1,21 @@
+// run-pass
+#![allow(dead_code)]
+
+struct List<T> {
+    value: T,
+    next: Option<Box<List<T>>>,
+}
+
+fn to_refs<T>(mut list: (&mut List<T>,)) -> Vec<&mut T> {
+    let mut result = vec![];
+    loop {
+        result.push(&mut (list.0).value);
+        if let Some(n) = (list.0).next.as_mut() {
+            list.0 = n;
+        } else {
+            return result;
+        }
+    }
+}
+
+fn main() {}