Auto merge of rust-lang#114602 - compiler-errors:rpit-outlives-sadness, r=oli-obk

Map RPIT duplicated lifetimes back to fn captured lifetimes

Use the [`lifetime_mapping`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir/hir/struct.OpaqueTy.html#structfield.lifetime_mapping) to map an RPIT's captured lifetimes back to the early- or late-bound lifetimes from its parent function. We may be going thru several layers of mapping, since opaques can be nested, so we introduce `TyCtxt::map_rpit_lifetime_to_fn_lifetime` to loop through several opaques worth of mapping, and handle turning it into a `ty::Region` as well.

We can then use this instead of the identity substs for RPITs in `check_opaque_meets_bounds` to address rust-lang#114285.

We can then also use `map_rpit_lifetime_to_fn_lifetime` to properly install bidirectional-outlives predicates for both RPITs and RPITITs. This addresses rust-lang#114601.

I based this on rust-lang#114574, but I don't actually know how much of that PR we still need, so some code may be redundant now... 🤷

---

Fixes rust-lang#114597
Fixes rust-lang#114579
Fixes rust-lang#114285

Also fixes rust-lang#114601, since it turns out we had other bugs with RPITITs and their duplicated lifetime params 😅.

Supersedes rust-lang#114574

r? `@oli-obk`

Author: bors

compiler/rustc_ast_lowering/src/lib.rs

@@ -1663,11 +1663,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
             );
             debug!("lower_async_fn_ret_ty: generic_params={:#?}", generic_params);
 
-            let lifetime_mapping = if in_trait {
-                Some(&*self.arena.alloc_slice(&synthesized_lifetime_args))
-            } else {
-                None
-            };
+            let lifetime_mapping = self.arena.alloc_slice(&synthesized_lifetime_args);
 
             let opaque_ty_item = hir::OpaqueTy {
                 generics: this.arena.alloc(hir::Generics {

compiler/rustc_hir/src/hir.rs

@@ -2675,7 +2675,7 @@ pub struct OpaqueTy<'hir> {
     ///
     /// This mapping associated a captured lifetime (first parameter) with the new
     /// early-bound lifetime that was generated for the opaque.
-    pub lifetime_mapping: Option<&'hir [(&'hir Lifetime, LocalDefId)]>,
+    pub lifetime_mapping: &'hir [(&'hir Lifetime, LocalDefId)],
     /// Whether the opaque is a return-position impl trait (or async future)
     /// originating from a trait method. This makes it so that the opaque is
     /// lowered as an associated type.

compiler/rustc_hir_analysis/src/check/check.rs

@@ -407,7 +407,17 @@ fn check_opaque_meets_bounds<'tcx>(
         .build();
     let ocx = ObligationCtxt::new(&infcx);
 
-    let args = GenericArgs::identity_for_item(tcx, def_id.to_def_id());
+    let args = match *origin {
+        hir::OpaqueTyOrigin::FnReturn(parent) | hir::OpaqueTyOrigin::AsyncFn(parent) => {
+            GenericArgs::identity_for_item(tcx, parent).extend_to(
+                tcx,
+                def_id.to_def_id(),
+                |param, _| tcx.map_rpit_lifetime_to_fn_lifetime(param.def_id.expect_local()).into(),
+            )
+        }
+        hir::OpaqueTyOrigin::TyAlias { .. } => GenericArgs::identity_for_item(tcx, def_id),
+    };
+
     let opaque_ty = Ty::new_opaque(tcx, def_id.to_def_id(), args);
 
     // `ReErased` regions appear in the "parent_args" of closures/generators.
@@ -468,9 +478,10 @@ fn check_opaque_meets_bounds<'tcx>(
         }
     }
     // Check that any hidden types found during wf checking match the hidden types that `type_of` sees.
-    for (key, mut ty) in infcx.take_opaque_types() {
+    for (mut key, mut ty) in infcx.take_opaque_types() {
         ty.hidden_type.ty = infcx.resolve_vars_if_possible(ty.hidden_type.ty);
-        sanity_check_found_hidden_type(tcx, key, ty.hidden_type, defining_use_anchor, origin)?;
+        key = infcx.resolve_vars_if_possible(key);
+        sanity_check_found_hidden_type(tcx, key, ty.hidden_type)?;
     }
     Ok(())
 }
