about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFelix S. Klock II <pnkfelix@pnkfx.org>2018-01-16 10:52:52 +0100
committerFelix S. Klock II <pnkfelix@pnkfx.org>2018-02-08 12:16:30 +0100
commit1855ab742458cc4359e27deadbdf3d8747ce361d (patch)
treed33b5aedc00abf0d1116a9cc70005b1de1a18658
parentc00266b7ac97a8c04136e14f10eb70fb64ec2c94 (diff)
downloadrust-1855ab742458cc4359e27deadbdf3d8747ce361d.tar.gz
rust-1855ab742458cc4359e27deadbdf3d8747ce361d.zip
Restrict two-phase borrows to solely borrows introduced via autoref.
Added `-Z two-phase-beyond-autoref` to bring back old behavior (mainly
to allow demonstration of desugared examples).

Updated tests to use aforementioned flag when necessary. (But in each
case where I added the flag, I made sure to also include a revision
without the flag so that one can readily see what the actual behavior
we expect is for the initial deployment of NLL.)
-rw-r--r--src/librustc/mir/mod.rs9
-rw-r--r--src/librustc/session/config.rs2
-rw-r--r--src/librustc_mir/borrow_check/mod.rs13
-rw-r--r--src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs34
-rw-r--r--src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs23
-rw-r--r--src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs28
6 files changed, 89 insertions, 20 deletions
diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs
index 70bfc1e2d32..c035d02a612 100644
--- a/src/librustc/mir/mod.rs
+++ b/src/librustc/mir/mod.rs
@@ -420,6 +420,15 @@ pub enum BorrowKind {
     }
 }
 
+impl BorrowKind {
+    pub fn allows_two_phase_borrow(&self) -> bool {
+        match *self {
+            BorrowKind::Shared | BorrowKind::Unique => false,
+            BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow,
+        }
+    }
+}
+
 ///////////////////////////////////////////////////////////////////////////
 // Variables and temps
 
diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs
index c56575f432d..76564b9b7d2 100644
--- a/src/librustc/session/config.rs
+++ b/src/librustc/session/config.rs
@@ -1085,6 +1085,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "select which borrowck is used (`ast`, `mir`, or `compare`)"),
     two_phase_borrows: bool = (false, parse_bool, [UNTRACKED],
         "use two-phase reserved/active distinction for `&mut` borrows in MIR borrowck"),
