Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make TAITs and ATPITs capture late-bound lifetimes in scope #122103

Merged
merged 3 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 14 additions & 2 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
Some(ty) => this.lower_ty(
ty,
ImplTraitContext::TypeAliasesOpaqueTy { in_assoc_ty: false },
ImplTraitContext::OpaqueTy {
origin: hir::OpaqueTyOrigin::TyAlias {
parent: this.local_def_id(id),
in_assoc_ty: false,
},
fn_kind: None,
},
),
},
);
Expand Down Expand Up @@ -936,7 +942,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
Some(ty) => {
let ty = this.lower_ty(
ty,
ImplTraitContext::TypeAliasesOpaqueTy { in_assoc_ty: true },
ImplTraitContext::OpaqueTy {
origin: hir::OpaqueTyOrigin::TyAlias {
parent: this.local_def_id(i.id),
in_assoc_ty: true,
},
fn_kind: None,
},
);
hir::ImplItemKind::Type(ty)
}
Expand Down
58 changes: 25 additions & 33 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,12 @@ enum ImplTraitContext {
/// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
/// equivalent to a new opaque type like `type T = impl Debug; fn foo() -> T`.
///
ReturnPositionOpaqueTy {
/// Origin: Either OpaqueTyOrigin::FnReturn or OpaqueTyOrigin::AsyncFn,
OpaqueTy {
origin: hir::OpaqueTyOrigin,
fn_kind: FnDeclKind,
/// Only used to change the lifetime capture rules, since
/// RPITIT captures all in scope, RPIT does not.
fn_kind: Option<FnDeclKind>,
},
/// Impl trait in type aliases.
TypeAliasesOpaqueTy { in_assoc_ty: bool },
/// `impl Trait` is unstably accepted in this position.
FeatureGated(ImplTraitPosition, Symbol),
/// `impl Trait` is not accepted in this position.
Expand Down Expand Up @@ -1075,9 +1074,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// Disallow ATB in dyn types
if self.is_in_dyn_type {
let suggestion = match itctx {
ImplTraitContext::ReturnPositionOpaqueTy { .. }
| ImplTraitContext::TypeAliasesOpaqueTy { .. }
| ImplTraitContext::Universal => {
ImplTraitContext::OpaqueTy { .. } | ImplTraitContext::Universal => {
let bound_end_span = constraint
.gen_args
.as_ref()
Expand Down Expand Up @@ -1417,24 +1414,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
TyKind::ImplTrait(def_node_id, bounds) => {
let span = t.span;
match itctx {
ImplTraitContext::ReturnPositionOpaqueTy { origin, fn_kind } => self
.lower_opaque_impl_trait(
span,
origin,
*def_node_id,
bounds,
Some(fn_kind),
itctx,
),
ImplTraitContext::TypeAliasesOpaqueTy { in_assoc_ty } => self
.lower_opaque_impl_trait(
span,
hir::OpaqueTyOrigin::TyAlias { in_assoc_ty },
*def_node_id,
bounds,
None,
itctx,
),
ImplTraitContext::OpaqueTy { origin, fn_kind } => self.lower_opaque_impl_trait(
span,
origin,
*def_node_id,
bounds,
fn_kind,
itctx,
),
ImplTraitContext::Universal => {
let span = t.span;

Expand Down Expand Up @@ -1553,9 +1540,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let captured_lifetimes_to_duplicate = match origin {
hir::OpaqueTyOrigin::TyAlias { .. } => {
// in a TAIT like `type Foo<'a> = impl Foo<'a>`, we don't duplicate any
// lifetimes, since we don't have the issue that any are late-bound.
Vec::new()
// type alias impl trait and associated type position impl trait were
// decided to capture all in-scope lifetimes, which we collect for
// all opaques during resolution.
self.resolver
.take_extra_lifetime_params(opaque_ty_node_id)
.into_iter()
.map(|(ident, id, _)| Lifetime { id, ident })
.collect()
}
hir::OpaqueTyOrigin::FnReturn(..) => {
if matches!(
Expand Down Expand Up @@ -1823,9 +1815,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
FnDeclKind::Fn
| FnDeclKind::Inherent
| FnDeclKind::Trait
| FnDeclKind::Impl => ImplTraitContext::ReturnPositionOpaqueTy {
| FnDeclKind::Impl => ImplTraitContext::OpaqueTy {
origin: hir::OpaqueTyOrigin::FnReturn(self.local_def_id(fn_node_id)),
fn_kind: kind,
fn_kind: Some(kind),
},
FnDeclKind::ExternFn => {
ImplTraitContext::Disallowed(ImplTraitPosition::ExternFnReturn)
Expand Down Expand Up @@ -1919,9 +1911,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
output,
coro,
opaque_ty_span,
ImplTraitContext::ReturnPositionOpaqueTy {
ImplTraitContext::OpaqueTy {
origin: hir::OpaqueTyOrigin::FnReturn(fn_def_id),
fn_kind,
fn_kind: Some(fn_kind),
},
);
arena_vec![this; bound]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// fn f(_: impl Fn() -> impl Debug) -> impl Fn() -> impl Debug
// // disallowed --^^^^^^^^^^ allowed --^^^^^^^^^^
// ```
FnRetTy::Ty(ty) if matches!(itctx, ImplTraitContext::ReturnPositionOpaqueTy { .. }) => {
FnRetTy::Ty(ty) if matches!(itctx, ImplTraitContext::OpaqueTy { .. }) => {
if self.tcx.features().impl_trait_in_fn_trait_return {
self.lower_ty(ty, itctx)
} else {
Expand Down
19 changes: 11 additions & 8 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,17 @@ fn check_opaque_type_parameter_valid(
span: Span,
) -> Result<(), ErrorGuaranteed> {
let opaque_ty_hir = tcx.hir().expect_item(opaque_type_key.def_id);
let is_ty_alias = match opaque_ty_hir.expect_opaque_ty().origin {
OpaqueTyOrigin::TyAlias { .. } => true,
OpaqueTyOrigin::AsyncFn(..) | OpaqueTyOrigin::FnReturn(..) => false,
let (parent, is_ty_alias) = match opaque_ty_hir.expect_opaque_ty().origin {
OpaqueTyOrigin::TyAlias { parent, .. } => (parent, true),
OpaqueTyOrigin::AsyncFn(parent) | OpaqueTyOrigin::FnReturn(parent) => (parent, false),
};

let opaque_generics = tcx.generics_of(opaque_type_key.def_id);
let parent_generics = tcx.generics_of(parent);
let mut seen_params: FxIndexMap<_, Vec<_>> = FxIndexMap::default();
for (i, arg) in opaque_type_key.args.iter().enumerate() {

// Only check the parent generics, which will ignore any of the
// duplicated lifetime args that come from reifying late-bounds.
for (i, arg) in opaque_type_key.args.iter().take(parent_generics.count()).enumerate() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent generics are bivariant and can easily be unconstrained lifetime variables. This caused #122307.

if let Err(guar) = arg.error_reported() {
return Err(guar);
}
Expand All @@ -395,7 +398,7 @@ fn check_opaque_type_parameter_valid(
seen_params.entry(arg).or_default().push(i);
} else {
// Prevent `fn foo() -> Foo<u32>` from being defining.
let opaque_param = opaque_generics.param_at(i, tcx);
let opaque_param = parent_generics.param_at(i, tcx);
let kind = opaque_param.kind.descr();

return Err(tcx.dcx().emit_err(NonGenericOpaqueTypeParam {
Expand All @@ -409,10 +412,10 @@ fn check_opaque_type_parameter_valid(

for (_, indices) in seen_params {
if indices.len() > 1 {
let descr = opaque_generics.param_at(indices[0], tcx).kind.descr();
let descr = parent_generics.param_at(indices[0], tcx).kind.descr();
let spans: Vec<_> = indices
.into_iter()
.map(|i| tcx.def_span(opaque_generics.param_at(i, tcx).def_id))
.map(|i| tcx.def_span(parent_generics.param_at(i, tcx).def_id))
.collect();
#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)]
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2562,6 +2562,8 @@ pub enum OpaqueTyOrigin {
AsyncFn(LocalDefId),
/// type aliases: `type Foo = impl Trait;`
TyAlias {
/// The type alias or associated type parent of the TAIT/ATPIT
parent: LocalDefId,
/// associated types in impl blocks for traits.
in_assoc_ty: bool,
},
Expand Down
21 changes: 11 additions & 10 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,9 @@ fn check_opaque_meets_bounds<'tcx>(
origin: &hir::OpaqueTyOrigin,
) -> Result<(), ErrorGuaranteed> {
let defining_use_anchor = match *origin {
hir::OpaqueTyOrigin::FnReturn(did) | hir::OpaqueTyOrigin::AsyncFn(did) => did,
hir::OpaqueTyOrigin::TyAlias { .. } => tcx.impl_trait_parent(def_id),
hir::OpaqueTyOrigin::FnReturn(did)
| hir::OpaqueTyOrigin::AsyncFn(did)
| hir::OpaqueTyOrigin::TyAlias { parent: did, .. } => did,
};
let param_env = tcx.param_env(defining_use_anchor);

Expand All @@ -351,14 +352,14 @@ fn check_opaque_meets_bounds<'tcx>(
let ocx = ObligationCtxt::new(&infcx);

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),
hir::OpaqueTyOrigin::FnReturn(parent)
| hir::OpaqueTyOrigin::AsyncFn(parent)
| hir::OpaqueTyOrigin::TyAlias { parent, .. } => GenericArgs::identity_for_item(
tcx, parent,
)
.extend_to(tcx, def_id.to_def_id(), |param, _| {
tcx.map_opaque_lifetime_to_parent_lifetime(param.def_id.expect_local()).into()
}),
};

let opaque_ty = Ty::new_opaque(tcx, def_id.to_def_id(), args);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_analysis/src/collect/predicates_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ fn compute_bidirectional_outlives_predicates<'tcx>(
predicates: &mut Vec<(ty::Clause<'tcx>, Span)>,
) {
for param in opaque_own_params {
let orig_lifetime = tcx.map_rpit_lifetime_to_fn_lifetime(param.def_id.expect_local());
let orig_lifetime = tcx.map_opaque_lifetime_to_parent_lifetime(param.def_id.expect_local());
if let ty::ReEarlyParam(..) = *orig_lifetime {
let dup_lifetime = ty::Region::new_early_param(
tcx,
Expand Down
56 changes: 5 additions & 51 deletions compiler/rustc_hir_analysis/src/collect/resolve_bound_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,38 +514,11 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
// These sorts of items have no lifetime parameters at all.
intravisit::walk_item(self, item);
}
hir::ItemKind::OpaqueTy(hir::OpaqueTy {
origin: hir::OpaqueTyOrigin::TyAlias { .. },
..
}) => {
// Opaque types are visited when we visit the
// `TyKind::OpaqueDef`, so that they have the lifetimes from
// their parent opaque_ty in scope.
//
// The core idea here is that since OpaqueTys are generated with the impl Trait as
// their owner, we can keep going until we find the Item that owns that. We then
// conservatively add all resolved lifetimes. Otherwise we run into problems in
// cases like `type Foo<'a> = impl Bar<As = impl Baz + 'a>`.
let parent_item = self.tcx.hir().get_parent_item(item.hir_id());
let resolved_lifetimes: &ResolveBoundVars =
self.tcx.resolve_bound_vars(parent_item);
// We need to add *all* deps, since opaque tys may want them from *us*
for (&owner, defs) in resolved_lifetimes.defs.iter() {
defs.iter().for_each(|(&local_id, region)| {
self.map.defs.insert(hir::HirId { owner, local_id }, *region);
});
}
for (&owner, late_bound_vars) in resolved_lifetimes.late_bound_vars.iter() {
late_bound_vars.iter().for_each(|(&local_id, late_bound_vars)| {
self.record_late_bound_vars(
hir::HirId { owner, local_id },
late_bound_vars.clone(),
);
});
}
}
hir::ItemKind::OpaqueTy(&hir::OpaqueTy {
origin: hir::OpaqueTyOrigin::FnReturn(parent) | hir::OpaqueTyOrigin::AsyncFn(parent),
origin:
hir::OpaqueTyOrigin::FnReturn(parent)
| hir::OpaqueTyOrigin::AsyncFn(parent)
| hir::OpaqueTyOrigin::TyAlias { parent, .. },
generics,
..
}) => {
Expand Down Expand Up @@ -683,26 +656,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BoundVarContext<'a, 'tcx> {
// the opaque_ty generics
let opaque_ty = self.tcx.hir().item(item_id);
match &opaque_ty.kind {
hir::ItemKind::OpaqueTy(hir::OpaqueTy {
origin: hir::OpaqueTyOrigin::TyAlias { .. },
..
}) => {
intravisit::walk_ty(self, ty);

// Elided lifetimes and late-bound lifetimes (from the parent)
// are not allowed in non-return position impl Trait
let scope = Scope::LateBoundary {
s: &Scope::TraitRefBoundary { s: self.scope },
what: "type alias impl trait",
};
self.with(scope, |this| intravisit::walk_item(this, opaque_ty));

return;
}
hir::ItemKind::OpaqueTy(hir::OpaqueTy {
origin: hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..),
..
}) => {}
hir::ItemKind::OpaqueTy(hir::OpaqueTy { origin: _, .. }) => {}
i => bug!("`impl Trait` pointed to non-opaque type?? {:#?}", i),
};

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_hir_analysis/src/collect/type_of.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,11 +553,11 @@ pub(super) fn type_of_opaque(
Ok(ty::EarlyBinder::bind(match tcx.hir_node_by_def_id(def_id) {
Node::Item(item) => match item.kind {
ItemKind::OpaqueTy(OpaqueTy {
origin: hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: false },
origin: hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: false, .. },
..
}) => opaque::find_opaque_ty_constraints_for_tait(tcx, def_id),
ItemKind::OpaqueTy(OpaqueTy {
origin: hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: true },
origin: hir::OpaqueTyOrigin::TyAlias { in_assoc_ty: true, .. },
..
}) => opaque::find_opaque_ty_constraints_for_impl_trait_in_assoc_type(tcx, def_id),
// Opaque types desugared from `impl Trait`.
Expand Down
10 changes: 1 addition & 9 deletions compiler/rustc_hir_analysis/src/variance/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,7 @@ fn variance_of_opaque(tcx: TyCtxt<'_>, item_def_id: LocalDefId) -> &[ty::Varianc

// By default, RPIT are invariant wrt type and const generics, but they are bivariant wrt
// lifetime generics.
let variances = std::iter::repeat(ty::Invariant).take(generics.count());

let mut variances: Vec<_> = match tcx.opaque_type_origin(item_def_id) {
rustc_hir::OpaqueTyOrigin::FnReturn(_) | rustc_hir::OpaqueTyOrigin::AsyncFn(_) => {
variances.collect()
}
// But TAIT are invariant for all generics
rustc_hir::OpaqueTyOrigin::TyAlias { .. } => return tcx.arena.alloc_from_iter(variances),
};
let mut variances = vec![ty::Invariant; generics.count()];

// Mark all lifetimes from parent generics as unused (Bivariant).
// This will be overridden later if required.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/opaque_types/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ impl<'tcx> InferCtxt<'tcx> {
// Anonymous `impl Trait`
hir::OpaqueTyOrigin::FnReturn(parent) => parent == parent_def_id,
// Named `type Foo = impl Bar;`
hir::OpaqueTyOrigin::TyAlias { in_assoc_ty } => {
hir::OpaqueTyOrigin::TyAlias { in_assoc_ty, .. } => {
if in_assoc_ty {
self.tcx.opaque_types_defined_by(parent_def_id).contains(&def_id)
} else {
Expand Down