@@ -479,8 +490,6 @@ fn sanity_check_found_hidden_type<'tcx>(
     tcx: TyCtxt<'tcx>,
     key: ty::OpaqueTypeKey<'tcx>,
     mut ty: ty::OpaqueHiddenType<'tcx>,
-    defining_use_anchor: LocalDefId,
-    origin: &hir::OpaqueTyOrigin,
 ) -> Result<(), ErrorGuaranteed> {
     if ty.ty.is_ty_var() {
         // Nothing was actually constrained.
@@ -493,29 +502,23 @@ fn sanity_check_found_hidden_type<'tcx>(
             return Ok(());
         }
     }
+    let strip_vars = |ty: Ty<'tcx>| {
+        ty.fold_with(&mut BottomUpFolder {
+            tcx,
+            ty_op: |t| t,
+            ct_op: |c| c,
+            lt_op: |l| match l.kind() {
+                RegionKind::ReVar(_) => tcx.lifetimes.re_erased,
+                _ => l,
+            },
+        })
+    };
     // Closures frequently end up containing erased lifetimes in their final representation.
     // These correspond to lifetime variables that never got resolved, so we patch this up here.
-    ty.ty = ty.ty.fold_with(&mut BottomUpFolder {
-        tcx,
-        ty_op: |t| t,
-        ct_op: |c| c,
-        lt_op: |l| match l.kind() {
-            RegionKind::ReVar(_) => tcx.lifetimes.re_erased,
-            _ => l,
-        },
-    });
+    ty.ty = strip_vars(ty.ty);
     // Get the hidden type.
-    let mut hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args);
-    if let hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) = origin {
-        if hidden_ty != ty.ty {
-            hidden_ty = find_and_apply_rpit_args(
-                tcx,
-                hidden_ty,
-                defining_use_anchor.to_def_id(),
-                key.def_id.to_def_id(),
-            )?;
-        }
-    }
+    let hidden_ty = tcx.type_of(key.def_id).instantiate(tcx, key.args);
+    let hidden_ty = strip_vars(hidden_ty);
 
     // If the hidden types differ, emit a type mismatch diagnostic.
     if hidden_ty == ty.ty {
@@ -527,105 +530,6 @@ fn sanity_check_found_hidden_type<'tcx>(
     }
 }
 
