Give precendence to regions from member constaints when inferring concrete types.

Author: cjgillot

compiler/rustc_borrowck/src/member_constraints.rs

@@ -11,6 +11,7 @@ use std::ops::Index;
 
 /// Compactly stores a set of `R0 member of [R1...Rn]` constraints,
 /// indexed by the region `R0`.
+#[derive(Debug)]
 pub(crate) struct MemberConstraintSet<'tcx, R>
 where
     R: Copy + Eq,
@@ -31,6 +32,7 @@ where
 }
 
 /// Represents a `R0 member of [R1..Rn]` constraint
+#[derive(Debug)]
 pub(crate) struct NllMemberConstraint<'tcx> {
     next_constraint: Option<NllMemberConstraintIndex>,
 

compiler/rustc_borrowck/src/region_infer/mod.rs

@@ -128,6 +128,7 @@ pub struct RegionInferenceContext<'tcx> {
 /// 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(Debug)]
 pub(crate) struct AppliedMemberConstraint {
     /// The SCC that was affected. (The "member region".)
     ///

compiler/rustc_borrowck/src/region_infer/opaque_types.rs

@@ -1,4 +1,4 @@
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxIndexSet};
 use rustc_data_structures::vec_map::VecMap;
 use rustc_hir::def_id::LocalDefId;
 use rustc_hir::OpaqueTyOrigin;
@@ -63,17 +63,21 @@ impl<'tcx> RegionInferenceContext<'tcx> {
         opaque_ty_decls: VecMap<OpaqueTypeKey<'tcx>, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>,
     ) -> VecMap<LocalDefId, OpaqueHiddenType<'tcx>> {
         let mut result: VecMap<LocalDefId, OpaqueHiddenType<'tcx>> = VecMap::new();
+
+        let member_constraints: FxHashMap<_, _> = self
+            .member_constraints
+            .all_indices()
+            .map(|ci| (self.member_constraints[ci].key, ci))
+            .collect();
+        debug!(?member_constraints);
+
         for (opaque_type_key, (concrete_type, origin)) in opaque_ty_decls {
             let substs = opaque_type_key.substs;
             debug!(?concrete_type, ?substs);
 
             let mut subst_regions = vec![self.universal_regions.fr_static];
-            let universal_substs = infcx.tcx.fold_regions(substs, |region, _| {
-                if let ty::RePlaceholder(..) = region.kind() {
-                    // Higher kinded regions don't need remapping, they don't refer to anything outside of this the substs.
-                    return region;
-                }
-                let vid = self.to_region_vid(region);
+
+            let to_universal_region = |vid, subst_regions: &mut Vec<_>| {
                 trace!(?vid);
                 let scc = self.constraint_sccs.scc(vid);
                 trace!(?scc);
@@ -94,10 +98,33 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                         infcx.tcx.lifetimes.re_static
                     }
                 }
+            };
+
+            // Start by inserting universal regions from the member_constraint choice regions.
+            // This will ensure they get precedence when folding the regions in the concrete type.
+            if let Some(&ci) = member_constraints.get(&opaque_type_key) {
+                for &vid in self.member_constraints.choice_regions(ci) {
+                    to_universal_region(vid, &mut subst_regions);
+                }
+            }
+            debug!(?subst_regions);
+
+            // Next, insert universal regions from substs, so we can translate regions that appear
+            // in them but are not subject to member constraints, for instance closure substs.
+            let universal_substs = infcx.tcx.fold_regions(substs, |region, _| {
+                if let ty::RePlaceholder(..) = region.kind() {
+                    // Higher kinded regions don't need remapping, they don't refer to anything outside of this the substs.
+                    return region;
+                }
+                let vid = self.to_region_vid(region);
+                to_universal_region(vid, &mut subst_regions)
             });
+            debug!(?universal_substs);
+            debug!(?subst_regions);
 
-            subst_regions.sort();
-            subst_regions.dedup();
+            // Deduplicate the set of regions while keeping the chosen order.
+            let subst_regions = subst_regions.into_iter().collect::<FxIndexSet<_>>();
+            debug!(?subst_regions);
 
             let universal_concrete_type =
                 infcx.tcx.fold_regions(concrete_type, |region, _| match *region {
@@ -108,8 +135,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
                         .unwrap_or(infcx.tcx.lifetimes.re_erased),
                     _ => region,
                 });
-
-            debug!(?universal_concrete_type, ?universal_substs);
+            debug!(?universal_concrete_type);
 
             let opaque_type_key =
                 OpaqueTypeKey { def_id: opaque_type_key.def_id, substs: universal_substs };

compiler/rustc_middle/src/ty/mod.rs

@@ -1257,7 +1257,7 @@ impl<'tcx> InstantiatedPredicates<'tcx> {
     }
 }
 
-#[derive(Copy, Clone, Debug, PartialEq, Eq, HashStable, TyEncodable, TyDecodable, Lift)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable, TyEncodable, TyDecodable, Lift)]
 #[derive(TypeFoldable, TypeVisitable)]
 pub struct OpaqueTypeKey<'tcx> {
     pub def_id: LocalDefId,
@@ -1333,17 +1333,16 @@ impl<'tcx> OpaqueHiddenType<'tcx> {
         debug!(?id_substs);
 
         // This zip may have several times the same lifetime in `substs` paired with a different
-        // lifetime from `id_substs`.  In that case, we actually want to pick the last one, as it
-        // is the one we introduced in the impl-trait desugaring to be meaningful.  The other ones
-        // are redundant.
+        // lifetime from `id_substs`.  Simply `collect`ing the iterator is the correct behaviour:
+        // it will pick the last one, which is the one we introduced in the impl-trait desugaring.
         let map = substs.iter().zip(id_substs);
 
         let map: FxHashMap<GenericArg<'tcx>, GenericArg<'tcx>> = match origin {
             // HACK: The HIR lowering for async fn does not generate
             // any `+ Captures<'x>` bounds for the `impl Future<...>`, so all async fns with lifetimes
             // would now fail to compile. We should probably just make hir lowering fill this in properly.
-            OpaqueTyOrigin::FnReturn(_) | OpaqueTyOrigin::AsyncFn(_) => map.collect(),
-            OpaqueTyOrigin::TyAlias => {
+            OpaqueTyOrigin::AsyncFn(_) => map.collect(),
+            OpaqueTyOrigin::FnReturn(_) | OpaqueTyOrigin::TyAlias => {
                 // Opaque types may only use regions that are bound. So for
                 // ```rust
                 // type Foo<'a, 'b, 'c> = impl Trait<'a> + 'b;