Auto merge of rust-lang#54088 - matthewjasper:use-reason-in-dlle-errors, r=pnkfelix

[NLL] Suggest let binding

Closes rust-lang#49821

Also adds an alternative to `explain_why_borrow_contains_point` that allows changing error messages based on the reason that will be given. This will also be useful for rust-lang#51026, rust-lang#51169 and maybe further changes to does not live long enough messages.

Author: bors

src/librustc_mir/borrow_check/error_reporting.rs

@@ -24,6 +24,7 @@ use super::borrow_set::BorrowData;
 use super::{Context, MirBorrowckCtxt};
 use super::{InitializationRequiringAction, PrefixSet};
 
+use borrow_check::nll::explain_borrow::BorrowContainsPointReason;
 use dataflow::drop_flag_effects;
 use dataflow::move_paths::indexes::MoveOutIndex;
 use dataflow::move_paths::MovePathIndex;
@@ -409,6 +410,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         self.access_place_error_reported
             .insert((root_place.clone(), borrow_span));
 
+        let borrow_reason = self.find_why_borrow_contains_point(context, borrow);
+
         let mut err = match &self.describe_place(&borrow.borrowed_place) {
             Some(_) if self.is_place_thread_local(root_place) => {
                 self.report_thread_local_value_does_not_live_long_enough(drop_span, borrow_span)
@@ -418,17 +421,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 name,
                 &scope_tree,
                 &borrow,
+                borrow_reason,
                 drop_span,
                 borrow_span,
-                proper_span,
                 kind.map(|k| (k, place_span.0)),
             ),
             None => self.report_temporary_value_does_not_live_long_enough(
                 context,
                 &scope_tree,
                 &borrow,
+                borrow_reason,
                 drop_span,
-                borrow_span,
                 proper_span,
             ),
         };
@@ -444,16 +447,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         name: &String,
         scope_tree: &Lrc<ScopeTree>,
         borrow: &BorrowData<'tcx>,
+        reason: BorrowContainsPointReason<'tcx>,
         drop_span: Span,
         borrow_span: Span,
-        _proper_span: Span,
         kind_place: Option<(WriteKind, &Place<'tcx>)>,
     ) -> DiagnosticBuilder<'cx> {
         debug!(
             "report_local_value_does_not_live_long_enough(\
-             {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
+             {:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
              )",
-            context, name, scope_tree, borrow, drop_span, borrow_span
+            context, name, scope_tree, borrow, reason, drop_span, borrow_span
         );
 
         let mut err = self.tcx.path_does_not_live_long_enough(
@@ -468,7 +471,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             format!("`{}` dropped here while still borrowed", name),
         );
 
-        self.explain_why_borrow_contains_point(context, borrow, kind_place, &mut err);
+        self.report_why_borrow_contains_point(&mut err, reason, kind_place);
         err
     }
 
@@ -501,15 +504,15 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         context: Context,
         scope_tree: &Lrc<ScopeTree>,
         borrow: &BorrowData<'tcx>,
+        reason: BorrowContainsPointReason<'tcx>,
         drop_span: Span,
-        _borrow_span: Span,
         proper_span: Span,
     ) -> DiagnosticBuilder<'cx> {
         debug!(
             "report_temporary_value_does_not_live_long_enough(\
-             {:?}, {:?}, {:?}, {:?}, {:?}\
+             {:?}, {:?}, {:?}, {:?}, {:?}, {:?}\
              )",
-            context, scope_tree, borrow, drop_span, proper_span
+            context, scope_tree, borrow, reason, drop_span, proper_span
         );
 
         let tcx = self.tcx;
@@ -518,7 +521,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         err.span_label(proper_span, "temporary value does not live long enough");
         err.span_label(drop_span, "temporary value only lives until here");
 
-        self.explain_why_borrow_contains_point(context, borrow, None, &mut err);
+        // Only give this note and suggestion if they could be relevant
+        match reason {
+            BorrowContainsPointReason::Liveness {..}
+            | BorrowContainsPointReason::DropLiveness {..} => {
+                err.note("consider using a `let` binding to create a longer lived value");
+            }
+            BorrowContainsPointReason::OutlivesFreeRegion {..} => (),
+        }
+
+        self.report_why_borrow_contains_point(&mut err, reason, None);
         err
     }
 

