Skip to content

Commit

Permalink
Rollup merge of rust-lang#58788 - matthewjasper:compare-children, r=p…
Browse files Browse the repository at this point in the history
…nkfelix

Make migrate mode work at item level granularity

Migrate mode now works entirely at the item level rather than the body level,
ensuring that we don't lose any errors in contained closures.

Closes rust-lang#58776

r? @pnkfelix
  • Loading branch information
pietroalbini authored Mar 1, 2019
2 parents 9bbd413 + a046c38 commit ba87c3d
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 25 deletions.
8 changes: 6 additions & 2 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ pub trait Delegate<'tcx> {
assignment_span: Span,
assignee_cmt: &mc::cmt_<'tcx>,
mode: MutateMode);

// A nested closure or generator - only one layer deep.
fn nested_body(&mut self, _body_id: hir::BodyId) {}
}

#[derive(Copy, Clone, PartialEq, Debug)]
Expand Down Expand Up @@ -532,8 +535,9 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> {
self.consume_expr(&base);
}

hir::ExprKind::Closure(.., fn_decl_span, _) => {
self.walk_captures(expr, fn_decl_span)
hir::ExprKind::Closure(_, _, body_id, fn_decl_span, _) => {
self.delegate.nested_body(body_id);
self.walk_captures(expr, fn_decl_span);
}

hir::ExprKind::Box(ref base) => {
Expand Down
16 changes: 16 additions & 0 deletions src/librustc_borrowck/borrowck/check_loans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,22 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for CheckLoanCtxt<'a, 'tcx> {
}

fn decl_without_init(&mut self, _id: ast::NodeId, _span: Span) { }

fn nested_body(&mut self, body_id: hir::BodyId) {
// rust-lang/rust#58776: MIR and AST borrow check disagree on where
// certain closure errors are reported. As such migrate borrowck has to
// operate at the level of items, rather than bodies. Check if the
// contained closure had any errors and set `signalled_any_error` if it
// has.
let bccx = self.bccx;
if bccx.tcx.migrate_borrowck() {
if let SignalledError::NoErrorsSeen = bccx.signalled_any_error.get() {
let closure_def_id = bccx.tcx.hir().body_owner_def_id(body_id);

bccx.signalled_any_error.set(bccx.tcx.borrowck(closure_def_id).signalled_any_error);
}
}
}
}

pub fn check_loans<'a, 'b, 'c, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
Expand Down
28 changes: 5 additions & 23 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,30 +329,12 @@ fn do_mir_borrowck<'a, 'gcx, 'tcx>(
// 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;
}
};
// rust-lang/rust#55492, rust-lang/rust#58776 check the base def id
// for errors. AST borrowck is responsible for aggregating
// `signalled_any_error` from all of the nested closures here.
let base_def_id = tcx.closure_base_def_id(def_id);

match signalled_any_error {
match tcx.borrowck(base_def_id).signalled_any_error {
SignalledError::NoErrorsSeen => {
// if AST-borrowck signalled no errors, then
// downgrade all the buffered MIR-borrowck errors
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
error[E0597]: `**greeting` does not live long enough
--> $DIR/issue-58776-borrowck-scans-children.rs:10:24
|
LL | let res = (|| (|| &greeting)())();
| -- ^^^^^^^^ - borrowed value only lives until here
| | |
| | borrowed value does not live long enough
| capture occurs here
...
LL | }
| - borrowed value needs to live until here

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error[E0506]: cannot assign to `greeting` because it is borrowed
--> $DIR/issue-58776-borrowck-scans-children.rs:13:5
|
LL | let res = (|| (|| &greeting)())();
| -- -------- borrow occurs due to use in closure
| |
| borrow of `greeting` occurs here
...
LL | greeting = "DEALLOCATED".to_string();
| ^^^^^^^^ assignment to borrowed `greeting` occurs here
...
LL | println!("thread result: {:?}", res);
| --- borrow later used here

error[E0505]: cannot move out of `greeting` because it is borrowed
--> $DIR/issue-58776-borrowck-scans-children.rs:16:10
|
LL | let res = (|| (|| &greeting)())();
| -- -------- borrow occurs due to use in closure
| |
| borrow of `greeting` occurs here
...
LL | drop(greeting);
| ^^^^^^^^ move out of `greeting` occurs here
...
LL | println!("thread result: {:?}", res);
| --- borrow later used here

error: aborting due to 2 previous errors

Some errors occurred: E0505, E0506.
For more information about an error, try `rustc --explain E0505`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
error[E0506]: cannot assign to `greeting` because it is borrowed
--> $DIR/issue-58776-borrowck-scans-children.rs:13:5
|
LL | let res = (|| (|| &greeting)())();
| -- -------- borrow occurs due to use in closure
| |
| borrow of `greeting` occurs here
...
LL | greeting = "DEALLOCATED".to_string();
| ^^^^^^^^ assignment to borrowed `greeting` occurs here
...
LL | println!("thread result: {:?}", res);
| --- borrow later used here

error[E0505]: cannot move out of `greeting` because it is borrowed
--> $DIR/issue-58776-borrowck-scans-children.rs:16:10
|
LL | let res = (|| (|| &greeting)())();
| -- -------- borrow occurs due to use in closure
| |
| borrow of `greeting` occurs here
...
LL | drop(greeting);
| ^^^^^^^^ move out of `greeting` occurs here
...
LL | println!("thread result: {:?}", res);
| --- borrow later used here

error: aborting due to 2 previous errors

Some errors occurred: E0505, E0506.
For more information about an error, try `rustc --explain E0505`.
21 changes: 21 additions & 0 deletions src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// ignore-compare-mode-nll

// revisions: ast migrate nll

//[migrate]compile-flags: -Z borrowck=migrate
#![cfg_attr(nll, feature(nll))]

fn main() {
let mut greeting = "Hello world!".to_string();
let res = (|| (|| &greeting)())();
//[ast]~^ ERROR does not live long enough

greeting = "DEALLOCATED".to_string();
//[migrate]~^ ERROR cannot assign
//[nll]~^^ ERROR cannot assign
drop(greeting);
//[migrate]~^ ERROR cannot move
//[nll]~^^ ERROR cannot move

println!("thread result: {:?}", res);
}

0 comments on commit ba87c3d

Please sign in to comment.