Auto merge of #35732 - jonathandturner:region_error_labels, r=nikomatsakis

Move 'doesn't live long enough' errors to labels

This patch moves the "doesn't live long enough" region-style errors to instead use labels.

An example follows.

Before:

```
error: `x` does not live long enough
  --> src/test/compile-fail/send-is-not-static-ensures-scoping.rs:26:18
   |
26 |         let y = &x;
   |                  ^
   |
note: reference must be valid for the block at 23:10...
  --> src/test/compile-fail/send-is-not-static-ensures-scoping.rs:23:11
   |
23 | fn main() {
   |           ^
note: ...but borrowed value is only valid for the block suffix following statement 0 at 25:18
  --> src/test/compile-fail/send-is-not-static-ensures-scoping.rs:25:19
   |
25 |         let x = 1;
   |                   ^
```

After:

```
error: `x` does not live long enough
  --> src/test/compile-fail/send-is-not-static-ensures-scoping.rs:26:18
   |
26 |         let y = &x;
   |                  ^ does not live long enough
...
32 |     };
   |     - borrowed value only valid until here
...
35 | }
   | - borrowed value must be valid until here
```

r? @nikomatsakis

Author: bors

src/librustc_borrowck/borrowck/gather_loans/lifetime.rs

@@ -96,7 +96,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
         //! Reports an error if `loan_region` is larger than `max_scope`
 
         if !self.bccx.is_subregion_of(self.loan_region, max_scope) {
-            Err(self.report_error(err_out_of_scope(max_scope, self.loan_region)))
+            Err(self.report_error(err_out_of_scope(max_scope, self.loan_region, self.cause)))
         } else {
             Ok(())
         }

src/librustc_borrowck/borrowck/mod.rs

@@ -566,7 +566,7 @@ pub fn opt_loan_path<'tcx>(cmt: &mc::cmt<'tcx>) -> Option<Rc<LoanPath<'tcx>>> {
 #[derive(PartialEq)]
 pub enum bckerr_code {
     err_mutbl,
-    err_out_of_scope(ty::Region, ty::Region), // superscope, subscope
+    err_out_of_scope(ty::Region, ty::Region, euv::LoanCause), // superscope, subscope, loan cause
     err_borrowed_pointer_too_short(ty::Region, ty::Region), // loan, ptr
 }
 
