Use new region infer errors for explaining borrows

This gives at least some explanation for why a borrow is expected to
last for a certain free region. Also:

* Reports E0373: "closure may outlive the current function" with NLL.
* Special cases the case of returning a reference to (or value
  referencing) a local variable or temporary (E0515).
* Special case assigning a reference to a local variable in a closure
  to a captured variable.

Author: matthewjasper

src/librustc_mir/borrow_check/error_reporting.rs

@@ -10,27 +10,35 @@
 
 use borrow_check::borrow_set::BorrowData;
 use borrow_check::error_reporting::UseSpans;
-use borrow_check::nll::region_infer::Cause;
+use borrow_check::nll::ConstraintDescription;
+use borrow_check::nll::region_infer::{Cause, RegionName};
 use borrow_check::{Context, MirBorrowckCtxt, WriteKind};
-use rustc::ty::{self, Region, TyCtxt};
+use rustc::ty::{self, TyCtxt};
 use rustc::mir::{
-    CastKind, FakeReadCause, Local, Location, Mir, Operand, Place, Projection, ProjectionElem,
-    Rvalue, Statement, StatementKind, TerminatorKind
+    CastKind, ConstraintCategory, FakeReadCause, Local, Location, Mir, Operand,
+    Place, Projection, ProjectionElem, Rvalue, Statement, StatementKind,
+    TerminatorKind
 };
 use rustc_errors::DiagnosticBuilder;
 use syntax_pos::Span;
 
 mod find_use;
 
-pub(in borrow_check) enum BorrowExplanation<'tcx> {
+pub(in borrow_check) enum BorrowExplanation {
     UsedLater(LaterUseKind, Span),
     UsedLaterInLoop(LaterUseKind, Span),
     UsedLaterWhenDropped {
         drop_loc: Location,
         dropped_local: Local,
         should_note_order: bool,
     },
-    MustBeValidFor(Region<'tcx>),
+    MustBeValidFor {
+        category: ConstraintCategory,
+        from_closure: bool,
+        span: Span,
+        region_name: RegionName,
+        opt_place_desc: Option<String>,
+    },
     Unexplained,
 }
 
@@ -43,8 +51,8 @@ pub(in borrow_check) enum LaterUseKind {
     Other,
 }
 
-impl<'tcx> BorrowExplanation<'tcx> {
-    pub(in borrow_check) fn add_explanation_to_diagnostic<'cx, 'gcx>(
+impl BorrowExplanation {
+    pub(in borrow_check) fn add_explanation_to_diagnostic<'cx, 'gcx, 'tcx>(
         &self,
         tcx: TyCtxt<'cx, 'gcx, 'tcx>,
         mir: &Mir<'tcx>,
@@ -142,15 +150,27 @@ impl<'tcx> BorrowExplanation<'tcx> {
                         }
                     }
                 }
-            }
-
-            BorrowExplanation::MustBeValidFor(region) => {
-                tcx.note_and_explain_free_region(
-                    err,
-                    &format!("{}{}", borrow_desc, "borrowed value must be valid for "),
-                    region,
-                    "...",
-                );
+            },
+            BorrowExplanation::MustBeValidFor {
+                category,
+                span,
+                ref region_name,
+                ref opt_place_desc,
+                from_closure: _,
+            } => {
+                region_name.highlight_region_name(err);
+
+                if let Some(desc) = opt_place_desc {
+                    err.span_label(span, format!(
+                        "{}requires that `{}` is borrowed for `{}`",
+                        category.description(), desc, region_name,
+                    ));
+                } else {
+                    err.span_label(span, format!(
+                        "{}requires that {}borrow lasts for `{}`",
+                        category.description(), borrow_desc, region_name,
+                    ));
+                };
             },
             _ => {},
         }
@@ -176,7 +196,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
         context: Context,
         borrow: &BorrowData<'tcx>,
         kind_place: Option<(WriteKind, &Place<'tcx>)>,
-    ) -> BorrowExplanation<'tcx> {
+    ) -> BorrowExplanation {
         debug!(
             "explain_why_borrow_contains_point(context={:?}, borrow={:?}, kind_place={:?})",
             context, borrow, kind_place
@@ -241,11 +261,27 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                  }
             }
 
