account for the pick-constraint edges when reporting errors

Also, thread through better span info to improve the error message to
something tolerable.

src/librustc/infer/mod.rs

@@ -910,13 +910,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     pub fn pick_constraint(
         &self,
         opaque_type_def_id: DefId,
+        definition_span: Span,
         hidden_ty: Ty<'tcx>,
         region: ty::Region<'tcx>,
         in_regions: &Rc<Vec<ty::Region<'tcx>>>,
     ) {
         debug!("sub_regions({:?} <: {:?})", region, in_regions);
         self.borrow_region_constraints()
-            .pick_constraint(opaque_type_def_id, hidden_ty, region, in_regions);
+            .pick_constraint(opaque_type_def_id, definition_span, hidden_ty, region, in_regions);
     }
 
     pub fn subtype_predicate(

src/librustc/infer/opaque_types/mod.rs

@@ -12,6 +12,7 @@ use crate::util::nodemap::DefIdMap;
 use errors::DiagnosticBuilder;
 use rustc_data_structures::fx::FxHashMap;
 use std::rc::Rc;
+use syntax_pos::Span;
 
 pub type OpaqueTypeMap<'tcx> = DefIdMap<OpaqueTypeDecl<'tcx>>;
 
@@ -33,6 +34,20 @@ pub struct OpaqueTypeDecl<'tcx> {
     /// then `substs` would be `['a, T]`.
     pub substs: SubstsRef<'tcx>,
 
+    /// The span of this particular definition of the opaque type.  So
+    /// for example:
+    ///
+    /// ```
+    /// existential type Foo;
+    /// fn bar() -> Foo {
+    ///             ^^^ this is the span we are looking for!
+    /// ```
+    ///
+    /// In cases where the fn returns `(impl Trait, impl Trait)` or
+    /// other such combinations, the result is currently
+    /// over-approximated, but better than nothing.
+    pub definition_span: Span,
+
     /// The type variable that represents the value of the abstract type
     /// that we require. In other words, after we compile this function,
     /// we will be created a constraint like:
@@ -99,12 +114,14 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
     /// - `param_env` -- the in-scope parameter environment to be used for
     ///   obligations
     /// - `value` -- the value within which we are instantiating opaque types
+    /// - `value_span` -- the span where the value came from, used in error reporting
     pub fn instantiate_opaque_types<T: TypeFoldable<'tcx>>(
         &self,
         parent_def_id: DefId,
         body_id: hir::HirId,
         param_env: ty::ParamEnv<'tcx>,
         value: &T,
+        value_span: Span,
     ) -> InferOk<'tcx, (T, OpaqueTypeMap<'tcx>)> {
         debug!(
             "instantiate_opaque_types(value={:?}, parent_def_id={:?}, body_id={:?}, \
@@ -116,6 +133,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             parent_def_id,
             body_id,
             param_env,
+            value_span,
             opaque_types: Default::default(),
             obligations: vec![],
         };
@@ -427,6 +445,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
             tcx: self.tcx,
             op: |r| self.pick_constraint(
                 opaque_type_def_id,
+                opaque_defn.definition_span,
                 concrete_ty,
                 r,
                 &option_regions,
@@ -807,6 +826,7 @@ struct Instantiator<'a, 'tcx> {
     parent_def_id: DefId,
     body_id: hir::HirId,
     param_env: ty::ParamEnv<'tcx>,
+    value_span: Span,
     opaque_types: OpaqueTypeMap<'tcx>,
     obligations: Vec<PredicateObligation<'tcx>>,
 }
@@ -954,10 +974,18 @@ impl<'a, 'tcx> Instantiator<'a, 'tcx> {
         debug!("instantiate_opaque_types: param_env={:#?}", self.param_env,);
         debug!("instantiate_opaque_types: generics={:#?}", tcx.generics_of(def_id),);
 
+        // Ideally, we'd get the span where *this specific `ty` came
+        // from*, but right now we just use the span from the overall
+        // value being folded. In simple cases like `-> impl Foo`,
+        // these are the same span, but not in cases like `-> (impl
+        // Foo, impl Bar)`.
+        let definition_span = self.value_span;
+
         self.opaque_types.insert(
             def_id,
             OpaqueTypeDecl {
                 substs,
+                definition_span,
                 concrete_ty: ty_var,
                 has_required_region_bounds: !required_region_bounds.is_empty(),
                 origin,

src/librustc/infer/region_constraints/mod.rs

@@ -14,6 +14,7 @@ use crate::ty::ReStatic;
 use crate::ty::{self, Ty, TyCtxt};
 use crate::ty::{ReLateBound, ReVar};
 use crate::ty::{Region, RegionVid};
+use syntax_pos::Span;
 
 use std::collections::BTreeMap;
 use std::{cmp, fmt, mem};
@@ -155,6 +156,9 @@ pub struct PickConstraint<'tcx> {
     /// the def-id of the opaque type causing this constraint: used for error reporting
     pub opaque_type_def_id: DefId,
 
+    /// the span where the hidden type was instantiated
+    pub definition_span: Span,
+
     /// the hidden type in which `pick_region` appears: used for error reporting
     pub hidden_ty: Ty<'tcx>,
 
@@ -167,14 +171,14 @@ pub struct PickConstraint<'tcx> {
 
 BraceStructTypeFoldableImpl! {
     impl<'tcx> TypeFoldable<'tcx> for PickConstraint<'tcx> {
-        opaque_type_def_id, hidden_ty, pick_region, option_regions
+        opaque_type_def_id, definition_span, hidden_ty, pick_region, option_regions
     }
 }
 
 BraceStructLiftImpl! {
     impl<'a, 'tcx> Lift<'tcx> for PickConstraint<'a> {
         type Lifted = PickConstraint<'tcx>;
-        opaque_type_def_id, hidden_ty, pick_region, option_regions
+        opaque_type_def_id, definition_span, hidden_ty, pick_region, option_regions
     }
 }
 
@@ -687,6 +691,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> {
     pub fn pick_constraint(
         &mut self,
         opaque_type_def_id: DefId,
+        definition_span: Span,
         hidden_ty: Ty<'tcx>,
         pick_region: ty::Region<'tcx>,
         option_regions: &Rc<Vec<ty::Region<'tcx>>>,
@@ -699,6 +704,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> {
 
         self.data.pick_constraints.push(PickConstraint {
             opaque_type_def_id,
+            definition_span,
             hidden_ty,
             pick_region,
             option_regions: option_regions.clone()

src/librustc_mir/borrow_check/nll/pick_constraints.rs

@@ -5,6 +5,7 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_data_structures::indexed_vec::{Idx, IndexVec};
 use std::hash::Hash;
 use std::ops::Index;
+use syntax_pos::Span;
 
 /// Compactly stores a set of `pick R0 in [R1...Rn]` constraints,
 /// indexed by the region R0.
@@ -34,6 +35,9 @@ crate struct NllPickConstraint<'tcx> {
     /// The opaque type whose hidden type is being inferred. (Used in error reporting.)
     crate opaque_type_def_id: DefId,
 
+    /// The span where the hidden type was instantiated.
+    crate definition_span: Span,
+
     /// The hidden type in which R0 appears. (Used in error reporting.)
     crate hidden_ty: Ty<'tcx>,
 
@@ -79,6 +83,7 @@ impl<'tcx> PickConstraintSet<'tcx, ty::RegionVid> {
             next_constraint,
             pick_region_vid,
             opaque_type_def_id: p_c.opaque_type_def_id,
+            definition_span: p_c.definition_span,
             hidden_ty: p_c.hidden_ty,
             start_index,
             end_index,

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

@@ -1,4 +1,5 @@
 use crate::borrow_check::nll::constraints::OutlivesConstraint;
+use crate::borrow_check::nll::region_infer::AppliedPickConstraint;
 use crate::borrow_check::nll::region_infer::RegionInferenceContext;
 use crate::borrow_check::nll::type_check::Locations;
 use crate::borrow_check::nll::universal_regions::DefiningTy;
@@ -195,6 +196,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                         Trace::NotVisited => {
                             bug!("found unvisited region {:?} on path to {:?}", p, r)
                         }
+
                         Trace::FromOutlivesConstraint(c) => {
                             result.push(c);
                             p = c.sup;
@@ -211,10 +213,30 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             // Otherwise, walk over the outgoing constraints and
             // enqueue any regions we find, keeping track of how we
             // reached them.
+
+            // A constraint like `'r: 'x` can come from our constraint
+            // graph.
             let fr_static = self.universal_regions.fr_static;
-            for constraint in self.constraint_graph
+            let outgoing_edges_from_graph = self.constraint_graph
-                .outgoing_edges(r, &self.constraints, fr_static)
+                .outgoing_edges(r, &self.constraints, fr_static);
-            {
+
+
+            // But pick-constraints can also give rise to `'r: 'x`
+            // edges that were not part of the graph initially, so
+            // watch out for those.
+            let outgoing_edges_from_picks = self.applied_pick_constraints(r)
+                .iter()
+                .map(|&AppliedPickConstraint { best_option, pick_constraint_index, .. }| {
+                    let p_c = &self.pick_constraints[pick_constraint_index];
+                    OutlivesConstraint {
+                        sup: r,
+                        sub: best_option,
+                        locations: Locations::All(p_c.definition_span),
+                        category: ConstraintCategory::OpaqueType,
+                    }
+                });
+
+            for constraint in outgoing_edges_from_graph.chain(outgoing_edges_from_picks) {
                 debug_assert_eq!(constraint.sup, r);
                 let sub_region = constraint.sub;
                 if let Trace::NotVisited = context[sub_region] {
@@ -687,7 +709,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
 
     // 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 {
+    crate fn find_sub_region_live_at(
+        &self,
+        fr1: RegionVid,
+        elem: Location,
+    ) -> RegionVid {
         debug!("find_sub_region_live_at(fr1={:?}, elem={:?})", fr1, elem);
         self.find_constraint_paths_between_regions(fr1, |r| {
             // First look for some `r` such that `fr1: r` and `r` is live at `elem`
@@ -729,8 +755,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         fr1: RegionVid,
         fr2: RegionVid,
     ) -> (ConstraintCategory, Span) {
-        let (category, _, span) =
+        let (category, _, span) = self.best_blame_constraint(
-            self.best_blame_constraint(body, fr1, |r| self.provides_universal_region(r, fr1, fr2));
+            body,
+            fr1,
+            |r| self.provides_universal_region(r, fr1, fr2),
+        );
         (category, span)
     }
 

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

@@ -3,7 +3,7 @@ use crate::borrow_check::nll::constraints::graph::NormalConstraintGraph;
 use crate::borrow_check::nll::constraints::{
     ConstraintSccIndex, OutlivesConstraint, OutlivesConstraintSet,
 };
-use crate::borrow_check::nll::pick_constraints::PickConstraintSet;
+use crate::borrow_check::nll::pick_constraints::{PickConstraintSet, NllPickConstraintIndex};
 use crate::borrow_check::nll::region_infer::values::{
     PlaceholderIndices, RegionElement, ToElementIndex,
 };
@@ -21,9 +21,10 @@ use rustc::mir::{
 };
 use rustc::ty::{self, subst::SubstsRef, RegionVid, Ty, TyCtxt, TypeFoldable};
 use rustc::util::common::{self, ErrorReported};
+use rustc_data_structures::binary_search_util;
 use rustc_data_structures::bit_set::BitSet;
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use crate::rustc_data_structures::graph::WithSuccessors;
+use rustc_data_structures::graph::WithSuccessors;
 use rustc_data_structures::graph::scc::Sccs;
 use rustc_data_structures::graph::vec_graph::VecGraph;
 use rustc_data_structures::indexed_vec::IndexVec;
@@ -74,6 +75,12 @@ pub struct RegionInferenceContext<'tcx> {
     /// The "pick R0 from [R1..Rn]" constraints, indexed by SCC.
     pick_constraints: Rc<PickConstraintSet<'tcx, ConstraintSccIndex>>,
 
+    /// Records the pick-constraints that we applied to each scc.
+    /// This is useful for error reporting. Once constraint
+    /// propagation is done, this vector is sorted according to
+    /// `pick_region_scc`.
+    pick_constraints_applied: Vec<AppliedPickConstraint>,
+
     /// Map closure bounds to a `Span` that should be used for error reporting.
     closure_bounds_mapping:
         FxHashMap<Location, FxHashMap<(RegionVid, RegionVid), (ConstraintCategory, Span)>>,
@@ -109,6 +116,32 @@ pub struct RegionInferenceContext<'tcx> {
     universal_region_relations: Rc<UniversalRegionRelations<'tcx>>,
 }
 
+/// Each time that `apply_pick_constraint` is successful, it appends
+/// one of these structs to the `pick_constraints_applied` field.
+/// This is used in error reporting to trace out what happened.
+///
+/// The way that `apply_pick_constraint` works is that it effectively
+/// adds a new lower bound to the SCC it is analyzing: so you wind up
+/// with `'R: 'O` where `'R` is the pick-region and `'O` is the
+/// minimal viable option.
+#[derive(Copy, Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
+struct AppliedPickConstraint {
+    /// The SCC that was affected. (The "pick region".)
+    ///
+    /// The vector if `AppliedPickConstraint` elements is kept sorted
+    /// by this field.
+    pick_region_scc: ConstraintSccIndex,
+
+    /// The "best option" that `apply_pick_constraint` found -- this was
+    /// added as an "ad-hoc" lower-bound to `pick_region_scc`.
+    best_option: ty::RegionVid,
+
+    /// The "pick constraint index" -- we can find out details about
+    /// the constraint from
+    /// `set.pick_constraints[pick_constraint_index]`.
+    pick_constraint_index: NllPickConstraintIndex,
+}
+
 struct RegionDefinition<'tcx> {
     /// What kind of variable is this -- a free region? existential
     /// variable? etc. (See the `NLLRegionVariableOrigin` for more
@@ -243,6 +276,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             constraint_sccs,
             rev_constraint_graph: None,
             pick_constraints,
+            pick_constraints_applied: Vec::new(),
             closure_bounds_mapping,
             scc_universes,
             scc_representatives,
@@ -411,6 +445,18 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         self.scc_universes[scc]
     }
 
+    /// Once region solving has completed, this function will return
+    /// the pick-constraints that were applied to the value of a given
+    /// region `r`. See `AppliedPickConstraint`.
+    fn applied_pick_constraints(&self, r: impl ToRegionVid) -> &[AppliedPickConstraint] {
+        let scc = self.constraint_sccs.scc(r.to_region_vid());
+        binary_search_util::binary_search_slice(
+            &self.pick_constraints_applied,
+            |applied| applied.pick_region_scc,
+            &scc,
+        )
+    }
+
     /// Performs region inference and report errors if we see any
     /// unsatisfiable constraints. If this is a closure, returns the
     /// region requirements to propagate to our creator, if any.
@@ -501,6 +547,10 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         for scc_index in self.constraint_sccs.all_sccs() {
             self.propagate_constraint_sccs_if_new(scc_index, visited);
         }
+
+        // Sort the applied pick constraints so we can binary search
+        // through them later.
+        self.pick_constraints_applied.sort_by_key(|applied| applied.pick_region_scc);
     }
 
     /// Computes the value of the SCC `scc_a` if it has not already
@@ -552,7 +602,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         for p_c_i in pick_constraints.indices(scc_a) {
             self.apply_pick_constraint(
                 scc_a,
-                pick_constraints[p_c_i].opaque_type_def_id,
+                p_c_i,
                 pick_constraints.option_regions(p_c_i),
             );
         }
@@ -578,7 +628,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
     fn apply_pick_constraint(
         &mut self,
         scc: ConstraintSccIndex,
-        opaque_type_def_id: DefId,
+        pick_constraint_index: NllPickConstraintIndex,
         option_regions: &[ty::RegionVid],
     ) -> bool {
         debug!("apply_pick_constraint(scc={:?}, option_regions={:#?})", scc, option_regions,);
@@ -593,7 +643,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             bug!(
                 "pick constraint for `{:?}` has an option region `{:?}` \
                  that is not a universal region",
-                opaque_type_def_id,
+                self.pick_constraints[pick_constraint_index].opaque_type_def_id,
                 uh_oh,
             );
         }
@@ -681,7 +731,17 @@ impl<'tcx> RegionInferenceContext<'tcx> {
             best_option,
             best_option_scc,
         );
-        self.scc_values.add_region(scc, best_option_scc)
+        if self.scc_values.add_region(scc, best_option_scc) {
+            self.pick_constraints_applied.push(AppliedPickConstraint {
+                pick_region_scc: scc,
+                best_option,
+                pick_constraint_index,
+            });
+
+            true
+        } else {
+            false
+        }
     }
 
     /// Compute and return the reverse SCC-based constraint graph (lazilly).

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

@@ -836,6 +836,7 @@ struct TypeChecker<'a, 'tcx> {
     infcx: &'a InferCtxt<'a, 'tcx>,
     param_env: ty::ParamEnv<'tcx>,
     last_span: Span,
+    body: &'a Body<'tcx>,
     /// User type annotations are shared between the main MIR and the MIR of
     /// all of the promoted items.
     user_type_annotations: &'a CanonicalUserTypeAnnotations<'tcx>,
@@ -996,6 +997,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
             infcx,
             last_span: DUMMY_SP,
             mir_def_id,
+            body,
             user_type_annotations: &body.user_type_annotations,
             param_env,
             region_bound_pairs,
@@ -1233,6 +1235,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
         let infcx = self.infcx;
         let tcx = infcx.tcx;
         let param_env = self.param_env;
+        let body = self.body;
         debug!("eq_opaque_type_and_type: mir_def_id={:?}", self.mir_def_id);
         let opaque_type_map = self.fully_perform_op(
             locations,
@@ -1248,6 +1251,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
                             dummy_body_id,
                             param_env,
                             &anon_ty,
+                            locations.span(body),
                         ));
                     debug!(
                         "eq_opaque_type_and_type: \

src/librustc_typeck/check/mod.rs

@@ -856,7 +856,8 @@ fn typeck_tables_of<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> &'tcx ty::TypeckT
             let revealed_ty = if tcx.features().impl_trait_in_bindings {
                 fcx.instantiate_opaque_types_from_value(
                     id,
-                    &expected_type
+                    &expected_type,
+                    body.value.span,
                 )
             } else {
                 expected_type
@@ -962,7 +963,8 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherLocalsVisitor<'a, 'tcx> {
                 let revealed_ty = if self.fcx.tcx.features().impl_trait_in_bindings {
                     self.fcx.instantiate_opaque_types_from_value(
                         self.parent_id,
-                        &o_ty
+                        &o_ty,
+                        ty.span,
                     )
                 } else {
                     o_ty
@@ -1058,7 +1060,11 @@ fn check_fn<'a, 'tcx>(
 
     let declared_ret_ty = fn_sig.output();
     fcx.require_type_is_sized(declared_ret_ty, decl.output.span(), traits::SizedReturnType);
-    let revealed_ret_ty = fcx.instantiate_opaque_types_from_value(fn_id, &declared_ret_ty);
+    let revealed_ret_ty = fcx.instantiate_opaque_types_from_value(
+        fn_id,
+        &declared_ret_ty,
+        decl.output.span(),
+    );
     fcx.ret_coercion = Some(RefCell::new(CoerceMany::new(revealed_ret_ty)));
     fn_sig = fcx.tcx.mk_fn_sig(
         fn_sig.inputs().iter().cloned(),
@@ -2445,6 +2451,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         &self,
         parent_id: hir::HirId,
         value: &T,
+        value_span: Span,
     ) -> T {
         let parent_def_id = self.tcx.hir().local_def_id_from_hir_id(parent_id);
         debug!("instantiate_opaque_types_from_value(parent_def_id={:?}, value={:?})",
@@ -2457,6 +2464,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 self.body_id,
                 self.param_env,
                 value,
+                value_span,
             )
         );
 

src/test/ui/impl-trait/multiple-lifetimes/error-handling.rs

@@ -0,0 +1,21 @@
+// compile-flags:-Zborrowck=mir
+
+#![feature(existential_type)]
+
+#[derive(Clone)]
+struct CopyIfEq<T, U>(T, U);
+
+impl<T: Copy> Copy for CopyIfEq<T, T> {}
+
+existential type E<'a, 'b>: Sized;
+//~^ ERROR lifetime may not live long enough
+
+fn foo<'a, 'b, 'c>(x: &'static i32, mut y: &'a i32) -> E<'b, 'c> {
+    let v = CopyIfEq::<*mut _, *mut _>(&mut {x}, &mut y);
+    let u = v;
+    let _: *mut &'a i32 = u.1;
+    unsafe { let _: &'b i32 = *u.0; }
+    u.0
+}
+
+fn main() {}

src/test/ui/impl-trait/multiple-lifetimes/error-handling.stderr

@@ -0,0 +1,15 @@
+error: lifetime may not live long enough
+  --> $DIR/error-handling.rs:10:1
+   |
+LL | existential type E<'a, 'b>: Sized;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ opaque type requires that `'a` must outlive `'static`
+...
+LL | fn foo<'a, 'b, 'c>(x: &'static i32, mut y: &'a i32) -> E<'b, 'c> {
+   |        -- lifetime `'a` defined here
+help: to allow this `impl Trait` to capture borrowed data with lifetime `'a`, add `'a` as a constraint
+   |
+LL | existential type E<'a, 'b>: Sized; + 'a
+   |
+
+error: aborting due to previous error
+