Improve closure region bound errors

Now use the category and span that are associated to the most
interesting bound that led to the closure bound.

Author: matthewjasper

src/librustc/mir/mod.rs

@@ -2639,11 +2639,14 @@ 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

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

@@ -10,6 +10,7 @@
 
 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;
@@ -18,7 +19,6 @@ 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;
@@ -93,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={:#?}",
@@ -474,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

@@ -25,9 +25,11 @@ use rustc::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/mod.rs

@@ -42,15 +42,15 @@ use rustc::traits::query::type_op::custom::CustomTypeOp;
 use rustc::traits::query::{Fallible, NoSolution};
 use rustc::traits::{ObligationCause, PredicateObligations};
 use rustc::ty::fold::TypeFoldable;
-use rustc::ty::subst::Subst;
+use rustc::ty::subst::{Subst, UnpackedKind};
 use rustc::ty::{self, CanonicalTy, RegionVid, ToPolyTraitRef, Ty, TyCtxt, TyKind};
 use std::rc::Rc;
 use std::{fmt, iter};
 use syntax_pos::{Span, DUMMY_SP};
 use transform::{MirPass, MirSource};
 
 use either::Either;
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 
 macro_rules! span_mirbug {
     ($context:expr, $elem:expr, $($message:tt)*) => ({
@@ -128,6 +128,7 @@ pub(crate) fn type_check<'gcx, 'tcx>(
     let mut constraints = MirTypeckRegionConstraints {
         liveness_constraints: LivenessValues::new(elements),
         outlives_constraints: ConstraintSet::default(),
+        closure_bounds_mapping: FxHashMap(),
         type_tests: Vec::default(),
     };
     let mut placeholder_indices = PlaceholderIndices::default();
@@ -752,6 +753,11 @@ crate struct MirTypeckRegionConstraints<'tcx> {
 
     crate outlives_constraints: ConstraintSet,
 
+    crate closure_bounds_mapping: FxHashMap<
+        Location,
+        FxHashMap<(RegionVid, RegionVid), (ConstraintCategory, Span)>,
+    >,
+
     crate type_tests: Vec<TypeTest<'tcx>>,
 }
 
@@ -860,7 +866,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
         &mut self,
         locations: Locations,
         category: ConstraintCategory,
-        op: impl type_op::TypeOp<'gcx, 'tcx, Output = R>,
+        op: impl type_op::TypeOp<'gcx, 'tcx, Output=R>,
     ) -> Fallible<R> {
         let (r, opt_data) = op.fully_perform(self.infcx)?;
 
@@ -1103,17 +1109,17 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
                 let place_ty = place.ty(mir, tcx).to_ty(tcx);
                 let rv_ty = rv.ty(mir, tcx);
                 if let Err(terr) =
-                    self.sub_types_or_anon(rv_ty, place_ty, location.to_locations(), category)
-                {
-                    span_mirbug!(
+                self.sub_types_or_anon(rv_ty, place_ty, location.to_locations(), category)
+                    {
+                        span_mirbug!(
                         self,
                         stmt,
                         "bad assignment ({:?} = {:?}): {:?}",
                         place_ty,
                         rv_ty,
                         terr
                     );
-                }
+                    }
 
                 if let Some(user_ty) = self.rvalue_user_ty(rv) {
                     if let Err(terr) = self.relate_type_and_user_type(
@@ -1233,17 +1239,17 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
 
                 let locations = term_location.to_locations();
                 if let Err(terr) =
-                    self.sub_types(rv_ty, place_ty, locations, ConstraintCategory::Assignment)
-                {
-                    span_mirbug!(
+                self.sub_types(rv_ty, place_ty, locations, ConstraintCategory::Assignment)
+                    {
+                        span_mirbug!(
                         self,
                         term,
                         "bad DropAndReplace ({:?} = {:?}): {:?}",
                         place_ty,
                         rv_ty,
                         terr
                     );
-                }
+                    }
             }
             TerminatorKind::SwitchInt {
                 ref discr,
@@ -1387,17 +1393,17 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
                 let locations = term_location.to_locations();
 
                 if let Err(terr) =
-                    self.sub_types_or_anon(sig.output(), dest_ty, locations, category)
-                {
-                    span_mirbug!(
+                self.sub_types_or_anon(sig.output(), dest_ty, locations, category)
+                    {
+                        span_mirbug!(
                         self,
                         term,
                         "call dest mismatch ({:?} <- {:?}): {:?}",
                         dest_ty,
                         sig.output(),
                         terr
                     );
-                }
+                    }
 
                 // When `#![feature(unsized_locals)]` is not enabled,
                 // this check is done at `check_local`.
@@ -2038,7 +2044,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
             aggregate_kind, location
         );
 
-        let instantiated_predicates = match aggregate_kind {
+        let instantiated_predicates = match aggregate_kind  {
             AggregateKind::Adt(def, _, substs, _, _) => {
                 tcx.predicates_of(def.did).instantiate(tcx, substs)
             }
@@ -2064,24 +2070,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
             // these extra requirements are basically like where
             // clauses on the struct.
             AggregateKind::Closure(def_id, substs) => {
-                if let Some(closure_region_requirements) =
-                    tcx.mir_borrowck(*def_id).closure_requirements
-                {
-                    let closure_constraints = closure_region_requirements.apply_requirements(
-                        self.infcx.tcx,
-                        location,
-                        *def_id,
-                        *substs,
-                    );
-
-                    self.push_region_constraints(
-                        location.to_locations(),
-                        ConstraintCategory::ClosureBounds,
-                        &closure_constraints,
-                    );
-                }
-
-                tcx.predicates_of(*def_id).instantiate(tcx, substs.substs)
+                self.prove_closure_bounds(tcx, *def_id, *substs, location)
             }
 
             AggregateKind::Generator(def_id, substs, _) => {
@@ -2097,6 +2086,72 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
         );
     }
 
+    fn prove_closure_bounds(
+        &mut self,
+        tcx: TyCtxt<'a, 'gcx, 'tcx>,
+        def_id: DefId,
+        substs: ty::ClosureSubsts<'tcx>,
+        location: Location,
+    ) -> ty::InstantiatedPredicates<'tcx> {
+        if let Some(closure_region_requirements) =
+            tcx.mir_borrowck(def_id).closure_requirements
+        {
+            let closure_constraints = closure_region_requirements.apply_requirements(
+                tcx,
+                location,
+                def_id,
+                substs,
+            );
+
+            if let Some(ref mut borrowck_context) = self.borrowck_context {
+                let bounds_mapping = closure_constraints
+                    .iter()
+                    .enumerate()
+                    .filter_map(|(idx, constraint)| {
+                        let ty::OutlivesPredicate(k1, r2) =
+                            constraint.no_late_bound_regions().unwrap_or_else(|| {
+                                bug!(
+                                    "query_constraint {:?} contained bound regions",
+                                    constraint,
+                                );
+                            });
+
+                        match k1.unpack() {
+                            UnpackedKind::Lifetime(r1) => {
+                                // constraint is r1: r2
+                                let r1_vid = borrowck_context.universal_regions.to_region_vid(r1);
+                                let r2_vid = borrowck_context.universal_regions.to_region_vid(r2);
+                                let outlives_requirements = &closure_region_requirements
+                                    .outlives_requirements[idx];
+                                Some((
+                                    (r1_vid, r2_vid),
+                                    (
+                                        outlives_requirements.category,
+                                        outlives_requirements.blame_span,
+                                    ),
+                                ))
+                            }
+                            UnpackedKind::Type(_) => None,
+                        }
+                    })
+                    .collect();
+
+                let existing = borrowck_context.constraints
+                    .closure_bounds_mapping
+                    .insert(location, bounds_mapping);
+                assert!(existing.is_none(), "Multiple closures at the same location.");
+            }
+
+            self.push_region_constraints(
+                location.to_locations(),
+                ConstraintCategory::ClosureBounds,
+                &closure_constraints,
+            );
+        }
+
+        tcx.predicates_of(def_id).instantiate(tcx, substs.substs)
+    }
+
     fn prove_trait_ref(
         &mut self,
         trait_ref: ty::TraitRef<'tcx>,