+    two_phase_beyond_autoref: bool = (false, parse_bool, [UNTRACKED],
+        "when using two-phase-borrows, allow two phases even for non-autoref `&mut` borrows"),
     time_passes: bool = (false, parse_bool, [UNTRACKED],
         "measure time of each rustc pass"),
     count_llvm_insns: bool = (false, parse_bool,
diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
index 647cf178310..217dfdf8d41 100644
--- a/src/librustc_mir/borrow_check/mod.rs
+++ b/src/librustc_mir/borrow_check/mod.rs
@@ -707,6 +707,15 @@ impl InitializationRequiringAction {
 }
 
 impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
+    /// Returns true if the borrow represented by `kind` is
+    /// allowed to be split into separate Reservation and
+    /// Activation phases.
+    fn allow_two_phase_borrow(&self, kind: BorrowKind) -> bool {
+        self.tcx.sess.two_phase_borrows() &&
+            (kind.allows_two_phase_borrow() ||
+             self.tcx.sess.opts.debugging_opts.two_phase_beyond_autoref)
+    }
+
     /// Checks an access to the given place to see if it is allowed. Examines the set of borrows
     /// that are in scope, as well as which paths have been initialized, to ensure that (a) the
     /// place is initialized and (b) it is not borrowed in some way that would prevent this
@@ -799,7 +808,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
 
                 (Read(kind), BorrowKind::Unique) | (Read(kind), BorrowKind::Mut { .. }) => {
                     // Reading from mere reservations of mutable-borrows is OK.
-                    if this.tcx.sess.two_phase_borrows() && index.is_reservation()
+                    if this.allow_two_phase_borrow(borrow.kind) && index.is_reservation()
                     {
                         return Control::Continue;
                     }
@@ -947,7 +956,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
                     BorrowKind::Unique | BorrowKind::Mut { .. } => {
                         let wk = WriteKind::MutableBorrow(bk);
-                        if self.tcx.sess.two_phase_borrows() {
+                        if self.allow_two_phase_borrow(bk) {
                             (Deep, Reservation(wk))
                         } else {
                             (Deep, Write(wk))
diff --git a/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs b/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs
index a9797e4d215..709c00ba846 100644
--- a/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs
+++ b/src/test/compile-fail/borrowck/two-phase-activation-sharing-interference.rs
@@ -8,9 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// revisions: lxl nll
-//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
-//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
+// ignore-tidy-linelength
+
+// revisions: lxl_beyond nll_beyond nll_target
+
+//[lxl_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref
+//[nll_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref -Z nll
+//[nll_target] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
 
 // This is an important corner case pointed out by Niko: one is
 // allowed to initiate a shared borrow during a reservation, but it
@@ -18,6 +22,14 @@
 //
 // FIXME: for clarity, diagnostics for these cases might be better off
 // if they specifically said "cannot activate mutable borrow of `x`"
+//
+// The convention for the listed revisions: "lxl" means lexical
+// lifetimes (which can be easier to reason about). "nll" means
+// non-lexical lifetimes. "nll_target" means the initial conservative
+// two-phase borrows that only applies to autoref-introduced borrows.
+// "nll_beyond" means the generalization of two-phase borrows to all
+// `&mut`-borrows (doing so makes it easier to write code for specific
+// corner cases).
 
 #![allow(dead_code)]
 
@@ -27,6 +39,7 @@ fn ok() {
     let mut x = 3;
     let y = &mut x;
     { let z = &x; read(z); }
+    //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
     *y += 1;
 }
 
@@ -34,9 +47,11 @@ fn not_ok() {
     let mut x = 3;
     let y = &mut x;
     let z = &x;
+    //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
     *y += 1;
-    //[lxl]~^  ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
-    //[nll]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
+    //[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);
 }
 
@@ -44,18 +59,21 @@ fn should_be_ok_with_nll() {
     let mut x = 3;
     let y = &mut x;
     let z = &x;
+    //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
     read(z);
     *y += 1;
-    //[lxl]~^  ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
-    // (okay with nll today)
+    //[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
+    // (okay with (generalized) nll today)
 }
 
 fn should_also_eventually_be_ok_with_nll() {
     let mut x = 3;
     let y = &mut x;
     let _z = &x;
+    //[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
     *y += 1;
-    //[lxl]~^  ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
+    //[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
+    // (okay with (generalized) nll today)
 }
 
 fn main() { }
diff --git a/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs b/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs
index 7695bd3e465..dd174981fb1 100644
--- a/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs
+++ b/src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs
@@ -8,9 +8,12 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// revisions: lxl nll
-//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
-//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
+// ignore-tidy-linelength
+
+// revisions: lxl_beyond nll_beyond nll_target
+//[lxl_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two_phase_beyond_autoref
+//[nll_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two_phase_beyond_autoref -Z nll
+//[nll_target] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
 
 // This is the second counter-example from Niko's blog post
 // smallcultfollowing.com/babysteps/blog/2017/03/01/nested-method-calls-via-two-phase-borrowing/
@@ -18,6 +21,14 @@
 // It is "artificial". It is meant to illustrate directly that we
 // should allow an aliasing access during reservation, but *not* while
 // the mutable borrow is active.
+//
+// The convention for the listed revisions: "lxl" means lexical
+// lifetimes (which can be easier to reason about). "nll" means
+// non-lexical lifetimes. "nll_target" means the initial conservative
+// two-phase borrows that only applies to autoref-introduced borrows.
+// "nll_beyond" means the generalization of two-phase borrows to all
+// `&mut`-borrows (doing so makes it easier to write code for specific
+// corner cases).
 
 fn main() {
     /*0*/ let mut i = 0;
@@ -25,11 +36,13 @@ fn main() {
     /*1*/ let p = &mut i; // (reservation of `i` starts here)
 
     /*2*/ let j = i;      // OK: `i` is only reserved here
+                          //[nll_target]~^  ERROR cannot use `i` because it was mutably borrowed [E0503]
 
     /*3*/ *p += 1;        // (mutable borrow of `i` starts here, since `p` is used)
 
-    /*4*/ let k = i;      //[lxl]~  ERROR cannot use `i` because it was mutably borrowed [E0503]
-                          //[nll]~^ ERROR cannot use `i` because it was mutably borrowed [E0503]
+    /*4*/ let k = i;      //[lxl_beyond]~   ERROR cannot use `i` because it was mutably borrowed [E0503]
+                          //[nll_beyond]~^  ERROR cannot use `i` because it was mutably borrowed [E0503]
+                          //[nll_target]~^^ ERROR cannot use `i` because it was mutably borrowed [E0503]
 
     /*5*/ *p += 1;
 
diff --git a/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs b/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs
index cc85315263a..b5fda4985f2 100644
--- a/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs
+++ b/src/test/compile-fail/borrowck/two-phase-reservation-sharing-interference.rs
@@ -8,9 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// revisions: lxl nll
-//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
-//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
+// ignore-tidy-linelength
+
+// revisions: lxl_beyond nll_beyond nll_target
+
+//[lxl_beyond]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref
+//[nll_beyond]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref -Z nll
+//[nll_target]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
 
 // This is a corner case that the current implementation is (probably)
 // treating more conservatively than is necessary. But it also does
@@ -19,6 +23,18 @@
 // So this test is just making a note of the current behavior, with
 // the caveat that in the future, the rules may be loosened, at which
 // point this test might be thrown out.
+//
+// The convention for the listed revisions: "lxl" means lexical
+// lifetimes (which can be easier to reason about). "nll" means
+// non-lexical lifetimes. "nll_target" means the initial conservative
+// two-phase borrows that only applies to autoref-introduced borrows.
+// "nll_beyond" means the generalization of two-phase borrows to all
+// `&mut`-borrows (doing so makes it easier to write code for specific
+// corner cases).
+//
+// FIXME: in "nll_target", we currently see the same error reported
+// twice. This is injected by `-Z two-phase-borrows`; not sure why as
+// of yet.
 
 fn main() {
     let mut vec = vec![0, 1];
@@ -30,8 +46,10 @@ fn main() {
         // with the shared borrow. But in the current implementation,
         // its an error.
         delay = &mut vec;
-        //[lxl]~^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
-        //[nll]~^^   ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
+        //[lxl_beyond]~^   ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
+        //[nll_beyond]~^^  ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
+        //[nll_target]~^^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
+        //[nll_target]~|   ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
 
         shared[0];
     }