Auto merge of rust-lang#54798 - matthewjasper:free-region-closure-errors, r=nikomatsakis

[NLL]  Improve closure region bound errors

Previously, we would report free region errors that originate from closure with the span of the closure and a "closure body requires ..." message. This is now updated to use a reason and span from inside the closure.

Author: bors

src/librustc/ich/impls_mir.rs

@@ -551,7 +551,23 @@ impl_stable_hash_for!(struct mir::ClosureRegionRequirements<'tcx> {
 impl_stable_hash_for!(struct mir::ClosureOutlivesRequirement<'tcx> {
     subject,
     outlived_free_region,
-    blame_span
+    blame_span,
+    category
+});
+
+impl_stable_hash_for!(enum mir::ConstraintCategory {
+    Return,
+    TypeAnnotation,
+    Cast,
+    ClosureBounds,
+    CallArgument,
+    CopyBound,
+    SizedBound,
+    Assignment,
+    OpaqueType,
+    Boring,
+    BoringNoLocation,
+    Internal,
 });
 
 impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for mir::ClosureOutlivesSubject<'gcx> {

src/librustc/mir/mod.rs

@@ -2676,11 +2676,51 @@ pub struct ClosureOutlivesRequirement<'tcx> {
     // This region or type ...
     pub subject: ClosureOutlivesSubject<'tcx>,
 
-    // .. must outlive this one.
+    // ... must outlive this one.
     pub outlived_free_region: ty::RegionVid,
 
-    // If not, report an error here.
+    // If not, report an error here ...
     pub blame_span: Span,
+
+    // ... due to this reason.
+    pub category: ConstraintCategory,
+}
+
+/// Outlives constraints can be categorized to determine whether and why they
+/// are interesting (for error reporting). Order of variants indicates sort
+/// order of the category, thereby influencing diagnostic output.
+///
+/// See also [rustc_mir::borrow_check::nll::constraints]
+#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
+pub enum ConstraintCategory {
+    Return,
+    TypeAnnotation,
+    Cast,
+
+    /// A constraint that came from checking the body of a closure.
+    ///
+    /// We try to get the category that the closure used when reporting this.
+    ClosureBounds,
+    CallArgument,
+    CopyBound,
+    SizedBound,
+    Assignment,
+    OpaqueType,
+
+    /// A "boring" constraint (caused by the given location) is one that
+    /// the user probably doesn't want to see described in diagnostics,
+    /// because it is kind of an artifact of the type system setup.
+    /// Example: `x = Foo { field: y }` technically creates
+    /// intermediate regions representing the "type of `Foo { field: y
+    /// }`", and data flows from `y` into those variables, but they
+    /// are not very interesting. The assignment into `x` on the other
+    /// hand might be.
+    Boring,
+    // Boring and applicable everywhere.
+    BoringNoLocation,
+
+    /// A constraint that doesn't correspond to anything the user sees.
+    Internal,
 }
 
 /// The subject of a ClosureOutlivesRequirement -- that is, the thing

src/librustc_mir/borrow_check/nll/constraints/graph.rs

@@ -9,8 +9,9 @@
 // except according to those terms.
 
 use borrow_check::nll::type_check::Locations;
-use borrow_check::nll::constraints::{ConstraintCategory, ConstraintIndex};
+use borrow_check::nll::constraints::ConstraintIndex;
 use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint};
+use rustc::mir::ConstraintCategory;
 use rustc::ty::RegionVid;
 use rustc_data_structures::graph;
 use rustc_data_structures::indexed_vec::IndexVec;

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

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use rustc::mir::ConstraintCategory;
 use rustc::ty::RegionVid;
 use rustc_data_structures::graph::scc::Sccs;
 use rustc_data_structures::indexed_vec::{Idx, IndexVec};
@@ -23,42 +24,6 @@ crate struct ConstraintSet {
     constraints: IndexVec<ConstraintIndex, OutlivesConstraint>,
 }
 
