Allow opaque types to hide lifetimes in concrete type so long as they outlive concrete type.

Author: Alexander Regueiro

src/librustc/infer/opaque_types/mod.rs

@@ -3,7 +3,7 @@ use crate::hir;
 use crate::hir::Node;
 use crate::infer::{self, InferCtxt, InferOk, TypeVariableOrigin};
 use crate::infer::outlives::free_region_map::FreeRegionRelations;
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use crate::traits::{self, PredicateObligation};
 use crate::ty::{self, Ty, TyCtxt, GenericParamDefKind};
 use crate::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder};
@@ -276,84 +276,87 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         opaque_defn: &OpaqueTypeDecl<'tcx>,
         free_region_relations: &FRR,
     ) {
-        debug!("constrain_opaque_type()");
-        debug!("constrain_opaque_type: def_id={:?}", def_id);
-        debug!("constrain_opaque_type: opaque_defn={:#?}", opaque_defn);
+        debug!("constrain_opaque_type(def_id={:?}, opaque_defn={:#?})", def_id, opaque_defn);
 
         let concrete_ty = self.resolve_type_vars_if_possible(&opaque_defn.concrete_ty);
 
         debug!("constrain_opaque_type: concrete_ty={:?}", concrete_ty);
 
-        let abstract_type_generics = self.tcx.generics_of(def_id);
+        let opaque_type_generics = self.tcx.generics_of(def_id);
 
         let span = self.tcx.def_span(def_id);
 
         // If there are required region bounds, we can just skip
-        // ahead.  There will already be a registered region
-        // obligation related `concrete_ty` to those regions.
-        if opaque_defn.has_required_region_bounds {
-            return;
-        }
-
-        // There were no `required_region_bounds`,
-        // so we have to search for a `least_region`.
-        // Go through all the regions used as arguments to the
-        // abstract type. These are the parameters to the abstract
-        // type; so in our example above, `substs` would contain
-        // `['a]` for the first impl trait and `'b` for the
-        // second.
-        let mut least_region = None;
-        for param in &abstract_type_generics.params {
-            match param.kind {
-                GenericParamDefKind::Lifetime => {}
-                _ => continue
-            }
-            // Get the value supplied for this region from the substs.
-            let subst_arg = opaque_defn.substs.region_at(param.index as usize);
-
-            // Compute the least upper bound of it with the other regions.
-            debug!("constrain_opaque_types: least_region={:?}", least_region);
-            debug!("constrain_opaque_types: subst_arg={:?}", subst_arg);
-            match least_region {
-                None => least_region = Some(subst_arg),
-                Some(lr) => {
-                    if free_region_relations.sub_free_regions(lr, subst_arg) {
-                        // keep the current least region
-                    } else if free_region_relations.sub_free_regions(subst_arg, lr) {
-                        // switch to `subst_arg`
-                        least_region = Some(subst_arg);
-                    } else {
-                        // There are two regions (`lr` and
-                        // `subst_arg`) which are not relatable. We can't
-                        // find a best choice.
-                        self.tcx
-                            .sess
-                            .struct_span_err(span, "ambiguous lifetime bound in `impl Trait`")
-                            .span_label(
-                                span,
-                                format!("neither `{}` nor `{}` outlives the other", lr, subst_arg),
-                            )
-                            .emit();
-
-                        least_region = Some(self.tcx.mk_region(ty::ReEmpty));
-                        break;
+        // ahead. There will already be a registered region
+        // obligation relating `concrete_ty` to those regions.
+        let least_region = if opaque_defn.has_required_region_bounds {
+            None
+        } else {
+            // There were no `required_region_bounds`,
+            // so we have to search for a `least_region`.
+            // Go through all the regions used as arguments to the
+            // existential type. These are the parameters to the existential
+            // type; so in our example above, `substs` would contain
+            // `['a]` for the first impl trait and `'b` for the
+            // second.
+            let mut least_region = None;
+            for param in &opaque_type_generics.params {
+                match param.kind {
+                    GenericParamDefKind::Lifetime => {}
+                    _ => continue
+                }
+                // Get the value supplied for this region from the substs.
+                let subst_arg = opaque_defn.substs.region_at(param.index as usize);
+
+                // Compute the least upper bound of it with the other regions.
+                debug!("constrain_opaque_type: least_region={:?}", least_region);
+                debug!("constrain_opaque_type: subst_arg={:?}", subst_arg);
+                match least_region {
+                    None => least_region = Some(subst_arg),
+                    Some(lr) => {
+                        if free_region_relations.sub_free_regions(lr, subst_arg) {
+                            // Keep the current least region.
+                        } else if free_region_relations.sub_free_regions(subst_arg, lr) {
+                            // Switch to `subst_arg`.
+                            least_region = Some(subst_arg);
+                        } else {
+                            // There are two regions (`lr` and `subst_arg`) that are not relatable.
+                            // We can't find a best choice.
+                            self.tcx
+                                .sess
+                                .struct_span_err(span, "ambiguous lifetime bound in `impl Trait`")
+                                .span_label(
+                                    span,
+                                    format!("neither `{}` nor `{}` outlives the other",
+                                            lr, subst_arg),
+                                )
+                                .emit();
+
+                            least_region = Some(self.tcx.mk_region(ty::ReEmpty));
+                            break;
+                        }
                     }
                 }
             }
-        }
-
-        let least_region = least_region.unwrap_or(self.tcx.types.re_static);
-        debug!("constrain_opaque_types: least_region={:?}", least_region);
+            least_region
+        }.unwrap_or(self.tcx.types.re_static);
+        debug!("constrain_opaque_type: least_region={:?}", least_region);
 
         // Require that the type `concrete_ty` outlives
         // `least_region`, modulo any type parameters that appear
-        // in the type, which we ignore. This is because impl
-        // trait values are assumed to capture all the in-scope
+        // in the type, which we ignore. This is because `impl
+        // Trait` values are assumed to capture all the in-scope
         // type parameters. This little loop here just invokes
         // `outlives` repeatedly, draining all the nested
         // obligations that result.
+        let mut concrete_ty_regions = FxHashSet::default();
         let mut types = vec![concrete_ty];
-        let bound_region = |r| self.sub_regions(infer::CallReturn(span), least_region, r);
+        let mut bound_region = |r| {
+            concrete_ty_regions.insert(r);
+            if !opaque_defn.has_required_region_bounds {
+                self.sub_regions(infer::CallReturn(span), least_region, r)
+            }
+        };
         while let Some(ty) = types.pop() {
             let mut components = smallvec![];
             self.tcx.push_outlives_components(ty, &mut components);
@@ -485,49 +488,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 
 struct ReverseMapper<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
     tcx: TyCtxt<'cx, 'gcx, 'tcx>,
-
-    /// If errors have already been reported in this fn, we suppress
-    /// our own errors because they are sometimes derivative.
-    tainted_by_errors: bool,
-
-    opaque_type_def_id: DefId,
     map: FxHashMap<Kind<'tcx>, Kind<'gcx>>,
-    map_missing_regions_to_empty: bool,
-
-    /// initially `Some`, set to `None` once error has been reported
-    hidden_ty: Option<Ty<'tcx>>,
-}
-
-impl<'cx, 'gcx, 'tcx> ReverseMapper<'cx, 'gcx, 'tcx> {
-    fn new(
-        tcx: TyCtxt<'cx, 'gcx, 'tcx>,
-        tainted_by_errors: bool,
-        opaque_type_def_id: DefId,
-        map: FxHashMap<Kind<'tcx>, Kind<'gcx>>,
-        hidden_ty: Ty<'tcx>,
-    ) -> Self {
-        Self {
-            tcx,
-            tainted_by_errors,
-            opaque_type_def_id,
-            map,
-            map_missing_regions_to_empty: false,
-            hidden_ty: Some(hidden_ty),
-        }
-    }
-
-    fn fold_kind_mapping_missing_regions_to_empty(&mut self, kind: Kind<'tcx>) -> Kind<'tcx> {
-        assert!(!self.map_missing_regions_to_empty);
-        self.map_missing_regions_to_empty = true;
-        let kind = kind.fold_with(self);
-        self.map_missing_regions_to_empty = false;
-        kind
-    }
-
-    fn fold_kind_normally(&mut self, kind: Kind<'tcx>) -> Kind<'tcx> {
-        assert!(!self.map_missing_regions_to_empty);
-        kind.fold_with(self)
-    }
 }
 
 impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for ReverseMapper<'cx, 'gcx, 'tcx> {
@@ -537,109 +498,53 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for ReverseMapper<'cx, 'gcx, 'tcx>
 
     fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
         match r {
-            // ignore bound regions that appear in the type (e.g., this
+            // Ignore bound regions that appear in the type (e.g., this
             // would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
             ty::ReLateBound(..) |
 
-            // ignore `'static`, as that can appear anywhere
+            // Ignore `'static`, as that can appear anywhere.
             ty::ReStatic |
 
-            // ignore `ReScope`, as that can appear anywhere
-            // See `src/test/run-pass/issue-49556.rs` for example.
+            // Ignore `ReScope`, as that can appear anywhere.
+            // See `src/test/run-pass/issue-49556.rs`, for example.
             ty::ReScope(..) => return r,
 
-            _ => { }
+            _ => {}
         }
 
         match self.map.get(&r.into()).map(|k| k.unpack()) {
             Some(UnpackedKind::Lifetime(r1)) => r1,
             Some(u) => panic!("region mapped to unexpected kind: {:?}", u),
             None => {
-                if !self.map_missing_regions_to_empty && !self.tainted_by_errors {
-                    if let Some(hidden_ty) = self.hidden_ty.take() {
-                        let span = self.tcx.def_span(self.opaque_type_def_id);
-                        let mut err = struct_span_err!(
-                            self.tcx.sess,
-                            span,
-                            E0700,
-                            "hidden type for `impl Trait` captures lifetime that \
-                             does not appear in bounds",
-                        );
-
-                        // Assuming regionck succeeded, then we must
-                        // be capturing *some* region from the fn
-                        // header, and hence it must be free, so it's
-                        // ok to invoke this fn (which doesn't accept
-                        // all regions, and would ICE if an
-                        // inappropriate region is given). We check
-                        // `is_tainted_by_errors` by errors above, so
-                        // we don't get in here unless regionck
-                        // succeeded. (Note also that if regionck
-                        // failed, then the regions we are attempting
-                        // to map here may well be giving errors
-                        // *because* the constraints were not
-                        // satisfiable.)
-                        self.tcx.note_and_explain_free_region(
-                            &mut err,
-                            &format!("hidden type `{}` captures ", hidden_ty),
-                            r,
-                            ""
-                        );
-
-                        err.emit();
-                    }
-                }
+                // No mapping was found. This means that it is either a
+                // disallowed lifetime, which will be caught by regionck,
+                // a region in a non-upvar closure generic, which is
+                // explicitly allowed (see below for more on this),
+                // or a lifetime that does not explicitly appear in
+                // `impl Trait` bounds but which outlives the lifetimes
+                // or types which *do* appear in the `impl Trait` bounds.
+                //
+                // These lifetimes are explicitly allowed to prevent
+                // the user from having to add dummy references to the
+                // unnamed lifetimes in their `impl Trait` bounds
+                // (e.g., `+ DummyTraitWithALifetimeArg<'a>`). Adding
+                // the lifetimes via `+` doesn't work because the type
+                // doesn't outlive those lifetimes, it just contains them.
+                //
+                // On closures: there is a somewhat subtle (read: hacky)
+                // consideration. The problem is that our closure types
+                // currently include all the lifetime parameters declared
+                // on the enclosing function, even if they are unused by
+                // the closure itself. We can't readily filter them out,
+                // so we replace them with `empty`. This can't really make
+                // a diference to the rest of hte compiler; those regions
+                // are ignored for the outlives relation, and hence don't
+                // affect trait selection or auto traits, and they are
+                // erased during trans.
                 self.tcx.types.re_empty
             },
         }
     }
-
-    fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
-        match ty.sty {
-            ty::Closure(def_id, substs) => {
-                // I am a horrible monster and I pray for death. When
-                // we encounter a closure here, it is always a closure
-                // from within the function that we are currently
-                // type-checking -- one that is now being encapsulated
-                // in an existential abstract type. Ideally, we would
-                // go through the types/lifetimes that it references
-                // and treat them just like we would any other type,
-                // which means we would error out if we find any
-                // reference to a type/region that is not in the
-                // "reverse map".
-                //
-                // **However,** in the case of closures, there is a
-                // somewhat subtle (read: hacky) consideration. The
-                // problem is that our closure types currently include
-                // all the lifetime parameters declared on the
-                // enclosing function, even if they are unused by the
-                // closure itself. We can't readily filter them out,
-                // so here we replace those values with `'empty`. This
-                // can't really make a difference to the rest of the
-                // compiler; those regions are ignored for the
-                // outlives relation, and hence don't affect trait
-                // selection or auto traits, and they are erased
-                // during codegen.
-
-                let generics = self.tcx.generics_of(def_id);
-                let substs = self.tcx.mk_substs(substs.substs.iter().enumerate().map(
-                    |(index, &kind)| {
-                        if index < generics.parent_count {
-                            // Accommodate missing regions in the parent kinds...
-                            self.fold_kind_mapping_missing_regions_to_empty(kind)
-                        } else {
-                            // ...but not elsewhere.
-                            self.fold_kind_normally(kind)
-                        }
-                    },
-                ));
-
-                self.tcx.mk_closure(def_id, ty::ClosureSubsts { substs })
-            }
-
-            _ => ty.super_fold_with(self),
-        }
-    }
 }
 
 struct Instantiator<'a, 'gcx: 'tcx, 'tcx: 'a> {

src/librustc/infer/outlives/free_region_map.rs

@@ -1,4 +1,5 @@
 use crate::ty::{self, Lift, TyCtxt, Region};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::transitive_relation::TransitiveRelation;
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug, Default)]
@@ -51,12 +52,18 @@ impl<'tcx> FreeRegionMap<'tcx> {
 /// slightly different way; this trait allows functions to be abstract
 /// over which version is in use.
 pub trait FreeRegionRelations<'tcx> {
-    /// Tests whether `r_a <= r_b`. Both must be free regions or
-    /// `'static`.
+    /// Gets all the free regions in the set of relations.
+    fn all_regions(&self) -> FxHashSet<ty::Region<'tcx>>;
+
+    /// Tests whether `r_a <= r_b`. Both must be free regions or `'static`.
     fn sub_free_regions(&self, shorter: ty::Region<'tcx>, longer: ty::Region<'tcx>) -> bool;
 }
 
 impl<'tcx> FreeRegionRelations<'tcx> for FreeRegionMap<'tcx> {
+    fn all_regions(&self) -> FxHashSet<ty::Region<'tcx>> {
+        self.relation.elements().map(|r| *r).collect()
+    }
+
     fn sub_free_regions(&self,
                         r_a: Region<'tcx>,
                         r_b: Region<'tcx>)

src/librustc_data_structures/transitive_relation.rs

@@ -53,6 +53,10 @@ struct Edge {
 }
 
 impl<T: Clone + Debug + Eq + Hash> TransitiveRelation<T> {
+    pub fn elements(&self) -> impl Iterator<Item = &T> {
+        self.elements.iter()
+    }
+
     pub fn is_empty(&self) -> bool {
         self.edges.is_empty()
     }