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

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, _) => {
+            hir::ExprKind::Closure(_, _, body_id, fn_decl_span, _) => {
-                self.walk_captures(expr, fn_decl_span)
+                self.delegate.nested_body(body_id);
+                self.walk_captures(expr, fn_decl_span);
             }
 
             hir::ExprKind::Box(ref base) => {

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.

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
+            // rust-lang/rust#55492, rust-lang/rust#58776 check the base def id
-            // errors that AST-borrowck only detects in some parent of
+            // for errors. AST borrowck is responsible for aggregating
-            // a closure still allows NLL to signal an error.
+            // `signalled_any_error` from all of the nested closures here.
-            let mut curr_def_id = def_id;
+            let base_def_id = tcx.closure_base_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) {
+            match tcx.borrowck(base_def_id).signalled_any_error {
-                    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

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`.

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`.

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`.

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);
+}