Skip to content

Commit 1855ab7

Browse files
committed
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.)
1 parent c00266b commit 1855ab7

File tree

6 files changed

+89
-20
lines changed

6 files changed

+89
-20
lines changed

src/librustc/mir/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -420,6 +420,15 @@ pub enum BorrowKind {
420420
}
421421
}
422422

423+
impl BorrowKind {
424+
pub fn allows_two_phase_borrow(&self) -> bool {
425+
match *self {
426+
BorrowKind::Shared | BorrowKind::Unique => false,
427+
BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow,
428+
}
429+
}
430+
}
431+
423432
///////////////////////////////////////////////////////////////////////////
424433
// Variables and temps
425434

src/librustc/session/config.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1085,6 +1085,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
10851085
"select which borrowck is used (`ast`, `mir`, or `compare`)"),
10861086
two_phase_borrows: bool = (false, parse_bool, [UNTRACKED],
10871087
"use two-phase reserved/active distinction for `&mut` borrows in MIR borrowck"),
1088+
two_phase_beyond_autoref: bool = (false, parse_bool, [UNTRACKED],
1089+
"when using two-phase-borrows, allow two phases even for non-autoref `&mut` borrows"),
10881090
time_passes: bool = (false, parse_bool, [UNTRACKED],
10891091
"measure time of each rustc pass"),
10901092
count_llvm_insns: bool = (false, parse_bool,

src/librustc_mir/borrow_check/mod.rs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,15 @@ impl InitializationRequiringAction {
707707
}
708708

709709
impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
710+
/// Returns true if the borrow represented by `kind` is
711+
/// allowed to be split into separate Reservation and
712+
/// Activation phases.
713+
fn allow_two_phase_borrow(&self, kind: BorrowKind) -> bool {
714+
self.tcx.sess.two_phase_borrows() &&
715+
(kind.allows_two_phase_borrow() ||
716+
self.tcx.sess.opts.debugging_opts.two_phase_beyond_autoref)
717+
}
718+
710719
/// Checks an access to the given place to see if it is allowed. Examines the set of borrows
711720
/// that are in scope, as well as which paths have been initialized, to ensure that (a) the
712721
/// 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> {
799808

800809
(Read(kind), BorrowKind::Unique) | (Read(kind), BorrowKind::Mut { .. }) => {
801810
// Reading from mere reservations of mutable-borrows is OK.
802-
if this.tcx.sess.two_phase_borrows() && index.is_reservation()
811+
if this.allow_two_phase_borrow(borrow.kind) && index.is_reservation()
803812
{
804813
return Control::Continue;
805814
}
@@ -947,7 +956,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
947956
BorrowKind::Shared => (Deep, Read(ReadKind::Borrow(bk))),
948957
BorrowKind::Unique | BorrowKind::Mut { .. } => {
949958
let wk = WriteKind::MutableBorrow(bk);
950-
if self.tcx.sess.two_phase_borrows() {
959+
if self.allow_two_phase_borrow(bk) {
951960
(Deep, Reservation(wk))
952961
} else {
953962
(Deep, Write(wk))

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

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,28 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// revisions: lxl nll
12-
//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
13-
//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
11+
// ignore-tidy-linelength
12+
13+
// revisions: lxl_beyond nll_beyond nll_target
14+
15+
//[lxl_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref
16+
//[nll_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref -Z nll
17+
//[nll_target] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
1418

1519
// This is an important corner case pointed out by Niko: one is
1620
// allowed to initiate a shared borrow during a reservation, but it
1721
// *must end* before the activation occurs.
1822
//
1923
// FIXME: for clarity, diagnostics for these cases might be better off
2024
// if they specifically said "cannot activate mutable borrow of `x`"
25+
//
26+
// The convention for the listed revisions: "lxl" means lexical
27+
// lifetimes (which can be easier to reason about). "nll" means
28+
// non-lexical lifetimes. "nll_target" means the initial conservative
29+
// two-phase borrows that only applies to autoref-introduced borrows.
30+
// "nll_beyond" means the generalization of two-phase borrows to all
31+
// `&mut`-borrows (doing so makes it easier to write code for specific
32+
// corner cases).
2133

2234
#![allow(dead_code)]
2335

@@ -27,35 +39,41 @@ fn ok() {
2739
let mut x = 3;
2840
let y = &mut x;
2941
{ let z = &x; read(z); }
42+
//[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
3043
*y += 1;
3144
}
3245

3346
fn not_ok() {
3447
let mut x = 3;
3548
let y = &mut x;
3649
let z = &x;
50+
//[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
3751
*y += 1;
38-
//[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
39-
//[nll]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
52+
//[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
53+
//[nll_beyond]~^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
54+
//[nll_target]~^^^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
4055
read(z);
4156
}
4257

4358
fn should_be_ok_with_nll() {
4459
let mut x = 3;
4560
let y = &mut x;
4661
let z = &x;
62+
//[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
4763
read(z);
4864
*y += 1;
49-
//[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
50-
// (okay with nll today)
65+
//[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
66+
// (okay with (generalized) nll today)
5167
}
5268

5369
fn should_also_eventually_be_ok_with_nll() {
5470
let mut x = 3;
5571
let y = &mut x;
5672
let _z = &x;
73+
//[nll_target]~^ ERROR cannot borrow `x` as immutable because it is also borrowed as mutable
5774
*y += 1;
58-
//[lxl]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
75+
//[lxl_beyond]~^ ERROR cannot borrow `x` as mutable because it is also borrowed as immutable
76+
// (okay with (generalized) nll today)
5977
}
6078

6179
fn main() { }

src/test/compile-fail/borrowck/two-phase-allow-access-during-reservation.rs

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,28 +8,41 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// revisions: lxl nll
12-
//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
13-
//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
11+
// ignore-tidy-linelength
12+
13+
// revisions: lxl_beyond nll_beyond nll_target
14+
//[lxl_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two_phase_beyond_autoref
15+
//[nll_beyond] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two_phase_beyond_autoref -Z nll
16+
//[nll_target] compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
1417

1518
// This is the second counter-example from Niko's blog post
1619
// smallcultfollowing.com/babysteps/blog/2017/03/01/nested-method-calls-via-two-phase-borrowing/
1720
//
1821
// It is "artificial". It is meant to illustrate directly that we
1922
// should allow an aliasing access during reservation, but *not* while
2023
// the mutable borrow is active.
24+
//
25+
// The convention for the listed revisions: "lxl" means lexical
26+
// lifetimes (which can be easier to reason about). "nll" means
27+
// non-lexical lifetimes. "nll_target" means the initial conservative
28+
// two-phase borrows that only applies to autoref-introduced borrows.
29+
// "nll_beyond" means the generalization of two-phase borrows to all
30+
// `&mut`-borrows (doing so makes it easier to write code for specific
31+
// corner cases).
2132

2233
fn main() {
2334
/*0*/ let mut i = 0;
2435

2536
/*1*/ let p = &mut i; // (reservation of `i` starts here)
2637

2738
/*2*/ let j = i; // OK: `i` is only reserved here
39+
//[nll_target]~^ ERROR cannot use `i` because it was mutably borrowed [E0503]
2840

2941
/*3*/ *p += 1; // (mutable borrow of `i` starts here, since `p` is used)
3042

31-
/*4*/ let k = i; //[lxl]~ ERROR cannot use `i` because it was mutably borrowed [E0503]
32-
//[nll]~^ ERROR cannot use `i` because it was mutably borrowed [E0503]
43+
/*4*/ let k = i; //[lxl_beyond]~ ERROR cannot use `i` because it was mutably borrowed [E0503]
44+
//[nll_beyond]~^ ERROR cannot use `i` because it was mutably borrowed [E0503]
45+
//[nll_target]~^^ ERROR cannot use `i` because it was mutably borrowed [E0503]
3346

3447
/*5*/ *p += 1;
3548

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,13 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
// revisions: lxl nll
12-
//[lxl]compile-flags: -Z borrowck=mir -Z two-phase-borrows
13-
//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
11+
// ignore-tidy-linelength
12+
13+
// revisions: lxl_beyond nll_beyond nll_target
14+
15+
//[lxl_beyond]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref
16+
//[nll_beyond]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z two-phase-beyond-autoref -Z nll
17+
//[nll_target]compile-flags: -Z borrowck=mir -Z two-phase-borrows -Z nll
1418

1519
// This is a corner case that the current implementation is (probably)
1620
// treating more conservatively than is necessary. But it also does
@@ -19,6 +23,18 @@
1923
// So this test is just making a note of the current behavior, with
2024
// the caveat that in the future, the rules may be loosened, at which
2125
// point this test might be thrown out.
26+
//
27+
// The convention for the listed revisions: "lxl" means lexical
28+
// lifetimes (which can be easier to reason about). "nll" means
29+
// non-lexical lifetimes. "nll_target" means the initial conservative
30+
// two-phase borrows that only applies to autoref-introduced borrows.
31+
// "nll_beyond" means the generalization of two-phase borrows to all
32+
// `&mut`-borrows (doing so makes it easier to write code for specific
33+
// corner cases).
34+
//
35+
// FIXME: in "nll_target", we currently see the same error reported
36+
// twice. This is injected by `-Z two-phase-borrows`; not sure why as
37+
// of yet.
2238

2339
fn main() {
2440
let mut vec = vec![0, 1];
@@ -30,8 +46,10 @@ fn main() {
3046
// with the shared borrow. But in the current implementation,
3147
// its an error.
3248
delay = &mut vec;
33-
//[lxl]~^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
34-
//[nll]~^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
49+
//[lxl_beyond]~^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
50+
//[nll_beyond]~^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
51+
//[nll_target]~^^^ ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
52+
//[nll_target]~| ERROR cannot borrow `vec` as mutable because it is also borrowed as immutable
3553

3654
shared[0];
3755
}

0 commit comments

Comments
 (0)