src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs

@@ -11,11 +11,28 @@
 use borrow_check::borrow_set::BorrowData;
 use borrow_check::nll::region_infer::Cause;
 use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
-use rustc::mir::{Location, Place, TerminatorKind};
+use rustc::mir::{Local, Location, Place, TerminatorKind};
 use rustc_errors::DiagnosticBuilder;
+use rustc::ty::Region;
 
 mod find_use;
 
+#[derive(Copy, Clone, Debug)]
+pub enum BorrowContainsPointReason<'tcx> {
+    Liveness {
+        local: Local,
+        location: Location,
+        in_loop: bool,
+    },
+    DropLiveness {
+        local: Local,
+        location: Location,
+    },
+    OutlivesFreeRegion {
+        outlived_region: Option<Region<'tcx>>,
+    },
+}
+
 impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     /// Adds annotations to `err` explaining *why* the borrow contains the
     /// point from `context`. This is key for the "3-point errors"
@@ -32,15 +49,30 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
     ///
     /// [d]: https://rust-lang.github.io/rfcs/2094-nll.html#leveraging-intuition-framing-errors-in-terms-of-points
     pub(in borrow_check) fn explain_why_borrow_contains_point(
-        &mut self,
+        &self,
         context: Context,
         borrow: &BorrowData<'tcx>,
         kind_place: Option<(WriteKind, &Place<'tcx>)>,
         err: &mut DiagnosticBuilder<'_>,
     ) {
+        let reason = self.find_why_borrow_contains_point(context, borrow);
+        self.report_why_borrow_contains_point(err, reason, kind_place);
+    }
+
+    /// Finds the reason that [explain_why_borrow_contains_point] will report
+    /// but doesn't add it to any message. This is a separate function in case
+    /// the caller wants to change the error they report based on the reason
+    /// that will be reported.
+    pub(in borrow_check) fn find_why_borrow_contains_point(
+        &self,
+        context: Context,
+        borrow: &BorrowData<'tcx>
+    ) -> BorrowContainsPointReason<'tcx> {
+        use self::BorrowContainsPointReason::*;
+
         debug!(
-            "explain_why_borrow_contains_point(context={:?}, borrow={:?}, kind_place={:?})",
-            context, borrow, kind_place,
+            "find_why_borrow_contains_point(context={:?}, borrow={:?})",
+            context, borrow,
         );
 
         let regioncx = &self.nonlexical_regioncx;
@@ -62,11 +94,45 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         );
 
         match find_use::find(mir, regioncx, tcx, region_sub, context.loc) {
-            Some(Cause::LiveVar(local, location)) => {
+            Some(Cause::LiveVar(local, location)) => Liveness {
+                local,
+                location,
+                in_loop: self.is_borrow_location_in_loop(context.loc),
+            },
+            Some(Cause::DropVar(local, location)) => DropLiveness {
+                local,
+                location,
+            },
+            None => OutlivesFreeRegion {
+                outlived_region: regioncx.to_error_region(region_sub),
+            },
+        }
+    }
+
+    /// Adds annotations to `err` for the explanation `reason`. This is a
+    /// separate method so that the caller can change their error message based
+    /// on the reason that is going to be reported.
+    pub (in borrow_check) fn report_why_borrow_contains_point(
+        &self,
+        err: &mut DiagnosticBuilder,
+        reason: BorrowContainsPointReason<'tcx>,
+        kind_place: Option<(WriteKind, &Place<'tcx>)>,
+    ) {
+        use self::BorrowContainsPointReason::*;
+
+        debug!(
+            "find_why_borrow_contains_point(reason={:?}, kind_place={:?})",
+            reason, kind_place,
+        );
+
+        let mir = self.mir;
+
+        match reason {
+            Liveness { local, location, in_loop } => {
                 let span = mir.source_info(location).span;
                 let spans = self.move_spans(&Place::Local(local), location)
                     .or_else(|| self.borrow_spans(span, location));
-                let message = if self.is_borrow_location_in_loop(context.loc) {
+                let message = if in_loop {
                     if spans.for_closure() {
                         "borrow captured here by closure in later iteration of loop"
                     } else {
@@ -81,8 +147,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 };
                 err.span_label(spans.var_or_use(), message);
             }
-
-            Some(Cause::DropVar(local, location)) => match &mir.local_decls[local].name {
+            DropLiveness { local, location } => match &mir.local_decls[local].name {
                 Some(local_name) => {
                     err.span_label(
                         mir.source_info(location).span,
@@ -93,31 +158,29 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                         if let Place::Local(borrowed_local) = place {
                             let dropped_local_scope = mir.local_decls[local].visibility_scope;
                             let borrowed_local_scope =
-                                mir.local_decls[*borrowed_local].visibility_scope;
+                            mir.local_decls[*borrowed_local].visibility_scope;
 
                             if mir.is_sub_scope(borrowed_local_scope, dropped_local_scope) {
                                 err.note(
-                                    "values in a scope are dropped \
-                                     in the opposite order they are defined",
+                                "values in a scope are dropped \
+                                                     in the opposite order they are defined",
                                 );
                             }
                         }
                     }
                 }
 
                 None => {}
-            },
-
-            None => {
-                if let Some(region) = regioncx.to_error_region(region_sub) {
-                    self.tcx.note_and_explain_free_region(
-                        err,
-                        "borrowed value must be valid for ",
-                        region,
-                        "...",
-                    );
-                }
             }
+            OutlivesFreeRegion { outlived_region: Some(region) } => {
+                self.tcx.note_and_explain_free_region(
+                    err,
+                    "borrowed value must be valid for ",
+                    region,
+                    "...",
+                );
+            }
+            OutlivesFreeRegion { outlived_region: None } => (),
         }
     }
 
@@ -193,3 +256,4 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         false
     }
 }
+

src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue-2.nll.stderr

@@ -8,6 +8,7 @@ LL |     let x = defer(&vec!["Goodbye", "world!"]);
 LL |     x.x[0];
    |     ------ borrow later used here
    |
+   = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
 
 error: aborting due to previous error

src/test/ui/borrowck/borrowck-borrowed-uniq-rvalue.nll.stderr

@@ -8,6 +8,8 @@ LL |     buggy_map.insert(42, &*Box::new(1)); //~ ERROR borrowed value does not
 ...
 LL |     buggy_map.insert(43, &*tmp);
    |     --------- borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-36082.ast.nll.stderr

@@ -8,6 +8,8 @@ LL |     let val: &_ = x.borrow().0;
 ...
 LL |     println!("{}", val);
    |                    --- borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-36082.mir.stderr

@@ -8,6 +8,8 @@ LL |     let val: &_ = x.borrow().0;
 ...
 LL |     println!("{}", val);
    |                    --- borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
 
 error: aborting due to previous error
 

src/test/ui/issues/issue-36082.rs

@@ -28,6 +28,7 @@ fn main() {
     //[mir]~^^^^^ ERROR borrowed value does not live long enough [E0597]
     //[mir]~| NOTE temporary value does not live long enough
     //[mir]~| NOTE temporary value only lives until here
+    //[mir]~| NOTE consider using a `let` binding to create a longer lived value
     println!("{}", val);
     //[mir]~^ borrow later used here
 }

src/test/ui/lifetimes/borrowck-let-suggestion.nll.stderr

@@ -9,6 +9,7 @@ LL |     //~^ ERROR borrowed value does not live long enough
 LL |     x.use_mut();
    |     - borrow later used here
    |
+   = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
 
 error: aborting due to previous error

src/test/ui/nll/borrowed-temporary-error.stderr

@@ -8,6 +8,8 @@ LL |     });
    |       - temporary value only lives until here
 LL |     println!("{:?}", x);
    |                      - borrow later used here
+   |
+   = note: consider using a `let` binding to create a longer lived value
 
 error: aborting due to previous error
 

src/test/ui/regions/regions-var-type-out-of-scope.nll.stderr

@@ -8,6 +8,7 @@ LL |         x = &id(3); //~ ERROR borrowed value does not live long enough
 LL |         assert_eq!(*x, 3);
    |         ------------------ borrow later used here
    |
+   = note: consider using a `let` binding to create a longer lived value
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
 
 error: aborting due to previous error