Auto merge of #58788 - matthewjasper:compare-children, r=pnkfelix

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 #58776

r? @pnkfelix

7 files changed, 129 insertions, 25 deletions

diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs
index 78a9e406c95..8ada67efaaf 100644
--- a/src/librustc/middle/expr_use_visitor.rs
+++ b/src/librustc/middle/expr_use_visitor.rs
@@ -82,6 +82,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)]
@@ -531,8 +534,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) => {
diff --git a/src/librustc_borrowck/borrowck/gather_loans/mod.rs b/src/librustc_borrowck/borrowck/gather_loans/mod.rs
index bf730ba41f4..1e3364ecb9a 100644
--- a/src/librustc_borrowck/borrowck/gather_loans/mod.rs
+++ b/src/librustc_borrowck/borrowck/gather_loans/mod.rs
@@ -153,6 +153,24 @@ impl<'a, 'tcx> euv::Delegate<'tcx> for GatherLoanCtxt<'a, 'tcx> {
                      .node_type(id);
         gather_moves::gather_decl(self.bccx, &self.move_data, id, ty);
     }
+
+    fn nested_body(&mut self, body_id: hir::BodyId) {
+        debug!("nested_body(body_id={:?})", body_id);
+        // 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);
+                debug!("checking closure: {:?}", closure_def_id);
+
+                bccx.signalled_any_error.set(bccx.tcx.borrowck(closure_def_id).signalled_any_error);
+            }
+        }
+    }
 }
 
 /// Implements the A-* rules in README.md.
diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs
index d5dfdf0add5..0bdf44c2ae0 100644
--- a/src/librustc_mir/borrow_check/mod.rs
+++ b/src/librustc_mir/borrow_check/mod.rs
@@ -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
diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr
new file mode 100644
index 00000000000..9e0b0aac1e3
--- /dev/null
+++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.ast.stderr
@@ -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`.
diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr
new file mode 100644
index 00000000000..bd8f2286f17
--- /dev/null
+++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.migrate.stderr
@@ -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`.
diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr
new file mode 100644
index 00000000000..bd8f2286f17
--- /dev/null
+++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.nll.stderr
@@ -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`.
diff --git a/src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs
new file mode 100644
index 00000000000..378969f9a18
--- /dev/null
+++ b/src/test/ui/borrowck/issue-58776-borrowck-scans-children.rs
@@ -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);
+}