-/// In case it is in a nested opaque type, find that opaque type's
-/// usage in the function signature and use the generic arguments from the usage site.
-/// We need to do because RPITs ignore the lifetimes of the function,
-/// as they have their own copies of all the lifetimes they capture.
-/// So the only way to get the lifetimes represented in terms of the function,
-/// is to look how they are used in the function signature (or do some other fancy
-/// recording of this mapping at ast -> hir lowering time).
-///
-/// As an example:
-/// ```text
-/// trait Id {
-///     type Assoc;
-/// }
-/// impl<'a> Id for &'a () {
-///     type Assoc = &'a ();
-/// }
-/// fn func<'a>(x: &'a ()) -> impl Id<Assoc = impl Sized + 'a> { x }
-/// // desugared to
-/// fn func<'a>(x: &'a () -> Outer<'a> where <Outer<'a> as Id>::Assoc = Inner<'a> {
-///     // Note that in contrast to other nested items, RPIT type aliases can
-///     // access their parents' generics.
-///
-///     // hidden type is `&'aDupOuter ()`
-///     // During wfcheck the hidden type of `Inner<'aDupOuter>` is `&'a ()`, but
-///     // `typeof(Inner<'aDupOuter>) = &'aDupOuter ()`.
-///     // So we walk the signature of `func` to find the use of `Inner<'a>`
-///     // and then use that to replace the lifetimes in the hidden type, obtaining
-///     // `&'a ()`.
-///     type Outer<'aDupOuter> = impl Id<Assoc = Inner<'aDupOuter>>;
-///
-///     // hidden type is `&'aDupInner ()`
-///     type Inner<'aDupInner> = impl Sized + 'aDupInner;
-///
-///     x
-/// }
-/// ```
-fn find_and_apply_rpit_args<'tcx>(
-    tcx: TyCtxt<'tcx>,
-    mut hidden_ty: Ty<'tcx>,
-    function: DefId,
-    opaque: DefId,
-) -> Result<Ty<'tcx>, ErrorGuaranteed> {
-    // Find use of the RPIT in the function signature and thus find the right args to
-    // convert it into the parameter space of the function signature. This is needed,
-    // because that's what `type_of` returns, against which we compare later.
-    let ret = tcx.fn_sig(function).instantiate_identity().output();
-    struct Visitor<'tcx> {
-        tcx: TyCtxt<'tcx>,
-        opaque: DefId,
-        seen: FxHashSet<DefId>,
-    }
-    impl<'tcx> ty::TypeVisitor<TyCtxt<'tcx>> for Visitor<'tcx> {
-        type BreakTy = GenericArgsRef<'tcx>;
-
-        #[instrument(level = "trace", skip(self), ret)]
-        fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
-            trace!("{:#?}", t.kind());
-            match t.kind() {
-                ty::Alias(ty::Opaque, alias) => {
-                    trace!(?alias.def_id);
-                    if alias.def_id == self.opaque {
-                        return ControlFlow::Break(alias.args);
-                    } else if self.seen.insert(alias.def_id) {
-                        for clause in self
-                            .tcx
-                            .explicit_item_bounds(alias.def_id)
-                            .iter_instantiated_copied(self.tcx, alias.args)
-                        {
-                            trace!(?clause);
-                            clause.visit_with(self)?;
-                        }
-                    }
-                }
-                ty::Alias(ty::Weak, alias) => {
-                    self.tcx
-                        .type_of(alias.def_id)
-                        .instantiate(self.tcx, alias.args)
-                        .visit_with(self)?;
-                }
-                _ => (),
-            }
-
-            t.super_visit_with(self)
-        }
-    }
-    if let ControlFlow::Break(args) =
-        ret.visit_with(&mut Visitor { tcx, opaque, seen: Default::default() })
-    {
-        trace!(?args);
-        trace!("expected: {hidden_ty:#?}");
-        hidden_ty = ty::EarlyBinder::bind(hidden_ty).instantiate(tcx, args);
-        trace!("expected: {hidden_ty:#?}");
-    } else {
-        tcx.sess
-            .delay_span_bug(tcx.def_span(function), format!("{ret:?} does not contain {opaque:?}"));
-    }
-    Ok(hidden_ty)
-}
-
 fn is_enum_of_nonnullable_ptr<'tcx>(
     tcx: TyCtxt<'tcx>,
     adt_def: AdtDef<'tcx>,

compiler/rustc_hir_analysis/src/collect/predicates_of.rs

@@ -2,16 +2,16 @@ use crate::astconv::{AstConv, OnlySelfBounds, PredicateFilter};
 use crate::bounds::Bounds;
 use crate::collect::ItemCtxt;
 use crate::constrained_generic_params as cgp;
-use hir::{HirId, Lifetime, Node};
+use hir::{HirId, Node};
 use rustc_data_structures::fx::FxIndexSet;
 use rustc_hir as hir;
 use rustc_hir::def::DefKind;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::intravisit::{self, Visitor};
 use rustc_middle::ty::{self, Ty, TyCtxt};
-use rustc_middle::ty::{GenericPredicates, Generics, ImplTraitInTraitData, ToPredicate};
+use rustc_middle::ty::{GenericPredicates, ImplTraitInTraitData, ToPredicate};
 use rustc_span::symbol::Ident;
-use rustc_span::{Span, Symbol, DUMMY_SP};
+use rustc_span::{Span, DUMMY_SP};
 
 /// Returns a list of all type predicates (explicit and implicit) for the definition with
 /// ID `def_id`. This includes all predicates returned by `predicates_defined_on`, plus
