Auto merge of rust-lang#53045 - pnkfelix:issue-53026-migrate-never-lo…

…oser-than-ast-borrowck, r=estebank

Fix NLL migration mode so that reports region errors when necessary.

The code here was trying to be clever, and say "lets not report diagnostics when we 'know' NLL will report an error about them in the future."

The problem is that in migration mode, when no error was reported here, the NLL error that we "knew" was coming was downgraded to a warning (!).

Thus causing rust-lang#53026

(I hope it is the only instance of such a scenario, but we will see.)

Anyway, this PR fixes that by only doing the "clever" skipping of region error reporting when we are not in migration mode. As noted in the FIXME, I'm not really thrilled with this band-aid, but it is small enough to be back-ported easily if that is necessary.

Rather than make a separate test for issue 53026, I just took the test that uncovered this in a first place, and extended it (via our revisions system) to explicitly show all three modes in action: AST-borrowck, NLL, and NLL migration mode.

(To be honest I hope not to have to add such revisions to many tests. Instead I hope to adopt some sort of new `compare-mode` for either borrowck=migrate or for the 2018 edition as a whole.)

Fix rust-lang#53026

src/librustc/infer/error_reporting/mod.rs

@@ -303,7 +303,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     ) {
         debug!("report_region_errors(): {} errors to start", errors.len());
 
-        if will_later_be_reported_by_nll && self.tcx.use_mir_borrowck() {
+        if will_later_be_reported_by_nll &&
+            // FIXME: `use_mir_borrowck` seems wrong here...
+            self.tcx.use_mir_borrowck() &&
+            // ... this is a band-aid; may be better to explicitly
+            // match on every borrowck_mode variant to guide decision
+            // here.
+            !self.tcx.migrate_borrowck() {
+
             // With `#![feature(nll)]`, we want to present a nice user
             // experience, so don't even mention the errors from the
             // AST checker.

src/test/ui/borrowck/issue-45983.stderr → src/test/ui/borrowck/issue-45983.ast.stderr

@@ -1,5 +1,5 @@
 error: borrowed data cannot be stored outside of its closure
-  --> $DIR/issue-45983.rs:17:27
+  --> $DIR/issue-45983.rs:36:27
    |
 LL |     let x = None;
    |         - borrowed data cannot be stored into here...

src/test/ui/borrowck/issue-45983.migrate.stderr

@@ -0,0 +1,12 @@
+error: borrowed data cannot be stored outside of its closure
+  --> $DIR/issue-45983.rs:36:27
+   |
+LL |     let x = None;
+   |         - borrowed data cannot be stored into here...
+LL |     give_any(|y| x = Some(y));
+   |              ---          ^ cannot be stored outside of its closure
+   |              |
+   |              ...because it cannot outlive this closure
+
+error: aborting due to previous error
+

src/test/ui/borrowck/issue-45983.nll.stderr

@@ -1,11 +1,11 @@
 warning: not reporting region error due to nll
-  --> $DIR/issue-45983.rs:17:27
+  --> $DIR/issue-45983.rs:36:27
    |
 LL |     give_any(|y| x = Some(y));
    |                           ^
 
 error: borrowed data escapes outside of closure
-  --> $DIR/issue-45983.rs:17:18
+  --> $DIR/issue-45983.rs:36:18
    |
 LL |     let x = None;
    |         - `x` is declared here, outside of the closure body
@@ -15,7 +15,7 @@ LL |     give_any(|y| x = Some(y));
    |               `y` is a reference that is only valid in the closure body
 
 error[E0594]: cannot assign to `x`, as it is not declared as mutable
-  --> $DIR/issue-45983.rs:17:18
+  --> $DIR/issue-45983.rs:36:18
    |
 LL |     let x = None;
    |         - help: consider changing this to be mutable: `mut x`

src/test/ui/borrowck/issue-45983.rs

@@ -8,12 +8,35 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// As documented in Issue #45983, this test is evaluating the quality
+// of our diagnostics on erroneous code using higher-ranked closures.
+//
+// However, as documented on Issue #53026, this test also became a
+// prime example of our need to test the NLL migration mode
+// *separately* from the existing test suites that focus solely on
+// AST-borrwock and NLL.
+
+// revisions: ast migrate nll
+
+// Since we are testing nll (and migration) explicitly as a separate
+// revisions, dont 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
+
 fn give_any<F: for<'r> FnOnce(&'r ())>(f: F) {
     f(&());
 }
 
 fn main() {
     let x = None;
     give_any(|y| x = Some(y));
-    //~^ ERROR borrowed data cannot be stored outside of its closure
+    //[ast]~^ ERROR borrowed data cannot be stored outside of its closure
+    //[migrate]~^^ ERROR borrowed data cannot be stored outside of its closure
+    //[nll]~^^^ WARN not reporting region error due to nll
+    //[nll]~| ERROR borrowed data escapes outside of closure
+    //[nll]~| ERROR cannot assign to `x`, as it is not declared as mutable
 }