Skip to content

Commit

Permalink
Auto merge of rust-lang#114602 - compiler-errors:rpit-outlives-sadnes…
Browse files Browse the repository at this point in the history
…s, 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`
  • Loading branch information
bors committed Aug 8, 2023
2 parents 617821a + 0adf704 commit bf62436
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 218 deletions.
6 changes: 1 addition & 5 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
152 changes: 28 additions & 124 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(())
}
Expand All @@ -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.
Expand All @@ -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 {
Expand All @@ -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>,
Expand Down
94 changes: 28 additions & 66 deletions compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
);

Expand Down Expand Up @@ -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);
}

Expand All @@ -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,
));
}
}

Expand Down
Loading

0 comments on commit bf62436

Please sign in to comment.