@@ -55,17 +55,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
     use rustc_hir::*;
 
     match tcx.opt_rpitit_info(def_id.to_def_id()) {
-        Some(ImplTraitInTraitData::Trait { opaque_def_id, fn_def_id }) => {
-            let opaque_ty_id = tcx.hir().local_def_id_to_hir_id(opaque_def_id.expect_local());
-            let opaque_ty_node = tcx.hir().get(opaque_ty_id);
-            let Node::Item(&Item {
-                kind: ItemKind::OpaqueTy(OpaqueTy { lifetime_mapping: Some(lifetime_mapping), .. }),
-                ..
-            }) = opaque_ty_node
-            else {
-                bug!("unexpected {opaque_ty_node:?}")
-            };
-
+        Some(ImplTraitInTraitData::Trait { fn_def_id, .. }) => {
             let mut predicates = Vec::new();
 
             // RPITITs should inherit the predicates of their parent. This is
@@ -78,13 +68,12 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
 
             // We also install bidirectional outlives predicates for the RPITIT
             // to keep the duplicates lifetimes from opaque lowering in sync.
+            // We only need to compute bidirectional outlives for the duplicated
+            // opaque lifetimes, which explains the slicing below.
             compute_bidirectional_outlives_predicates(
                 tcx,
-                def_id,
-                lifetime_mapping.iter().map(|(lifetime, def_id)| {
-                    (**lifetime, (*def_id, lifetime.ident.name, lifetime.ident.span))
-                }),
-                tcx.generics_of(def_id.to_def_id()),
+                &tcx.generics_of(def_id.to_def_id()).params
+                    [tcx.generics_of(fn_def_id).params.len()..],
                 &mut predicates,
             );
 
@@ -351,21 +340,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
         };
         debug!(?lifetimes);
 
-        let lifetime_mapping = std::iter::zip(lifetimes, ast_generics.params)
-            .map(|(arg, dup)| {
-                let hir::GenericArg::Lifetime(arg) = arg else { bug!() };
-                (**arg, dup)
-            })
-            .filter(|(_, dup)| matches!(dup.kind, hir::GenericParamKind::Lifetime { .. }))
-            .map(|(lifetime, dup)| (lifetime, (dup.def_id, dup.name.ident().name, dup.span)));
-
-        compute_bidirectional_outlives_predicates(
-            tcx,
-            def_id,
-            lifetime_mapping,
-            generics,
-            &mut predicates,
-        );
+        compute_bidirectional_outlives_predicates(tcx, &generics.params, &mut predicates);
         debug!(?predicates);
     }
 
@@ -379,41 +354,28 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::Gen
 /// enforce that these lifetimes stay in sync.
 fn compute_bidirectional_outlives_predicates<'tcx>(
     tcx: TyCtxt<'tcx>,
-    item_def_id: LocalDefId,
-    lifetime_mapping: impl Iterator<Item = (Lifetime, (LocalDefId, Symbol, Span))>,
-    generics: &Generics,
+    opaque_own_params: &[ty::GenericParamDef],
     predicates: &mut Vec<(ty::Clause<'tcx>, Span)>,
 ) {
-    let icx = ItemCtxt::new(tcx, item_def_id);
-
-    for (arg, (dup_def, name, span)) in lifetime_mapping {
-        let orig_region = icx.astconv().ast_region_to_region(&arg, None);
-        if !matches!(orig_region.kind(), ty::ReEarlyBound(..)) {
-            // There is no late-bound lifetime to actually match up here, since the lifetime doesn't
-            // show up in the opaque's parent's args.
-            continue;
+    for param in opaque_own_params {
+        let orig_lifetime = tcx.map_rpit_lifetime_to_fn_lifetime(param.def_id.expect_local());
+        if let ty::ReEarlyBound(..) = *orig_lifetime {
+            let dup_lifetime = ty::Region::new_early_bound(
+                tcx,
+                ty::EarlyBoundRegion { def_id: param.def_id, index: param.index, name: param.name },
+            );
+            let span = tcx.def_span(param.def_id);
+            predicates.push((
+                ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(orig_lifetime, dup_lifetime))
+                    .to_predicate(tcx),
+                span,
+            ));
+            predicates.push((
+                ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(dup_lifetime, orig_lifetime))
+                    .to_predicate(tcx),
+                span,
+            ));
         }
-
-        let Some(dup_index) = generics.param_def_id_to_index(icx.tcx, dup_def.to_def_id()) else {
-            bug!()
-        };
-
-        let dup_region = ty::Region::new_early_bound(
-            tcx,
-            ty::EarlyBoundRegion { def_id: dup_def.to_def_id(), index: dup_index, name },
-        );
-
-        predicates.push((
-            ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(orig_region, dup_region))
-                .to_predicate(tcx),
-            span,
-        ));
-
-        predicates.push((
-            ty::ClauseKind::RegionOutlives(ty::OutlivesPredicate(dup_region, orig_region))
-                .to_predicate(tcx),
-            span,
-        ));
     }
 }