-            None => if let Some(region) = regioncx.to_error_region(region_sub) {
-                BorrowExplanation::MustBeValidFor(region)
+            None => if let Some(region) = regioncx.to_error_region_vid(borrow_region_vid) {
+                let (category, from_closure, span, region_name) = self
+                    .nonlexical_regioncx
+                    .free_region_constraint_info(
+                        self.mir,
+                        self.mir_def_id,
+                        self.infcx,
+                        borrow_region_vid,
+                        region,
+                    );
+                let opt_place_desc = self.describe_place(&borrow.borrowed_place);
+                BorrowExplanation::MustBeValidFor {
+                    category,
+                    from_closure,
+                    span,
+                    region_name,
+                    opt_place_desc,
+                }
             } else {
                 BorrowExplanation::Unexplained
-            },
+            }
         }
     }
 

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

@@ -371,3 +371,7 @@ impl ToRegionVid for RegionVid {
         self
     }
 }
+
+crate trait ConstraintDescription {
+    fn description(&self) -> &'static str;
+}

src/librustc_mir/borrow_check/nll/mod.rs

@@ -8,9 +8,9 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use borrow_check::nll::ConstraintDescription;
 use borrow_check::nll::constraints::{OutlivesConstraint};
 use borrow_check::nll::region_infer::RegionInferenceContext;
-use borrow_check::nll::region_infer::error_reporting::region_name::RegionNameSource;
 use borrow_check::nll::type_check::Locations;
 use borrow_check::nll::universal_regions::DefiningTy;
 use util::borrowck_errors::{BorrowckErrors, Origin};
@@ -29,11 +29,7 @@ use syntax::errors::Applicability;
 mod region_name;
 mod var_name;
 
-use self::region_name::RegionName;
-
-trait ConstraintDescription {
-    fn description(&self) -> &'static str;
-}
+crate use self::region_name::{RegionName, RegionNameSource};
 
 impl ConstraintDescription for ConstraintCategory {
     fn description(&self) -> &'static str {
@@ -76,7 +72,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir: &Mir<'tcx>,
         from_region: RegionVid,
         target_test: impl Fn(RegionVid) -> bool,
-    ) -> (ConstraintCategory, Span, RegionVid) {
+    ) -> (ConstraintCategory, bool, Span) {
         debug!("best_blame_constraint(from_region={:?})", from_region);
 
         // Find all paths
@@ -96,13 +92,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         );
 
         // Classify each of the constraints along the path.
-        let mut categorized_path: Vec<(ConstraintCategory, Span)> = path
+        let mut categorized_path: Vec<(ConstraintCategory, bool, Span)> = path
             .iter()
             .map(|constraint| {
                 if constraint.category == ConstraintCategory::ClosureBounds {
                     self.retrieve_closure_constraint_info(mir, &constraint)
                 } else {
-                    (constraint.category, constraint.locations.span(mir))
+                    (constraint.category, false, constraint.locations.span(mir))
                 }
             })
             .collect();
@@ -147,20 +143,17 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             }
         });
         if let Some(i) = best_choice {
-            let (category, span) = categorized_path[i];
-            return (category, span, target_region);
+            return categorized_path[i]
         }
 
         // If that search fails, that is.. unusual. Maybe everything
         // is in the same SCC or something. In that case, find what
         // appears to be the most interesting point to report to the
         // user via an even more ad-hoc guess.
         categorized_path.sort_by(|p0, p1| p0.0.cmp(&p1.0));
-        debug!("best_blame_constraint: sorted_path={:#?}", categorized_path);
+        debug!("`: sorted_path={:#?}", categorized_path);
 
-        let &(category, span) = categorized_path.first().unwrap();
-
-        (category, span, target_region)
+        *categorized_path.first().unwrap()
     }
 
     /// Walks the graph of constraints (where `'a: 'b` is considered
