Auto merge of rust-lang#52648 - davidtwco:issue-52533, r=nikomatsakis

[nll] improve the "fully elaborated type" case in region errors

Fixes rust-lang#52533.

r? @nikomatsakis

Author: bors

src/librustc/infer/error_reporting/need_type_info.rs

@@ -74,7 +74,7 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindLocalByTypeVisitor<'a, 'gcx, 'tcx> {
 
 
 impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
-    fn extract_type_name(&self, ty: &'a Ty<'tcx>) -> String {
+    pub fn extract_type_name(&self, ty: &'a Ty<'tcx>) -> String {
         if let ty::TyInfer(ty::TyVar(ty_vid)) = (*ty).sty {
             let ty_vars = self.type_variables.borrow();
             if let TypeVariableOrigin::TypeParameterDefinition(_, name) =

src/librustc/util/ppaux.rs

@@ -19,7 +19,7 @@ use ty::{TyError, TyStr, TyArray, TySlice, TyFloat, TyFnDef, TyFnPtr};
 use ty::{TyParam, TyRawPtr, TyRef, TyNever, TyTuple};
 use ty::{TyClosure, TyGenerator, TyGeneratorWitness, TyForeign, TyProjection, TyAnon};
 use ty::{TyDynamic, TyInt, TyUint, TyInfer};
-use ty::{self, Ty, TyCtxt, TypeFoldable, GenericParamCount, GenericParamDefKind};
+use ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable, GenericParamCount, GenericParamDefKind};
 use util::nodemap::FxHashSet;
 
 use std::cell::Cell;
@@ -32,6 +32,12 @@ use syntax::ast::CRATE_NODE_ID;
 use syntax::symbol::{Symbol, InternedString};
 use hir;
 
+thread_local! {
+    /// Mechanism for highlighting of specific regions for display in NLL region inference errors.
+    /// Contains region to highlight and counter for number to use when highlighting.
+    static HIGHLIGHT_REGION: Cell<Option<(RegionVid, usize)>> = Cell::new(None)
+}
+
 macro_rules! gen_display_debug_body {
     ( $with:path ) => {
         fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
@@ -562,6 +568,19 @@ pub fn parameterized<F: fmt::Write>(f: &mut F,
     PrintContext::new().parameterized(f, substs, did, projections)
 }
 
+fn get_highlight_region() -> Option<(RegionVid, usize)> {
+    HIGHLIGHT_REGION.with(|hr| hr.get())
+}
+
+pub fn with_highlight_region<R>(r: RegionVid, counter: usize, op: impl FnOnce() -> R) -> R {
+    HIGHLIGHT_REGION.with(|hr| {
+        assert_eq!(hr.get(), None);
+        hr.set(Some((r, counter)));
+        let r = op();
+        hr.set(None);
+        r
+    })
+}
 
 impl<'a, T: Print> Print for &'a T {
     fn print<F: fmt::Write>(&self, f: &mut F, cx: &mut PrintContext) -> fmt::Result {
@@ -733,7 +752,7 @@ define_print! {
 define_print! {
     () ty::RegionKind, (self, f, cx) {
         display {
-            if cx.is_verbose {
+            if cx.is_verbose || get_highlight_region().is_some() {
                 return self.print_debug(f, cx);
             }
 
@@ -905,6 +924,15 @@ impl fmt::Debug for ty::FloatVid {
 
 impl fmt::Debug for ty::RegionVid {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        if let Some((region, counter)) = get_highlight_region() {
+            debug!("RegionVid.fmt: region={:?} self={:?} counter={:?}", region, self, counter);
+            return if *self == region {
+                write!(f, "'{:?}", counter)
+            } else {
+                write!(f, "'_")
+            }
+        }
+
         write!(f, "'_#{}r", self.index())
     }
 }
@@ -1022,9 +1050,11 @@ define_print! {
                 TyRef(r, ty, mutbl) => {
                     write!(f, "&")?;
                     let s = r.print_to_string(cx);
-                    write!(f, "{}", s)?;
-                    if !s.is_empty() {
-                        write!(f, " ")?;
+                    if s != "'_" {
+                        write!(f, "{}", s)?;
+                        if !s.is_empty() {
+                            write!(f, " ")?;
+                        }
                     }
                     ty::TypeAndMut { ty, mutbl }.print(f, cx)
                 }

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

@@ -31,26 +31,21 @@ mod var_name;
 enum ConstraintCategory {
     Cast,
     Assignment,
-    AssignmentToUpvar,
     Return,
-    CallArgumentToUpvar,
     CallArgument,
     Other,
     Boring,
 }
 
 impl fmt::Display for ConstraintCategory {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        // Must end with a space. Allows for empty names to be provided.
         match self {
-            ConstraintCategory::Assignment | ConstraintCategory::AssignmentToUpvar => {
-                write!(f, "assignment")
-            }
-            ConstraintCategory::Return => write!(f, "return"),
-            ConstraintCategory::Cast => write!(f, "cast"),
-            ConstraintCategory::CallArgument | ConstraintCategory::CallArgumentToUpvar => {
-                write!(f, "argument")
-            }
-            _ => write!(f, "free region"),
+            ConstraintCategory::Assignment => write!(f, "assignment "),
+            ConstraintCategory::Return => write!(f, "return "),
+            ConstraintCategory::Cast => write!(f, "cast "),
+            ConstraintCategory::CallArgument => write!(f, "argument "),
+            _ => write!(f, ""),
         }
     }
 }
@@ -224,10 +219,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             "constraint_is_interesting: locations={:?} constraint={:?}",
             constraint.locations, constraint
         );
-        if let Locations::Interesting(_) = constraint.locations {
-            true
-        } else {
-            false
+
+        match constraint.locations {
+            Locations::Interesting(_) | Locations::All => true,
+            _ => false,
         }
     }
 
@@ -320,45 +315,26 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             }
         }
 
-        let category = match (
-            category,
+        let (fr_is_local, outlived_fr_is_local): (bool, bool) = (
             self.universal_regions.is_local_free_region(fr),
             self.universal_regions.is_local_free_region(outlived_fr),
-        ) {
-            (ConstraintCategory::Assignment, true, false) => ConstraintCategory::AssignmentToUpvar,
-            (ConstraintCategory::CallArgument, true, false) => {
-                ConstraintCategory::CallArgumentToUpvar
-            }
-            (category, _, _) => category,
+        );
+        debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}",
+               fr_is_local, outlived_fr_is_local, category);
+
+        match (category, fr_is_local, outlived_fr_is_local) {
+            (ConstraintCategory::Assignment, true, false) |
+            (ConstraintCategory::CallArgument, true, false) =>
+                self.report_escaping_data_error(mir, infcx, mir_def_id, fr, outlived_fr,
+                                                category, span, errors_buffer),
+            _ =>
+                self.report_general_error(mir, infcx, mir_def_id, fr, fr_is_local,
+                                          outlived_fr, outlived_fr_is_local,
+                                          category, span, errors_buffer),
         };
-
-        debug!("report_error: category={:?}", category);
-        match category {
-            ConstraintCategory::AssignmentToUpvar | ConstraintCategory::CallArgumentToUpvar => self
-                .report_closure_error(
-                    mir,
-                    infcx,
-                    mir_def_id,
-                    fr,
-                    outlived_fr,
-                    category,
-                    span,
-                    errors_buffer,
-                ),
-            _ => self.report_general_error(
-                mir,
-                infcx,
-                mir_def_id,
-                fr,
-                outlived_fr,
-                category,
-                span,
-                errors_buffer,
-            ),
-        }
     }
 
-    fn report_closure_error(
+    fn report_escaping_data_error(
         &self,
         mir: &Mir<'tcx>,
         infcx: &InferCtxt<'_, '_, 'tcx>,
@@ -373,29 +349,23 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         let outlived_fr_name_and_span =
             self.get_var_name_and_span_for_region(infcx.tcx, mir, outlived_fr);
 
+        let escapes_from = if infcx.tcx.is_closure(mir_def_id) { "closure" } else { "function" };
+
         if fr_name_and_span.is_none() && outlived_fr_name_and_span.is_none() {
-            return self.report_general_error(
-                mir,
-                infcx,
-                mir_def_id,
-                fr,
-                outlived_fr,
-                category,
-                span,
-                errors_buffer,
-            );
+            return self.report_general_error(mir, infcx, mir_def_id,
+                                             fr, true, outlived_fr, false,
+                                             category, span, errors_buffer);
         }
 
-        let mut diag = infcx
-            .tcx
-            .sess
-            .struct_span_err(span, &format!("borrowed data escapes outside of closure"));
+        let mut diag = infcx.tcx.sess.struct_span_err(
+            span, &format!("borrowed data escapes outside of {}", escapes_from),
+        );
 
         if let Some((outlived_fr_name, outlived_fr_span)) = outlived_fr_name_and_span {
             if let Some(name) = outlived_fr_name {
                 diag.span_label(
                     outlived_fr_span,
-                    format!("`{}` is declared here, outside of the closure body", name),
+                    format!("`{}` is declared here, outside of the {} body", name, escapes_from),
                 );
             }
         }
@@ -404,13 +374,12 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             if let Some(name) = fr_name {
                 diag.span_label(
                     fr_span,
-                    format!(
-                        "`{}` is a reference that is only valid in the closure body",
-                        name
-                    ),
+                    format!("`{}` is a reference that is only valid in the {} body",
+                            name, escapes_from),
                 );
 
-                diag.span_label(span, format!("`{}` escapes the closure body here", name));
+                diag.span_label(span, format!("`{}` escapes the {} body here",
+                                               name, escapes_from));
             }
         }
 
@@ -423,7 +392,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         infcx: &InferCtxt<'_, '_, 'tcx>,
         mir_def_id: DefId,
         fr: RegionVid,
+        fr_is_local: bool,
         outlived_fr: RegionVid,
+        outlived_fr_is_local: bool,
         category: ConstraintCategory,
         span: Span,
         errors_buffer: &mut Vec<Diagnostic>,
@@ -434,17 +405,28 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         );
 
         let counter = &mut 1;
-        let fr_name = self.give_region_a_name(infcx.tcx, mir, mir_def_id, fr, counter, &mut diag);
-        let outlived_fr_name =
-            self.give_region_a_name(infcx.tcx, mir, mir_def_id, outlived_fr, counter, &mut diag);
-
-        diag.span_label(
-            span,
-            format!(
-                "{} requires that `{}` must outlive `{}`",
-                category, fr_name, outlived_fr_name,
-            ),
-        );
+        let fr_name = self.give_region_a_name(
+            infcx, mir, mir_def_id, fr, counter, &mut diag);
+        let outlived_fr_name = self.give_region_a_name(
+            infcx, mir, mir_def_id, outlived_fr, counter, &mut diag);
+
+        let mir_def_name = if infcx.tcx.is_closure(mir_def_id) { "closure" } else { "function" };
+
+        match (category, outlived_fr_is_local, fr_is_local) {
+            (ConstraintCategory::Return, true, _) => {
+                diag.span_label(span, format!(
+                    "{} was supposed to return data with lifetime `{}` but it is returning \
+                    data with lifetime `{}`",
+                    mir_def_name, fr_name, outlived_fr_name,
+                ));
+            },
+            _ => {
+                diag.span_label(span, format!(
+                    "{}requires that `{}` must outlive `{}`",
+                    category, fr_name, outlived_fr_name,
+                ));
+            },
+        }
 
         diag.buffer(errors_buffer);
     }