about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMark Rousskov <mark.simulacrum@gmail.com>2018-07-06 21:29:12 -0600
committerGitHub <noreply@github.com>2018-07-06 21:29:12 -0600
commitb71b92199c5a04e41b10bcd09333ca8762933bdc (patch)
tree2b9cca885dfc4bdedd0dda369da889cbcf127d2c
parent9b3ec57c162ab0079a173750c4edb6f531cd83b3 (diff)
parentf90eada1c97dd0d0a752d34090a957661789379a (diff)
downloadrust-b71b92199c5a04e41b10bcd09333ca8762933bdc.tar.gz
rust-b71b92199c5a04e41b10bcd09333ca8762933bdc.zip
Rollup merge of #52058 - davidtwco:issue-51345, r=nikomatsakis
Use of unimplemented!() causing ICE with NLL

Fixes #51345.

r? @nikomatsakis
-rw-r--r--src/librustc_mir/borrow_check/borrow_set.rs66
-rw-r--r--src/librustc_mir/borrow_check/path_utils.rs10
-rw-r--r--src/test/run-fail/issue-51345.rs18
-rw-r--r--src/test/run-pass/issue-51345.rs17
4 files changed, 70 insertions, 41 deletions
diff --git a/src/librustc_mir/borrow_check/borrow_set.rs b/src/librustc_mir/borrow_check/borrow_set.rs
index 3d6f49c3772..e9e0c5c3613 100644
--- a/src/librustc_mir/borrow_check/borrow_set.rs
+++ b/src/librustc_mir/borrow_check/borrow_set.rs
@@ -53,15 +53,13 @@ impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
     }
 }
 
-/// Every two-phase borrow has *exactly one* use (or else it is not a
-/// proper two-phase borrow under our current definition). However, not
-/// all uses are actually ones that activate the reservation.. In
-/// particular, a shared borrow of a `&mut` does not activate the
-/// reservation.
+/// Location where a two phase borrow is activated, if a borrow
+/// is in fact a two phase borrow.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
-crate enum TwoPhaseUse {
-    MutActivate,
-    SharedUse,
+crate enum TwoPhaseActivation {
+    NotTwoPhase,
+    NotActivated,
+    ActivatedAt(Location),
 }
 
 #[derive(Debug)]
@@ -69,9 +67,8 @@ crate struct BorrowData<'tcx> {
     /// Location where the borrow reservation starts.
     /// In many cases, this will be equal to the activation location but not always.
     crate reserve_location: Location,
-    /// Location where the borrow is activated. None if this is not a
-    /// 2-phase borrow.
-    crate activation_location: Option<(TwoPhaseUse, Location)>,
+    /// Location where the borrow is activated.
+    crate activation_location: TwoPhaseActivation,
     /// What kind of borrow this is
     crate kind: mir::BorrowKind,
     /// The region for which this borrow is live
@@ -116,19 +113,6 @@ impl<'tcx> BorrowSet<'tcx> {
             visitor.visit_basic_block_data(block, block_data);
         }
 
-        // Double check: We should have found an activation for every pending
-        // activation.
-        assert_eq!(
-            visitor
-                .pending_activations
-                .iter()
-                .find(|&(_local, &borrow_index)| visitor.idx_vec[borrow_index]
-                    .activation_location
-                    .is_none()),
-            None,
-            "never found an activation for this borrow!",
-        );
-
         BorrowSet {
             borrows: visitor.idx_vec,
             location_map: visitor.location_map,
@@ -183,7 +167,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
                 kind,
                 region,
                 reserve_location: location,
-                activation_location: None,
+                activation_location: TwoPhaseActivation::NotTwoPhase,
                 borrowed_place: borrowed_place.clone(),
                 assigned_place: assigned_place.clone(),
             };
@@ -232,38 +216,43 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
                         return;
                     }
 