@@ -247,7 +240,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     ) {
         debug!("report_error(fr={:?}, outlived_fr={:?})", fr, outlived_fr);
 
-        let (category, span, _) = self.best_blame_constraint(
+        let (category, _, span) = self.best_blame_constraint(
             mir,
             fr,
             |r| r == outlived_fr
@@ -580,6 +573,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         }
     }
 
+    crate fn free_region_constraint_info(
+        &self,
+        mir: &Mir<'tcx>,
+        mir_def_id: DefId,
+        infcx: &InferCtxt<'_, '_, 'tcx>,
+        borrow_region: RegionVid,
+        outlived_region: RegionVid,
+    ) -> (ConstraintCategory, bool, Span, RegionName) {
+        let (category, from_closure, span) = self.best_blame_constraint(
+            mir,
+            borrow_region,
+            |r| r == outlived_region
+        );
+        let outlived_fr_name = self.give_region_a_name(
+            infcx, mir, mir_def_id, outlived_region, &mut 1);
+        (category, from_closure, span, outlived_fr_name)
+    }
+
     // Finds some region R such that `fr1: R` and `R` is live at
     // `elem`.
     crate fn find_sub_region_live_at(&self, fr1: RegionVid, elem: Location) -> RegionVid {
@@ -598,24 +609,26 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         fr1: RegionVid,
         fr2: RegionVid,
     ) -> (ConstraintCategory, Span) {
-        let (category, span, _) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
+        let (category, _, span) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
         (category, span)
     }
 
     fn retrieve_closure_constraint_info(
         &self,
         mir: &Mir<'tcx>,
         constraint: &OutlivesConstraint
-    ) -> (ConstraintCategory, Span) {
+    ) -> (ConstraintCategory, bool, Span) {
         let loc = match constraint.locations {
-            Locations::All(span) => return (constraint.category, span),
+            Locations::All(span) => return (constraint.category, false, span),
             Locations::Single(loc) => loc,
         };
 
         let opt_span_category = self
             .closure_bounds_mapping[&loc]
             .get(&(constraint.sup, constraint.sub));
-        *opt_span_category.unwrap_or(&(constraint.category, mir.source_info(loc).span))
+        opt_span_category
+            .map(|&(category, span)| (category, true, span))
+            .unwrap_or((constraint.category, false, mir.source_info(loc).span))
     }
 
     /// Returns `true` if a closure is inferred to be an `FnMut` closure.

src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs

@@ -35,6 +35,7 @@ use std::rc::Rc;
 
 mod dump_mir;
 mod error_reporting;
+crate use self::error_reporting::{RegionName, RegionNameSource};
 mod graphviz;
 pub mod values;
 use self::values::{LivenessValues, RegionValueElements, RegionValues};
@@ -669,13 +670,19 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     /// to find a good name from that. Returns `None` if we can't find
     /// one (e.g., this is just some random part of the CFG).
     pub fn to_error_region(&self, r: RegionVid) -> Option<ty::Region<'tcx>> {
+        self.to_error_region_vid(r).and_then(|r| self.definitions[r].external_name)
+    }
+
+    /// Returns the [RegionVid] corresponding to the region returned by
+    /// `to_error_region`.
+    pub fn to_error_region_vid(&self, r: RegionVid) -> Option<RegionVid> {
         if self.universal_regions.is_universal_region(r) {
-            return self.definitions[r].external_name;
+            Some(r)
         } else {
             let r_scc = self.constraint_sccs.scc(r);
             let upper_bound = self.universal_upper_bound(r);
             if self.scc_values.contains(r_scc, upper_bound) {
-                self.to_error_region(upper_bound)
+                self.to_error_region_vid(upper_bound)
             } else {
                 None
             }

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

@@ -2011,6 +2011,46 @@ match 5u32 {
 ```
 "##,
 
+E0515: r##"
+Cannot return value that references local variable
+
+Local variables, function parameters and temporaries are all dropped before the
+end of the function body. So a reference to them cannot be returned.
+
+```compile_fail,E0515
+#![feature(nll)]
+fn get_dangling_reference() -> &'static i32 {
+    let x = 0;
+    &x
+}
+```
+
+```compile_fail,E0515
+#![feature(nll)]
+use std::slice::Iter;
+fn get_dangling_iterator<'a>() -> Iter<'a, i32> {
+    let v = vec![1, 2, 3];
+    v.iter()
+}
+```
+
+Consider returning an owned value instead:
+
+```
+use std::vec::IntoIter;
+
+fn get_integer() -> i32 {
+    let x = 0;
+    x
+}
+
+fn get_owned_iterator() -> IntoIter<i32> {
+    let v = vec![1, 2, 3];
+    v.into_iter()
+}
+```
+"##,
+
 E0595: r##"
 Closures cannot mutate immutable captured variables.
 

src/librustc_mir/diagnostics.rs

@@ -632,6 +632,31 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
         self.cancel_if_wrong_origin(err, o)
     }
 
+    fn cannot_return_reference_to_local(
+        self,
+        span: Span,
+        reference_desc: &str,
+        path_desc: &str,
+        o: Origin,
+    ) -> DiagnosticBuilder<'cx> {
+        let mut err = struct_span_err!(
+            self,
+            span,
+            E0515,
+            "cannot return {REFERENCE} {LOCAL}{OGN}",
+            REFERENCE=reference_desc,
+            LOCAL=path_desc,
+            OGN = o
+        );
+
+        err.span_label(
+            span,
+            format!("returns a {} data owned by the current function", reference_desc),
+        );
+
+        self.cancel_if_wrong_origin(err, o)
+    }
+
     fn lifetime_too_short_for_reborrow(
         self,
         span: Span,