-/// Constraints can be categorized to determine whether and why they are
-/// interesting. Order of variants indicates sort order of the category,
-/// thereby influencing diagnostic output.
-#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash)]
-pub enum ConstraintCategory {
-    Return,
-    TypeAnnotation,
-    Cast,
-    CallArgument,
-
-    /// A constraint that came from checking the body of a closure.
-    ///
-    /// Ideally we would give an explanation that points to the relevant part
-    /// of the closure's body.
-    ClosureBounds,
-    CopyBound,
-    SizedBound,
-    Assignment,
-    OpaqueType,
-
-    /// A "boring" constraint (caused by the given location) is one that
-    /// the user probably doesn't want to see described in diagnostics,
-    /// because it is kind of an artifact of the type system setup.
-    /// Example: `x = Foo { field: y }` technically creates
-    /// intermediate regions representing the "type of `Foo { field: y
-    /// }`", and data flows from `y` into those variables, but they
-    /// are not very interesting. The assignment into `x` on the other
-    /// hand might be.
-    Boring,
-    // Boring and applicable everywhere.
-    BoringNoLocation,
-
-    /// A constraint that doesn't correspond to anything the user sees.
-    Internal,
-}
-
 impl ConstraintSet {
     crate fn push(&mut self, constraint: OutlivesConstraint) {
         debug!(

src/librustc_mir/borrow_check/nll/mod.rs

@@ -138,6 +138,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
     let MirTypeckRegionConstraints {
         mut liveness_constraints,
         outlives_constraints,
+        closure_bounds_mapping,
         type_tests,
     } = constraints;
 
@@ -157,6 +158,7 @@ pub(in borrow_check) fn compute_regions<'cx, 'gcx, 'tcx>(
         universal_region_relations,
         mir,
         outlives_constraints,
+        closure_bounds_mapping,
         type_tests,
         liveness_constraints,
         elements,

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

@@ -8,17 +8,17 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use borrow_check::nll::constraints::{OutlivesConstraint, ConstraintCategory};
+use borrow_check::nll::constraints::{OutlivesConstraint};
 use borrow_check::nll::region_infer::RegionInferenceContext;
+use borrow_check::nll::type_check::Locations;
 use rustc::hir::def_id::DefId;
 use rustc::infer::error_reporting::nice_region_error::NiceRegionError;
 use rustc::infer::InferCtxt;
-use rustc::mir::{Location, Mir};
+use rustc::mir::{ConstraintCategory, Location, Mir};
 use rustc::ty::{self, RegionVid};
 use rustc_data_structures::indexed_vec::IndexVec;
 use rustc_errors::{Diagnostic, DiagnosticBuilder};
 use std::collections::VecDeque;
-use std::fmt;
 use syntax::symbol::keywords;
 use syntax_pos::Span;
 use syntax::errors::Applicability;
@@ -28,22 +28,26 @@ mod var_name;
 
 use self::region_name::RegionName;
 
-impl fmt::Display for ConstraintCategory {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+trait ConstraintDescription {
+    fn description(&self) -> &'static str;
+}
+
+impl ConstraintDescription for ConstraintCategory {
+    fn description(&self) -> &'static str {
         // Must end with a space. Allows for empty names to be provided.
         match self {
-            ConstraintCategory::Assignment => write!(f, "assignment "),
-            ConstraintCategory::Return => write!(f, "returning this value "),
-            ConstraintCategory::Cast => write!(f, "cast "),
-            ConstraintCategory::CallArgument => write!(f, "argument "),
-            ConstraintCategory::TypeAnnotation => write!(f, "type annotation "),
-            ConstraintCategory::ClosureBounds => write!(f, "closure body "),
-            ConstraintCategory::SizedBound => write!(f, "proving this value is `Sized` "),
-            ConstraintCategory::CopyBound => write!(f, "copying this value "),
-            ConstraintCategory::OpaqueType => write!(f, "opaque type "),
+            ConstraintCategory::Assignment => "assignment ",
+            ConstraintCategory::Return => "returning this value ",
+            ConstraintCategory::Cast => "cast ",
+            ConstraintCategory::CallArgument => "argument ",
+            ConstraintCategory::TypeAnnotation => "type annotation ",
+            ConstraintCategory::ClosureBounds => "closure body ",
+            ConstraintCategory::SizedBound => "proving this value is `Sized` ",
+            ConstraintCategory::CopyBound => "copying this value ",
+            ConstraintCategory::OpaqueType => "opaque type ",
             ConstraintCategory::Boring
             | ConstraintCategory::BoringNoLocation
-            | ConstraintCategory::Internal => write!(f, ""),
+            | ConstraintCategory::Internal => "",
         }
     }
 }
@@ -89,7 +93,13 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         // Classify each of the constraints along the path.
         let mut categorized_path: Vec<(ConstraintCategory, Span)> = path
             .iter()
-            .map(|constraint| (constraint.category, constraint.locations.span(mir)))
+            .map(|constraint| {
+                if constraint.category == ConstraintCategory::ClosureBounds {
+                    self.retrieve_closure_constraint_info(mir, &constraint)
+                } else {
+                    (constraint.category, constraint.locations.span(mir))
+                }
+            })
             .collect();
         debug!(
             "best_blame_constraint: categorized_path={:#?}",
@@ -358,7 +368,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             _ => {
                 diag.span_label(span, format!(
                     "{}requires that `{}` must outlive `{}`",
-                    category, fr_name, outlived_fr_name,
+                    category.description(), fr_name, outlived_fr_name,
                 ));
             },
         }
@@ -470,8 +480,24 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         mir: &Mir<'tcx>,
         fr1: RegionVid,
         fr2: RegionVid,
-    ) -> Span {
-        let (_, span, _) = self.best_blame_constraint(mir, fr1, |r| r == fr2);
-        span
+    ) -> (ConstraintCategory, Span) {
+        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) {
+        let loc = match constraint.locations {
+            Locations::All(span) => return (constraint.category, 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))
     }
 }

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

@@ -19,15 +19,17 @@ use rustc::infer::canonical::QueryRegionConstraint;
 use rustc::infer::region_constraints::{GenericKind, VarInfos, VerifyBound};
 use rustc::infer::{InferCtxt, NLLRegionVariableOrigin, RegionVariableOrigin};
 use rustc::mir::{
-    ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements, Local, Location,
-    Mir,
+    ClosureOutlivesRequirement, ClosureOutlivesSubject, ClosureRegionRequirements,
+    ConstraintCategory, Local, Location, Mir,
 };
 use rustc::ty::{self, RegionVid, Ty, TyCtxt, TypeFoldable};
 use rustc::util::common;
 use rustc_data_structures::bit_set::BitSet;
+use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::graph::scc::Sccs;
 use rustc_data_structures::indexed_vec::IndexVec;
 use rustc_errors::{Diagnostic, DiagnosticBuilder};
+use syntax_pos::Span;
 
 use std::rc::Rc;
 
@@ -60,10 +62,16 @@ pub struct RegionInferenceContext<'tcx> {
     /// the SCC (see `constraint_sccs`) and for error reporting.
     constraint_graph: Rc<NormalConstraintGraph>,
 
-    /// The SCC computed from `constraints` and the constraint graph. Used to compute the values
-    /// of each region.
+    /// The SCC computed from `constraints` and the constraint graph. Used to
+    /// compute the values of each region.
     constraint_sccs: Rc<Sccs<RegionVid, ConstraintSccIndex>>,
 
+    /// Map closure bounds to a `Span` that should be used for error reporting.
+    closure_bounds_mapping: FxHashMap<
+        Location,
+        FxHashMap<(RegionVid, RegionVid), (ConstraintCategory, Span)>,
+    >,
+
     /// Contains the minimum universe of any variable within the same
     /// SCC. We will ensure that no SCC contains values that are not
     /// visible from this index.
@@ -187,6 +195,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         universal_region_relations: Rc<UniversalRegionRelations<'tcx>>,
         _mir: &Mir<'tcx>,
         outlives_constraints: ConstraintSet,
+        closure_bounds_mapping: FxHashMap<
+            Location,
+            FxHashMap<(RegionVid, RegionVid), (ConstraintCategory, Span)>,
+        >,
         type_tests: Vec<TypeTest<'tcx>>,
         liveness_constraints: LivenessValues<RegionVid>,
         elements: &Rc<RegionValueElements>,
@@ -220,6 +232,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             constraints,
             constraint_graph,
             constraint_sccs,
+            closure_bounds_mapping,
             scc_universes,
             scc_representatives,
             scc_values,
@@ -727,6 +740,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 subject,
                 outlived_free_region: non_local_ub,
                 blame_span: locations.span(mir),
+                category: ConstraintCategory::Boring,
             };
             debug!("try_promote_type_test: pushing {:#?}", requirement);
             propagated_outlives_requirements.push(requirement);