@@ -614,9 +614,9 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
     pub fn report(&self, err: BckError<'tcx>) {
         // Catch and handle some particular cases.
         match (&err.code, &err.cause) {
-            (&err_out_of_scope(ty::ReScope(_), ty::ReStatic),
+            (&err_out_of_scope(ty::ReScope(_), ty::ReStatic, _),
              &BorrowViolation(euv::ClosureCapture(span))) |
-            (&err_out_of_scope(ty::ReScope(_), ty::ReFree(..)),
+            (&err_out_of_scope(ty::ReScope(_), ty::ReFree(..), _),
              &BorrowViolation(euv::ClosureCapture(span))) => {
                 return self.report_out_of_scope_escaping_closure_capture(&err, span);
             }
@@ -963,6 +963,22 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
             .emit();
     }
 
+    fn region_end_span(&self, region: ty::Region) -> Option<Span> {
+        match region {
+            ty::ReScope(scope) => {
+                match scope.span(&self.tcx.region_maps, &self.tcx.map) {
+                    Some(s) => {
+                        Some(s.end_point())
+                    }
+                    None => {
+                        None
+                    }
+                }
+            }
+            _ => None
+        }
+    }
+
     pub fn note_and_explain_bckerr(&self, db: &mut DiagnosticBuilder, err: BckError<'tcx>,
         error_span: Span) {
         let code = err.code;
@@ -1003,19 +1019,65 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
                 }
             }
 
-            err_out_of_scope(super_scope, sub_scope) => {
-                self.tcx.note_and_explain_region(
-                    db,
-                    "reference must be valid for ",
-                    sub_scope,
-                    "...");
-                self.tcx.note_and_explain_region(
-                    db,
-                    "...but borrowed value is only valid for ",
-                    super_scope,
-                    "");
+            err_out_of_scope(super_scope, sub_scope, cause) => {
+                match cause {
+                    euv::ClosureCapture(s) => {
+                        // The primary span starts out as the closure creation point.
+                        // Change the primary span here to highlight the use of the variable
+                        // in the closure, because it seems more natural. Highlight
+                        // closure creation point as a secondary span.
+                        match db.span.primary_span() {
+                            Some(primary) => {
+                                db.span = MultiSpan::from_span(s);
+                                db.span_label(primary, &format!("capture occurs here"));
+                                db.span_label(s, &format!("does not live long enough"));
+                            }
+                            None => ()
+                        }
+                    }
+                    _ => {
+                        db.span_label(error_span, &format!("does not live long enough"));
+                    }
+                }
+
+                let sub_span = self.region_end_span(sub_scope);
+                let super_span = self.region_end_span(super_scope);
+
+                match (sub_span, super_span) {
+                    (Some(s1), Some(s2)) if s1 == s2 => {
+                        db.span_label(s1, &"borrowed value dropped before borrower");
+                        db.note("values in a scope are dropped in the opposite order \
+                                they are created");
+                    }
+                    _ => {
+                        match sub_span {
+                            Some(s) => {
+                                db.span_label(s, &"borrowed value must be valid until here");
+                            }
+                            None => {
+                                self.tcx.note_and_explain_region(
+                                    db,
+                                    "borrowed value must be valid for ",
+                                    sub_scope,
+                                    "...");
+                            }
+                        }
+                        match super_span {
+                            Some(s) => {
+                                db.span_label(s, &"borrowed value only valid until here");
+                            }
+                            None => {
+                                self.tcx.note_and_explain_region(
+                                    db,
+                                    "...but borrowed value is only valid for ",
+                                    super_scope,
+                                    "");
+                            }
+                        }
+                    }
+                }
+
                 if let Some(span) = statement_scope_span(self.tcx, super_scope) {
-                    db.span_label(error_span, &format!("does not live long enough"));
                     db.span_help(span,
                                  "consider using a `let` binding to increase its lifetime");
                 }

src/test/compile-fail/borrowck/borrowck-let-suggestion-suffixes.rs

@@ -13,47 +13,48 @@ fn f() {
     let mut v1 = Vec::new(); // statement 1
 
     let mut v2 = Vec::new(); // statement 2
-    //~^ NOTE reference must be valid for the block suffix following statement 2
 
     let young = ['y'];       // statement 3
-    //~^ NOTE ...but borrowed value is only valid for the block suffix following statement 3
 
     v2.push(&young[0]);      // statement 4
     //~^ ERROR `young[..]` does not live long enough
+    //~| NOTE does not live long enough
+    //~| NOTE values in a scope are dropped in the opposite order they are created
 
     let mut v3 = Vec::new(); // statement 5
-    //~^ NOTE reference must be valid for the block suffix following statement 5
 
     v3.push(&'x');           // statement 6
     //~^ ERROR borrowed value does not live long enough
-    //~| does not live long enough
-    //~| NOTE ...but borrowed value is only valid for the statement
+    //~| NOTE does not live long enough
+    //~| NOTE borrowed value only valid until here
     //~| HELP consider using a `let` binding to increase its lifetime
 
     {
 
         let mut v4 = Vec::new(); // (sub) statement 0
-        //~^ NOTE reference must be valid for the block suffix following statement 0
 
         v4.push(&'y');
         //~^ ERROR borrowed value does not live long enough
-        //~| does not live long enough
-        //~| NOTE ...but borrowed value is only valid for the statement
+        //~| NOTE does not live long enough
+        //~| NOTE borrowed value only valid until here
         //~| HELP consider using a `let` binding to increase its lifetime
 
     }                       // (statement 7)
+    //~^ NOTE borrowed value must be valid until here
 
     let mut v5 = Vec::new(); // statement 8
-    //~^ NOTE reference must be valid for the block suffix following statement 8
 
     v5.push(&'z');
     //~^ ERROR borrowed value does not live long enough
-    //~| does not live long enough
-    //~| NOTE ...but borrowed value is only valid for the statement
+    //~| NOTE does not live long enough
+    //~| NOTE borrowed value only valid until here
     //~| HELP consider using a `let` binding to increase its lifetime
 
     v1.push(&old[0]);
 }
+//~^ NOTE borrowed value dropped before borrower
+//~| NOTE borrowed value must be valid until here
+//~| NOTE borrowed value must be valid until here
 
 fn main() {
     f();

src/test/compile-fail/borrowck/borrowck-let-suggestion.rs

@@ -11,11 +11,11 @@
 fn f() {
     let x = [1].iter();
     //~^ ERROR borrowed value does not live long enough
-    //~|does not live long enough
-    //~| NOTE reference must be valid for the block suffix following statement
+    //~| NOTE does not live long enough
+    //~| NOTE borrowed value only valid until here
     //~| HELP consider using a `let` binding to increase its lifetime
-    //~| NOTE ...but borrowed value is only valid for the statement at 12:4
 }
+//~^ borrowed value must be valid until here
 
 fn main() {
     f();

src/test/compile-fail/impl-trait/loan-extend.rs

@@ -14,10 +14,10 @@
 fn borrow<'a, T>(_: &'a mut T) -> impl Copy { () }
 
 fn main() {
-    //~^ NOTE reference must be valid for the block
     let long;
     let mut short = 0;
-    //~^ NOTE but borrowed value is only valid for the block suffix following statement 1
     long = borrow(&mut short);
     //~^ ERROR `short` does not live long enough
-}
+    //~| NOTE does not live long enough
+    //~| NOTE values in a scope are dropped in the opposite order they are created
+} //~ borrowed value dropped before borrower

src/test/compile-fail/region-borrow-params-issue-29793-small.rs

@@ -16,15 +16,19 @@
 
 fn escaping_borrow_of_closure_params_1() {
     let g = |x: usize, y:usize| {
-        //~^ NOTE reference must be valid for the scope of call-site for function
-        //~| NOTE ...but borrowed value is only valid for the scope of function body
-        //~| NOTE reference must be valid for the scope of call-site for function
-        //~| NOTE ...but borrowed value is only valid for the scope of function body
         let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`)
         //~^ ERROR `x` does not live long enough
         //~| ERROR `y` does not live long enough
+        //~| NOTE capture occurs here
+        //~| NOTE capture occurs here
+        //~| NOTE does not live long enough
+        //~| NOTE does not live long enough
+        //~| NOTE values in a scope are dropped in the opposite order they are created
+        //~| NOTE values in a scope are dropped in the opposite order they are created
         return f;
     };
+    //~^ NOTE borrowed value dropped before borrower
+    //~| NOTE borrowed value dropped before borrower
 
     // We delberately do not call `g`; this small version of the test,
     // after adding such a call, was (properly) rejected even when the
@@ -35,15 +39,19 @@ fn escaping_borrow_of_closure_params_1() {
 
 fn escaping_borrow_of_closure_params_2() {
     let g = |x: usize, y:usize| {
-        //~^ NOTE reference must be valid for the scope of call-site for function
-        //~| NOTE ...but borrowed value is only valid for the scope of function body
-        //~| NOTE reference must be valid for the scope of call-site for function
-        //~| NOTE ...but borrowed value is only valid for the scope of function body
         let f = |t: bool| if t { x } else { y }; // (separate errors for `x` vs `y`)
         //~^ ERROR `x` does not live long enough
         //~| ERROR `y` does not live long enough
+        //~| NOTE capture occurs here
+        //~| NOTE capture occurs here
+        //~| NOTE does not live long enough
+        //~| NOTE does not live long enough
+        //~| NOTE values in a scope are dropped in the opposite order they are created
+        //~| NOTE values in a scope are dropped in the opposite order they are created
         f
     };
+    //~^ NOTE borrowed value dropped before borrower
+    //~| NOTE borrowed value dropped before borrower
 
     // (we don't call `g`; see above)
 }

src/test/compile-fail/send-is-not-static-ensures-scoping.rs

@@ -26,8 +26,8 @@ fn main() {
         let y = &x; //~ ERROR `x` does not live long enough
 
         scoped(|| {
-            //~^ ERROR `y` does not live long enough
             let _z = y;
+            //~^ ERROR `y` does not live long enough
         })
     };
 

src/test/compile-fail/unboxed-closures-failed-recursive-fn-1.rs

@@ -22,8 +22,8 @@ fn a() {
     let mut factorial: Option<Box<Fn(u32) -> u32>> = None;
 
     let f = |x: u32| -> u32 {
-        //~^ ERROR `factorial` does not live long enough
         let g = factorial.as_ref().unwrap();
+        //~^ ERROR `factorial` does not live long enough
         if x == 0 {1} else {x * g(x-1)}
     };
 

src/test/compile-fail/issue-11925.rs renamed to src/test/ui/span/issue-11925.rs

@@ -15,7 +15,7 @@ fn to_fn_once<A,F:FnOnce<A>>(f: F) -> F { f }
 fn main() {
     let r = {
         let x: Box<_> = box 42;
-        let f = to_fn_once(move|| &x); //~ ERROR: `x` does not live long enough
+        let f = to_fn_once(move|| &x);
         f()
     };
 

src/test/ui/span/issue-11925.stderr

@@ -0,0 +1,14 @@
+error: `x` does not live long enough
+  --> $DIR/issue-11925.rs:18:36
+   |
+18 |         let f = to_fn_once(move|| &x);
+   |                                    ^
+   |                                    |
+   |                                    does not live long enough
+   |                                    borrowed value only valid until here
+...
+23 | }
+   | - borrowed value must be valid until here
+
+error: aborting due to previous error
+