-                    if let Some(other_activation) = borrow_data.activation_location {
+                    if let TwoPhaseActivation::ActivatedAt(other_location) =
+                            borrow_data.activation_location {
                         span_bug!(
                             self.mir.source_info(location).span,
                             "found two uses for 2-phase borrow temporary {:?}: \
                              {:?} and {:?}",
                             temp,
                             location,
-                            other_activation,
+                            other_location,
                         );
                     }
 
                     // Otherwise, this is the unique later use
                     // that we expect.
-
-                    let two_phase_use;
-
-                    match context {
+                    borrow_data.activation_location = match context {
                         // The use of TMP in a shared borrow does not
                         // count as an actual activation.
                         PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. } => {
-                            two_phase_use = TwoPhaseUse::SharedUse;
+                            TwoPhaseActivation::NotActivated
                         }
                         _ => {
-                            two_phase_use = TwoPhaseUse::MutActivate;
+                            // Double check: This borrow is indeed a two-phase borrow (that is,
+                            // we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
+                            // we've not found any other activations (checked above).
+                            assert_eq!(
+                                borrow_data.activation_location,
+                                TwoPhaseActivation::NotActivated,
+                                "never found an activation for this borrow!",
+                            );
+
                             self.activation_map
                                 .entry(location)
                                 .or_insert(Vec::new())
                                 .push(borrow_index);
+                            TwoPhaseActivation::ActivatedAt(location)
                         }
-                    }
-
-                    borrow_data.activation_location = Some((two_phase_use, location));
+                    };
                 }
 
                 None => {}
@@ -342,6 +331,11 @@ impl<'a, 'gcx, 'tcx> GatherBorrows<'a, 'gcx, 'tcx> {
             );
         };
 
+        // Consider the borrow not activated to start. When we find an activation, we'll update
+        // this field.
+        let borrow_data = &mut self.idx_vec[borrow_index];
+        borrow_data.activation_location = TwoPhaseActivation::NotActivated;
+
         // Insert `temp` into the list of pending activations. From
         // now on, we'll be on the lookout for a use of it. Note that
         // we are guaranteed that this use will come after the
diff --git a/src/librustc_mir/borrow_check/path_utils.rs b/src/librustc_mir/borrow_check/path_utils.rs
index 8ae98bde003..ca2a120ceb7 100644
--- a/src/librustc_mir/borrow_check/path_utils.rs
+++ b/src/librustc_mir/borrow_check/path_utils.rs
@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseUse};
+use borrow_check::borrow_set::{BorrowSet, BorrowData, TwoPhaseActivation};
 use borrow_check::places_conflict;
 use borrow_check::Context;
 use borrow_check::ShallowOrDeep;
@@ -83,11 +83,11 @@ pub(super) fn is_active<'tcx>(
 
     let activation_location = match borrow_data.activation_location {
         // If this is not a 2-phase borrow, it is always active.
-        None => return true,
+        TwoPhaseActivation::NotTwoPhase => return true,
         // And if the unique 2-phase use is not an activation, then it is *never* active.
-        Some((TwoPhaseUse::SharedUse, _)) => return false,
-        // Otherwise, we derive info from the activation point `v`:
-        Some((TwoPhaseUse::MutActivate, v)) => v,
+        TwoPhaseActivation::NotActivated => return false,
+        // Otherwise, we derive info from the activation point `loc`:
+        TwoPhaseActivation::ActivatedAt(loc) => loc,
     };
 
     // Otherwise, it is active for every location *except* in between
diff --git a/src/test/run-fail/issue-51345.rs b/src/test/run-fail/issue-51345.rs
new file mode 100644
index 00000000000..9efd08195a1
--- /dev/null
+++ b/src/test/run-fail/issue-51345.rs
@@ -0,0 +1,18 @@
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// error-pattern: thread 'main' panicked at 'explicit panic'
+
+#![feature(nll)]
+
+fn main() {
+    let mut vec = vec![];
+    vec.push((vec.len(), panic!()));
+}
diff --git a/src/test/run-pass/issue-51345.rs b/src/test/run-pass/issue-51345.rs
new file mode 100644
index 00000000000..7d392944685
--- /dev/null
+++ b/src/test/run-pass/issue-51345.rs
@@ -0,0 +1,17 @@
+// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(nll)]
+
+fn main() {
+    let mut v = Vec::new();
+
+    loop { v.push(break) }
+}