Auto merge of #52572 - davidtwco:issue-51027, r=nikomatsakis

NLL diagnostics replaced nice closure errors w/ indecipherable free region errors

Fixes #51027.

r? @nikomatsakis

Author: bors

src/librustc/mir/tcx.rs

@@ -119,6 +119,39 @@ impl<'tcx> Place<'tcx> {
                 proj.base.ty(local_decls, tcx).projection_ty(tcx, &proj.elem),
         }
     }
+
+    /// If this is a field projection, and the field is being projected from a closure type,
+    /// then returns the index of the field being projected. Note that this closure will always
+    /// be `self` in the current MIR, because that is the only time we directly access the fields
+    /// of a closure type.
+    pub fn is_upvar_field_projection<'cx, 'gcx>(&self, mir: &'cx Mir<'tcx>,
+                                                tcx: &TyCtxt<'cx, 'gcx, 'tcx>) -> Option<Field> {
+        let place = if let Place::Projection(ref proj) = self {
+            if let ProjectionElem::Deref = proj.elem {
+                &proj.base
+            } else {
+                self
+            }
+        } else {
+            self
+        };
+
+        match place {
+            Place::Projection(ref proj) => match proj.elem {
+                ProjectionElem::Field(field, _ty) => {
+                    let base_ty = proj.base.ty(mir, *tcx).to_ty(*tcx);
+
+                    if  base_ty.is_closure() || base_ty.is_generator() {
+                        Some(field)
+                    } else {
+                        None
+                    }
+                },
+                _ => None,
+            },
+            _ => None,
+        }
+    }
 }
 
 pub enum RvalueInitializationState {

src/librustc_mir/borrow_check/error_reporting.rs

@@ -726,7 +726,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             Place::Projection(ref proj) => {
                 match proj.elem {
                     ProjectionElem::Deref => {
-                        if let Some(field) = self.is_upvar_field_projection(&proj.base) {
+                        let upvar_field_projection = place.is_upvar_field_projection(
+                            self.mir, &self.tcx);
+                        if let Some(field) = upvar_field_projection {
                             let var_index = field.index();
                             let name = self.mir.upvar_decls[var_index].debug_name.to_string();
                             if self.mir.upvar_decls[var_index].by_ref {
@@ -785,7 +787,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     ProjectionElem::Field(field, _ty) => {
                         autoderef = true;
 
-                        if let Some(field) = self.is_upvar_field_projection(place) {
+                        let upvar_field_projection = place.is_upvar_field_projection(
+                            self.mir, &self.tcx);
+                        if let Some(field) = upvar_field_projection {
                             let var_index = field.index();
                             let name = self.mir.upvar_decls[var_index].debug_name.to_string();
                             buf.push_str(&name);

src/librustc_mir/borrow_check/mod.rs

@@ -1214,7 +1214,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                 }
                                 Operand::Move(ref place @ Place::Projection(_))
                                 | Operand::Copy(ref place @ Place::Projection(_)) => {
-                                    if let Some(field) = self.is_upvar_field_projection(place) {
+                                    if let Some(field) = place.is_upvar_field_projection(
+                                            self.mir, &self.tcx) {
                                         self.used_mut_upvars.push(field);
                                     }
                                 }
@@ -1803,7 +1804,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                 place: place @ Place::Projection(_),
                 is_local_mutation_allowed: _,
             } => {
-                if let Some(field) = self.is_upvar_field_projection(&place) {
+                if let Some(field) = place.is_upvar_field_projection(self.mir, &self.tcx) {
                     self.used_mut_upvars.push(field);
                 }
             }
@@ -1866,7 +1867,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                                     // Mutably borrowed data is mutable, but only if we have a
                                     // unique path to the `&mut`
                                     hir::MutMutable => {
-                                        let mode = match self.is_upvar_field_projection(&proj.base)
+                                        let mode = match place.is_upvar_field_projection(
+                                            self.mir, &self.tcx)
                                         {
                                             Some(field)
                                                 if {
@@ -1911,7 +1913,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
                     | ProjectionElem::ConstantIndex { .. }
                     | ProjectionElem::Subslice { .. }
                     | ProjectionElem::Downcast(..) => {
-                        if let Some(field) = self.is_upvar_field_projection(place) {
+                        let upvar_field_projection = place.is_upvar_field_projection(
+                            self.mir, &self.tcx);
+                        if let Some(field) = upvar_field_projection {
                             let decl = &self.mir.upvar_decls[field.index()];
                             debug!(
                                 "decl.mutability={:?} local_mutation_is_allowed={:?} place={:?}",
@@ -1965,28 +1969,6 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
             }
         }
     }
-
-    /// If this is a field projection, and the field is being projected from a closure type,
-    /// then returns the index of the field being projected. Note that this closure will always
-    /// be `self` in the current MIR, because that is the only time we directly access the fields
-    /// of a closure type.
-    fn is_upvar_field_projection(&self, place: &Place<'tcx>) -> Option<Field> {
-        match *place {
-            Place::Projection(ref proj) => match proj.elem {
-                ProjectionElem::Field(field, _ty) => {
-                    let base_ty = proj.base.ty(self.mir, self.tcx).to_ty(self.tcx);
-
-                    if base_ty.is_closure() || base_ty.is_generator() {
-                        Some(field)
-                    } else {
-                        None
-                    }
-                }
-                _ => None,
-            },
-            _ => None,
-        }
-    }
 }
 
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]

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

@@ -22,6 +22,7 @@ use std::fmt;
 use syntax_pos::Span;
 
 mod region_name;
+mod var_name;
 
 /// Constraints that are considered interesting can be categorized to
 /// determine why they are interesting. Order of variants indicates
@@ -30,7 +31,9 @@ mod region_name;
 enum ConstraintCategory {
     Cast,
     Assignment,
+    AssignmentToUpvar,
     Return,
+    CallArgumentToUpvar,
     CallArgument,
     Other,
     Boring,
@@ -39,10 +42,12 @@ enum ConstraintCategory {
 impl fmt::Display for ConstraintCategory {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            ConstraintCategory::Assignment => write!(f, "assignment"),
+            ConstraintCategory::Assignment |
+            ConstraintCategory::AssignmentToUpvar => write!(f, "assignment"),
             ConstraintCategory::Return => write!(f, "return"),
             ConstraintCategory::Cast => write!(f, "cast"),
-            ConstraintCategory::CallArgument => write!(f, "argument"),
+            ConstraintCategory::CallArgument |
+            ConstraintCategory::CallArgumentToUpvar => write!(f, "argument"),
             _ => write!(f, "free region"),
         }
     }
@@ -130,8 +135,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         &self,
         index: ConstraintIndex,
         mir: &Mir<'tcx>,
+        _infcx: &InferCtxt<'_, '_, 'tcx>,
     ) -> (ConstraintCategory, Span) {
         let constraint = self.constraints[index];
+        debug!("classify_constraint: constraint={:?}", constraint);
         let span = constraint.locations.span(mir);
         let location = constraint.locations.from_location().unwrap_or(Location::START);
 
@@ -140,8 +147,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         }
 
         let data = &mir[location.block];
+        debug!("classify_constraint: location={:?} data={:?}", location, data);
         let category = if location.statement_index == data.statements.len() {
             if let Some(ref terminator) = data.terminator {
+                debug!("classify_constraint: terminator.kind={:?}", terminator.kind);
                 match terminator.kind {
                     TerminatorKind::DropAndReplace { .. } => ConstraintCategory::Assignment,
                     TerminatorKind::Call { .. } => ConstraintCategory::CallArgument,
@@ -152,14 +161,17 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             }
         } else {
             let statement = &data.statements[location.statement_index];
+            debug!("classify_constraint: statement.kind={:?}", statement.kind);
             match statement.kind {
                 StatementKind::Assign(ref place, ref rvalue) => {
+                    debug!("classify_constraint: place={:?} rvalue={:?}", place, rvalue);
                     if *place == Place::Local(mir::RETURN_PLACE) {
                         ConstraintCategory::Return
                     } else {
                         match rvalue {
                             Rvalue::Cast(..) => ConstraintCategory::Cast,
-                            Rvalue::Use(..) => ConstraintCategory::Assignment,
+                            Rvalue::Use(..) |
+                            Rvalue::Aggregate(..) => ConstraintCategory::Assignment,
                             _ => ConstraintCategory::Other,
                         }
                     }
@@ -208,7 +220,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
         // Classify each of the constraints along the path.
         let mut categorized_path: Vec<(ConstraintCategory, Span)> = path.iter()
-            .map(|&index| self.classify_constraint(index, mir))
+            .map(|&index| self.classify_constraint(index, mir, infcx))
             .collect();
         debug!("report_error: categorized_path={:?}", categorized_path);
 
@@ -218,30 +230,100 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
         // Get a span
         let (category, span) = categorized_path.first().unwrap();
+
+        let category = match (
+            category,
+            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: category={:?}", category);
+        match category {
+            ConstraintCategory::AssignmentToUpvar |
+            ConstraintCategory::CallArgumentToUpvar =>
+                self.report_closure_error(mir, infcx, mir_def_id, fr, outlived_fr, category, span),
+            _ =>
+                self.report_general_error(mir, infcx, mir_def_id, fr, outlived_fr, category, span),
+        }
+    }
+
+    fn report_closure_error(
+        &self,
+        mir: &Mir<'tcx>,
+        infcx: &InferCtxt<'_, '_, 'tcx>,
+        mir_def_id: DefId,
+        fr: RegionVid,
+        outlived_fr: RegionVid,
+        category: &ConstraintCategory,
+        span: &Span,
+    ) {
+        let fr_name_and_span  = self.get_var_name_and_span_for_region(
+            infcx.tcx, mir, fr);
+        let outlived_fr_name_and_span = self.get_var_name_and_span_for_region(
+            infcx.tcx, mir,outlived_fr);
+
+        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);
+        }
+
+        let diag = &mut infcx.tcx.sess.struct_span_err(
+            *span, &format!("borrowed data escapes outside of closure"),
+        );
+
+        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),
+                );
+            }
+        }
+
+        if let Some((fr_name, fr_span)) = fr_name_and_span {
+            if let Some(name) = fr_name {
+                diag.span_label(
+                    fr_span,
+                    format!("`{}` is a reference that is only valid in the closure body", name),
+                );
+
+                diag.span_label(*span, format!("`{}` escapes the closure body here", name));
+            }
+        }
+
+        diag.emit();
+    }
+
+    fn report_general_error(
+        &self,
+        mir: &Mir<'tcx>,
+        infcx: &InferCtxt<'_, '_, 'tcx>,
+        mir_def_id: DefId,
+        fr: RegionVid,
+        outlived_fr: RegionVid,
+        category: &ConstraintCategory,
+        span: &Span,
+    ) {
         let diag = &mut infcx.tcx.sess.struct_span_err(
-            *span,
-            &format!("unsatisfied lifetime constraints"), // FIXME
+            *span, &format!("unsatisfied lifetime constraints"), // FIXME
         );
 
-        // Figure out how we can refer
         let counter = &mut 1;
-        let fr_name = self.give_region_a_name(infcx.tcx, mir, mir_def_id, fr, counter, diag);
+        let fr_name = self.give_region_a_name(
+            infcx.tcx, mir, mir_def_id, fr, counter, diag);
         let outlived_fr_name = self.give_region_a_name(
-            infcx.tcx,
-            mir,
-            mir_def_id,
-            outlived_fr,
-            counter,
-            diag,
-        );
+            infcx.tcx, mir, mir_def_id, outlived_fr, counter, diag);
 
-        diag.span_label(
-            *span,
-            format!(
-                "{} requires that `{}` must outlive `{}`",
-                category, fr_name, outlived_fr_name,
-            ),
-        );
+        diag.span_label(*span, format!(
+            "{} requires that `{}` must outlive `{}`",
+            category, fr_name, outlived_fr_name,
+        ));
 
         diag.emit();
     }