@@ -1125,7 +1139,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                 longer_fr, shorter_fr,
             );
 
-            let blame_span = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);
+            let blame_span_category = self.find_outlives_blame_span(mir, longer_fr, shorter_fr);
 
             if let Some(propagated_outlives_requirements) = propagated_outlives_requirements {
                 // Shrink `fr` until we find a non-local region (if we do).
@@ -1150,7 +1164,8 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                     propagated_outlives_requirements.push(ClosureOutlivesRequirement {
                         subject: ClosureOutlivesSubject::Region(fr_minus),
                         outlived_free_region: shorter_fr_plus,
-                        blame_span: blame_span,
+                        blame_span: blame_span_category.1,
+                        category: blame_span_category.0,
                     });
                     return;
                 }
@@ -1213,7 +1228,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         };
 
         // Find the code to blame for the fact that `longer_fr` outlives `error_fr`.
-        let span = self.find_outlives_blame_span(mir, longer_fr, error_region);
+        let (_, span) = self.find_outlives_blame_span(mir, longer_fr, error_region);
 
         // Obviously, this error message is far from satisfactory.
         // At present, though, it only appears in unit tests --

src/librustc_mir/borrow_check/nll/type_check/constraint_conversion.rs

@@ -8,7 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use borrow_check::nll::constraints::{ConstraintCategory, ConstraintSet, OutlivesConstraint};
+use borrow_check::nll::constraints::{ConstraintSet, OutlivesConstraint};
 use borrow_check::nll::region_infer::TypeTest;
 use borrow_check::nll::type_check::Locations;
 use borrow_check::nll::universal_regions::UniversalRegions;
