Skip to content

Commit

Permalink
Rollup merge of #55494 - pnkfelix:issue-55492-borrowck-migrate-must-l…
Browse files Browse the repository at this point in the history
…ook-at-parents-of-closures, r=davidtwco

borrowck=migrate must look at parents of closures

This fixes the NLL migration mode (which is the default with edition=2018) to inspect all parents of a closure in addition to the closure itself when looking to see if AST-borrowck issues an error for the given code.

This should be a candidate for beta backport.

Fix #55492
  • Loading branch information
kennytm authored Oct 30, 2018
2 parents 50b8879 + 87ce5ec commit 93f84e5
Show file tree
Hide file tree
Showing 11 changed files with 269 additions and 66 deletions.
28 changes: 27 additions & 1 deletion src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,33 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
mbcx.errors_buffer.sort_by_key(|diag| diag.span.primary_span());

if tcx.migrate_borrowck() {
match tcx.borrowck(def_id).signalled_any_error {
// When borrowck=migrate, check if AST-borrowck would
// error on the given code.

// rust-lang/rust#55492: loop over parents to ensure that
// errors that AST-borrowck only detects in some parent of
// a closure still allows NLL to signal an error.
let mut curr_def_id = def_id;
let signalled_any_error = loop {
match tcx.borrowck(curr_def_id).signalled_any_error {
SignalledError::NoErrorsSeen => {
// keep traversing (and borrow-checking) parents
}
SignalledError::SawSomeError => {
// stop search here
break SignalledError::SawSomeError;
}
}

if tcx.is_closure(curr_def_id) {
curr_def_id = tcx.parent_def_id(curr_def_id)
.expect("a closure must have a parent_def_id");
} else {
break SignalledError::NoErrorsSeen;
}
};

match signalled_any_error {
SignalledError::NoErrorsSeen => {
// if AST-borrowck signalled no errors, then
// downgrade all the buffered MIR-borrowck errors
Expand Down
8 changes: 2 additions & 6 deletions src/test/ui/borrowck/borrowck-closures-unique.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,15 @@ LL | let c2 = || set(x); //~ ERROR two closures require unique access to `x`
LL | c1;
| -- first borrow later used here

warning[E0594]: cannot assign to `x`, as it is not declared as mutable
error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/borrowck-closures-unique.rs:57:38
|
LL | fn e(x: &'static mut isize) {
| - help: consider changing this to be mutable: `mut x`
LL | let c1 = |y: &'static mut isize| x = y; //~ ERROR closure cannot assign to immutable argument
| ^^^^^ cannot assign
|
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

Some errors occurred: E0500, E0524, E0594.
For more information about an error, try `rustc --explain E0500`.
7 changes: 2 additions & 5 deletions src/test/ui/borrowck/borrowck-describe-lvalue.ast.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ LL | //[mir]~^ ERROR cannot borrow `x` as mutable more than o
LL | *y = 1;
| ------ first borrow later used here

warning: captured variable cannot escape `FnMut` closure body
error: captured variable cannot escape `FnMut` closure body
--> $DIR/borrowck-describe-lvalue.rs:305:16
|
LL | || {
Expand All @@ -36,9 +36,6 @@ LL | | }
|
= note: `FnMut` closures only have access to their captured variables while they are executing...
= note: ...therefore, they cannot allow references to captured variables to escape
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

error[E0503]: cannot use `f.x` because it was mutably borrowed
--> $DIR/borrowck-describe-lvalue.rs:53:9
Expand Down Expand Up @@ -382,7 +379,7 @@ LL | drop(x); //[ast]~ ERROR use of moved value: `x`
|
= note: move occurs because `x` has type `std::vec::Vec<i32>`, which does not implement the `Copy` trait

error: aborting due to 29 previous errors
error: aborting due to 30 previous errors

Some errors occurred: E0382, E0499, E0502, E0503.
For more information about an error, try `rustc --explain E0382`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
error[E0595]: closure cannot assign to immutable argument `x`
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:21:22
|
LL | let mut c1 = |y: &'static mut isize| x = y;
| ^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow mutably
help: consider removing the `&mut`, as it is an immutable binding to a mutable reference
|
LL | x
|

error[E0595]: closure cannot assign to immutable argument `x`
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:29:22
|
LL | let mut c1 = |z: &'static mut isize| {
| ^^^^^^^^^^^^^^^^^^^^^^^ cannot borrow mutably
help: consider removing the `&mut`, as it is an immutable binding to a mutable reference
|
LL | x
|

error[E0595]: closure cannot assign to immutable argument `x`
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:40:9
|
LL | pub fn capture_assign_whole(x: (i32,)) {
| - help: make this binding mutable: `mut x`
LL | || { x = (1,); };
| ^^ cannot borrow mutably

error[E0595]: closure cannot assign to immutable argument `x`
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:43:9
|
LL | pub fn capture_assign_part(x: (i32,)) {
| - help: make this binding mutable: `mut x`
LL | || { x.0 = 1; };
| ^^ cannot borrow mutably

error[E0595]: closure cannot assign to immutable argument `x`
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:46:9
|
LL | pub fn capture_reborrow_whole(x: (i32,)) {
| - help: make this binding mutable: `mut x`
LL | || { &mut x; };
| ^^ cannot borrow mutably

error[E0595]: closure cannot assign to immutable argument `x`
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:49:9
|
LL | pub fn capture_reborrow_part(x: (i32,)) {
| - help: make this binding mutable: `mut x`
LL | || { &mut x.0; };
| ^^ cannot borrow mutably

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0595`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:21:46
|
LL | pub fn e(x: &'static mut isize) {
| - help: consider changing this to be mutable: `mut x`
LL | static mut Y: isize = 3;
LL | let mut c1 = |y: &'static mut isize| x = y;
| ^^^^^ cannot assign

error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:30:50
|
LL | pub fn ee(x: &'static mut isize) {
| - help: consider changing this to be mutable: `mut x`
...
LL | let mut c2 = |y: &'static mut isize| x = y;
| ^^^^^ cannot assign

error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:40:14
|
LL | pub fn capture_assign_whole(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { x = (1,); };
| ^^^^^^^^ cannot assign

error[E0594]: cannot assign to `x.0`, as `x` is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:43:14
|
LL | pub fn capture_assign_part(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { x.0 = 1; };
| ^^^^^^^ cannot assign

error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:46:14
|
LL | pub fn capture_reborrow_whole(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { &mut x; };
| ^^^^^^ cannot borrow as mutable

error[E0596]: cannot borrow `x.0` as mutable, as `x` is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:49:14
|
LL | pub fn capture_reborrow_part(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { &mut x.0; };
| ^^^^^^^^ cannot borrow as mutable

error: aborting due to 6 previous errors

Some errors occurred: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:21:46
|
LL | pub fn e(x: &'static mut isize) {
| - help: consider changing this to be mutable: `mut x`
LL | static mut Y: isize = 3;
LL | let mut c1 = |y: &'static mut isize| x = y;
| ^^^^^ cannot assign

error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:30:50
|
LL | pub fn ee(x: &'static mut isize) {
| - help: consider changing this to be mutable: `mut x`
...
LL | let mut c2 = |y: &'static mut isize| x = y;
| ^^^^^ cannot assign

error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:40:14
|
LL | pub fn capture_assign_whole(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { x = (1,); };
| ^^^^^^^^ cannot assign

error[E0594]: cannot assign to `x.0`, as `x` is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:43:14
|
LL | pub fn capture_assign_part(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { x.0 = 1; };
| ^^^^^^^ cannot assign

error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:46:14
|
LL | pub fn capture_reborrow_whole(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { &mut x; };
| ^^^^^^ cannot borrow as mutable

error[E0596]: cannot borrow `x.0` as mutable, as `x` is not declared as mutable
--> $DIR/issue-55492-borrowck-migrate-scans-parents.rs:49:14
|
LL | pub fn capture_reborrow_part(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { &mut x.0; };
| ^^^^^^^^ cannot borrow as mutable

error: aborting due to 6 previous errors

Some errors occurred: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
61 changes: 61 additions & 0 deletions src/test/ui/borrowck/issue-55492-borrowck-migrate-scans-parents.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// rust-lang/rust#55492: errors detected during MIR-borrowck's
// analysis of a closure body may only be caught when AST-borrowck
// looks at some parent.

// revisions: ast migrate nll

// Since we are testing nll (and migration) explicitly as a separate
// revisions, don't worry about the --compare-mode=nll on this test.

// ignore-compare-mode-nll

//[ast]compile-flags: -Z borrowck=ast
//[migrate]compile-flags: -Z borrowck=migrate -Z two-phase-borrows
//[nll]compile-flags: -Z borrowck=mir -Z two-phase-borrows


// transcribed from borrowck-closures-unique.rs
mod borrowck_closures_unique {
pub fn e(x: &'static mut isize) {
static mut Y: isize = 3;
let mut c1 = |y: &'static mut isize| x = y;
unsafe { c1(&mut Y); }
}
}

mod borrowck_closures_unique_grandparent {
pub fn ee(x: &'static mut isize) {
static mut Z: isize = 3;
let mut c1 = |z: &'static mut isize| {
let mut c2 = |y: &'static mut isize| x = y;
c2(z);
};
unsafe { c1(&mut Z); }
}
}

// adapted from mutability_errors.rs
mod mutability_errors {
pub fn capture_assign_whole(x: (i32,)) {
|| { x = (1,); };
}
pub fn capture_assign_part(x: (i32,)) {
|| { x.0 = 1; };
}
pub fn capture_reborrow_whole(x: (i32,)) {
|| { &mut x; };
}
pub fn capture_reborrow_part(x: (i32,)) {
|| { &mut x.0; };
}
}

fn main() {
static mut X: isize = 2;
unsafe { borrowck_closures_unique::e(&mut X); }

mutability_errors::capture_assign_whole((1000,));
mutability_errors::capture_assign_part((2000,));
mutability_errors::capture_reborrow_whole((3000,));
mutability_errors::capture_reborrow_part((4000,));
}
26 changes: 5 additions & 21 deletions src/test/ui/borrowck/mutability-errors.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -277,57 +277,41 @@ LL | &mut x; //~ ERROR
LL | &mut x.0; //~ ERROR
| ^^^^^^^^ cannot borrow as mutable

warning[E0594]: cannot assign to `x`, as it is not declared as mutable
error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/mutability-errors.rs:70:9
|
LL | fn imm_capture(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
LL | || { //~ ERROR
LL | x = (1,);
| ^^^^^^^^ cannot assign
|
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

warning[E0594]: cannot assign to `x.0`, as `x` is not declared as mutable
error[E0594]: cannot assign to `x.0`, as `x` is not declared as mutable
--> $DIR/mutability-errors.rs:71:9
|
LL | fn imm_capture(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
...
LL | x.0 = 1;
| ^^^^^^^ cannot assign
|
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

warning[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
error[E0596]: cannot borrow `x` as mutable, as it is not declared as mutable
--> $DIR/mutability-errors.rs:72:9
|
LL | fn imm_capture(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
...
LL | &mut x;
| ^^^^^^ cannot borrow as mutable
|
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

warning[E0596]: cannot borrow `x.0` as mutable, as `x` is not declared as mutable
error[E0596]: cannot borrow `x.0` as mutable, as `x` is not declared as mutable
--> $DIR/mutability-errors.rs:73:9
|
LL | fn imm_capture(x: (i32,)) {
| - help: consider changing this to be mutable: `mut x`
...
LL | &mut x.0;
| ^^^^^^^^ cannot borrow as mutable
|
= warning: This error has been downgraded to a warning for backwards compatibility with previous releases.
It represents potential unsoundness in your code.
This warning will become a hard error in the future.

error[E0594]: cannot assign to `x`, as it is not declared as mutable
--> $DIR/mutability-errors.rs:76:9
Expand Down Expand Up @@ -389,7 +373,7 @@ error[E0596]: cannot borrow `X.0` as mutable, as `X` is an immutable static item
LL | &mut X.0; //~ ERROR
| ^^^^^^^^ cannot borrow as mutable

error: aborting due to 34 previous errors
error: aborting due to 38 previous errors

Some errors occurred: E0594, E0596.
For more information about an error, try `rustc --explain E0594`.
Loading

0 comments on commit 93f84e5

Please sign in to comment.