@@ -17,6 +17,7 @@ use rustc::infer::outlives::env::RegionBoundPairs;
 use rustc::infer::outlives::obligations::{TypeOutlives, TypeOutlivesDelegate};
 use rustc::infer::region_constraints::{GenericKind, VerifyBound};
 use rustc::infer::{self, SubregionOrigin};
+use rustc::mir::ConstraintCategory;
 use rustc::ty::subst::UnpackedKind;
 use rustc::ty::{self, TyCtxt};
 use syntax_pos::DUMMY_SP;

src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs

@@ -12,11 +12,11 @@ use borrow_check::nll::type_check::constraint_conversion;
 use borrow_check::nll::type_check::{Locations, MirTypeckRegionConstraints};
 use borrow_check::nll::universal_regions::UniversalRegions;
 use borrow_check::nll::ToRegionVid;
-use borrow_check::nll::constraints::ConstraintCategory;
 use rustc::infer::canonical::QueryRegionConstraint;
 use rustc::infer::outlives::free_region_map::FreeRegionRelations;
 use rustc::infer::region_constraints::GenericKind;
 use rustc::infer::InferCtxt;
+use rustc::mir::ConstraintCategory;
 use rustc::traits::query::outlives_bounds::{self, OutlivesBound};
 use rustc::traits::query::type_op::{self, TypeOp};
 use rustc::ty::{self, RegionVid, Ty};