Skip to content

Support using Self or projections inside an RPIT/async fn #103491

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

Merged
merged 15 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Inherit generics for impl-trait.
  • Loading branch information
cjgillot committed Nov 12, 2022
commit 5fc261e9a0902cc7aa41fd434b23fa0c18ed9d7f
33 changes: 5 additions & 28 deletions compiler/rustc_hir_analysis/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2767,35 +2767,11 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
let substs = InternalSubsts::for_item(tcx, def_id, |param, _| {
if let Some(i) = (param.index as usize).checked_sub(generics.parent_count) {
// Our own parameters are the resolved lifetimes.
if let GenericParamDefKind::Lifetime = param.kind {
if let hir::GenericArg::Lifetime(lifetime) = &lifetimes[i] {
self.ast_region_to_region(lifetime, None).into()
} else {
bug!()
}
} else {
bug!()
}
let GenericParamDefKind::Lifetime { .. } = param.kind else { bug!() };
let hir::GenericArg::Lifetime(lifetime) = &lifetimes[i] else { bug!() };
self.ast_region_to_region(lifetime, None).into()
} else {
match param.kind {
// For RPIT (return position impl trait), only lifetimes
// mentioned in the impl Trait predicate are captured by
// the opaque type, so the lifetime parameters from the
// parent item need to be replaced with `'static`.
//
// For `impl Trait` in the types of statics, constants,
// locals and type aliases. These capture all parent
// lifetimes, so they can use their identity subst.
GenericParamDefKind::Lifetime
if matches!(
origin,
hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..)
) =>
{
tcx.lifetimes.re_static.into()
}
_ => tcx.mk_param_from_def(param),
}
tcx.mk_param_from_def(param)
}
});
debug!("impl_trait_ty_to_ty: substs={:?}", substs);
Expand Down Expand Up @@ -2972,6 +2948,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
Some(tcx.liberate_late_bound_regions(fn_hir_id.expect_owner().to_def_id(), ty))
}

#[instrument(level = "trace", skip(self, generate_err))]
fn validate_late_bound_regions(
&self,
constrained_regions: FxHashSet<ty::BoundRegionKind>,
Expand Down
93 changes: 79 additions & 14 deletions compiler/rustc_hir_analysis/src/check/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ fn check_opaque<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) {
}
check_opaque_meets_bounds(tcx, item.owner_id.def_id, substs, span, &origin);
}

/// Checks that an opaque type does not use `Self` or `T::Foo` projections that would result
/// in "inheriting lifetimes".
#[instrument(level = "debug", skip(tcx, span))]
Expand All @@ -251,15 +252,19 @@ pub(super) fn check_opaque_for_inheriting_lifetimes<'tcx>(
let item = tcx.hir().expect_item(def_id);
debug!(?item, ?span);

#[derive(Debug)]
struct FoundParentLifetime;
struct FindParentLifetimeVisitor<'tcx>(&'tcx ty::Generics);
struct FindParentLifetimeVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
parent_count: u32,
}
impl<'tcx> ty::visit::TypeVisitor<'tcx> for FindParentLifetimeVisitor<'tcx> {
type BreakTy = FoundParentLifetime;

#[instrument(level = "trace", skip(self), ret)]
fn visit_region(&mut self, r: ty::Region<'tcx>) -> ControlFlow<Self::BreakTy> {
debug!("FindParentLifetimeVisitor: r={:?}", r);
if let ty::ReEarlyBound(ty::EarlyBoundRegion { index, .. }) = *r {
if index < self.0.parent_count as u32 {
if index < self.parent_count {
return ControlFlow::Break(FoundParentLifetime);
} else {
return ControlFlow::CONTINUE;
Expand All @@ -269,6 +274,63 @@ pub(super) fn check_opaque_for_inheriting_lifetimes<'tcx>(
r.super_visit_with(self)
}

#[instrument(level = "trace", skip(self), ret)]
fn visit_ty(&mut self, ty: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
// We're only interested in types involving regions
if !ty.flags().intersects(ty::TypeFlags::HAS_FREE_REGIONS) {
return ControlFlow::CONTINUE;
}

match ty.kind() {
ty::Closure(_, ref substs) => {
// Skip lifetime parameters of the enclosing item(s)

substs.as_closure().tupled_upvars_ty().visit_with(self)?;
substs.as_closure().sig_as_fn_ptr_ty().visit_with(self)?;
}

ty::Generator(_, ref substs, _) => {
// Skip lifetime parameters of the enclosing item(s)
// Also skip the witness type, because that has no free regions.

substs.as_generator().tupled_upvars_ty().visit_with(self)?;
substs.as_generator().return_ty().visit_with(self)?;
substs.as_generator().yield_ty().visit_with(self)?;
substs.as_generator().resume_ty().visit_with(self)?;
}

ty::Opaque(def_id, ref substs) => {
// Skip lifetime paramters that are not captures.
let variances = self.tcx.variances_of(*def_id);

for (v, s) in std::iter::zip(variances, substs.iter()) {
if *v != ty::Variance::Bivariant {
s.visit_with(self)?;
}
}
}

ty::Projection(proj)
if self.tcx.def_kind(proj.item_def_id) == DefKind::ImplTraitPlaceholder =>
{
// Skip lifetime paramters that are not captures.
let variances = self.tcx.variances_of(proj.item_def_id);

for (v, s) in std::iter::zip(variances, proj.substs.iter()) {
if *v != ty::Variance::Bivariant {
s.visit_with(self)?;
}
}
}

_ => {
ty.super_visit_with(self)?;
}
}

ControlFlow::CONTINUE
}

fn visit_const(&mut self, c: ty::Const<'tcx>) -> ControlFlow<Self::BreakTy> {
if let ty::ConstKind::Unevaluated(..) = c.kind() {
// FIXME(#72219) We currently don't detect lifetimes within substs
Expand All @@ -291,12 +353,15 @@ pub(super) fn check_opaque_for_inheriting_lifetimes<'tcx>(
type BreakTy = Ty<'tcx>;

fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
debug!("check_opaque_for_inheriting_lifetimes: (visit_ty) t={:?}", t);
debug!(?t, "root_visit_ty");
if t == self.opaque_identity_ty {
ControlFlow::CONTINUE
} else {
t.super_visit_with(&mut FindParentLifetimeVisitor(self.generics))
.map_break(|FoundParentLifetime| t)
t.visit_with(&mut FindParentLifetimeVisitor {
tcx: self.tcx,
parent_count: self.generics.parent_count as u32,
})
.map_break(|FoundParentLifetime| t)
}
}
}
Expand Down Expand Up @@ -329,14 +394,18 @@ pub(super) fn check_opaque_for_inheriting_lifetimes<'tcx>(

if let ItemKind::OpaqueTy(hir::OpaqueTy {
origin: hir::OpaqueTyOrigin::AsyncFn(..) | hir::OpaqueTyOrigin::FnReturn(..),
in_trait,
..
}) = item.kind
{
let substs = InternalSubsts::identity_for_item(tcx, def_id.to_def_id());
let opaque_identity_ty = if in_trait {
tcx.mk_projection(def_id.to_def_id(), substs)
} else {
tcx.mk_opaque(def_id.to_def_id(), substs)
};
let mut visitor = ProhibitOpaqueVisitor {
opaque_identity_ty: tcx.mk_opaque(
def_id.to_def_id(),
InternalSubsts::identity_for_item(tcx, def_id.to_def_id()),
),
opaque_identity_ty,
generics: tcx.generics_of(def_id),
tcx,
selftys: vec![],
Expand All @@ -345,10 +414,6 @@ pub(super) fn check_opaque_for_inheriting_lifetimes<'tcx>(
.explicit_item_bounds(def_id)
.iter()
.try_for_each(|(predicate, _)| predicate.visit_with(&mut visitor));
debug!(
"check_opaque_for_inheriting_lifetimes: prohibit_opaque={:?}, visitor.opaque_identity_ty={:?}, visitor.generics={:?}",
prohibit_opaque, visitor.opaque_identity_ty, visitor.generics
);

if let Some(ty) = prohibit_opaque.break_value() {
visitor.visit_item(&item);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ fn label_msg_span(
}
}

#[instrument(level = "trace", skip(tcx))]
pub fn unexpected_hidden_region_diagnostic<'tcx>(
tcx: TyCtxt<'tcx>,
span: Span,
Expand Down
63 changes: 36 additions & 27 deletions compiler/rustc_infer/src/infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,32 +332,11 @@ impl<'tcx> InferCtxt<'tcx> {
concrete_ty: Ty<'tcx>,
span: Span,
) {
let def_id = opaque_type_key.def_id;

let tcx = self.tcx;

let concrete_ty = self.resolve_vars_if_possible(concrete_ty);

debug!(?concrete_ty);

let first_own_region = match self.opaque_ty_origin_unchecked(def_id, span) {
hir::OpaqueTyOrigin::FnReturn(..) | hir::OpaqueTyOrigin::AsyncFn(..) => {
// We lower
//
// fn foo<'l0..'ln>() -> impl Trait<'l0..'lm>
//
// into
//
// type foo::<'p0..'pn>::Foo<'q0..'qm>
// fn foo<l0..'ln>() -> foo::<'static..'static>::Foo<'l0..'lm>.
//
// For these types we only iterate over `'l0..lm` below.
tcx.generics_of(def_id).parent_count
}
// These opaque type inherit all lifetime parameters from their
// parent, so we have to check them all.
hir::OpaqueTyOrigin::TyAlias => 0,
};
let variances = self.tcx.variances_of(opaque_type_key.def_id);
debug!(?variances);

// For a case like `impl Foo<'a, 'b>`, we would generate a constraint
// `'r in ['a, 'b, 'static]` for each region `'r` that appears in the
Expand All @@ -370,9 +349,12 @@ impl<'tcx> InferCtxt<'tcx> {
// type can be equal to any of the region parameters of the
// opaque type definition.
let choice_regions: Lrc<Vec<ty::Region<'tcx>>> = Lrc::new(
opaque_type_key.substs[first_own_region..]
opaque_type_key
.substs
.iter()
.filter_map(|arg| match arg.unpack() {
.enumerate()
.filter(|(i, _)| variances[*i] == ty::Variance::Invariant)
.filter_map(|(_, arg)| match arg.unpack() {
GenericArgKind::Lifetime(r) => Some(r),
GenericArgKind::Type(_) | GenericArgKind::Const(_) => None,
})
Expand All @@ -381,6 +363,7 @@ impl<'tcx> InferCtxt<'tcx> {
);

concrete_ty.visit_with(&mut ConstrainOpaqueTypeRegionVisitor {
tcx: self.tcx,
op: |r| self.member_constraint(opaque_type_key, span, concrete_ty, r, &choice_regions),
});
}
Expand Down Expand Up @@ -440,11 +423,12 @@ impl<'tcx> InferCtxt<'tcx> {
//
// We ignore any type parameters because impl trait values are assumed to
// capture all the in-scope type parameters.
struct ConstrainOpaqueTypeRegionVisitor<OP> {
struct ConstrainOpaqueTypeRegionVisitor<'tcx, OP> {
tcx: TyCtxt<'tcx>,
op: OP,
}

impl<'tcx, OP> TypeVisitor<'tcx> for ConstrainOpaqueTypeRegionVisitor<OP>
impl<'tcx, OP> TypeVisitor<'tcx> for ConstrainOpaqueTypeRegionVisitor<'tcx, OP>
where
OP: FnMut(ty::Region<'tcx>),
{
Expand Down Expand Up @@ -490,6 +474,31 @@ where
substs.as_generator().yield_ty().visit_with(self);
substs.as_generator().resume_ty().visit_with(self);
}

ty::Opaque(def_id, ref substs) => {
// Skip lifetime paramters that are not captures.
let variances = self.tcx.variances_of(*def_id);

for (v, s) in std::iter::zip(variances, substs.iter()) {
if *v != ty::Variance::Bivariant {
s.visit_with(self);
}
}
}

ty::Projection(proj)
if self.tcx.def_kind(proj.item_def_id) == DefKind::ImplTraitPlaceholder =>
{
// Skip lifetime paramters that are not captures.
let variances = self.tcx.variances_of(proj.item_def_id);

for (v, s) in std::iter::zip(variances, proj.substs.iter()) {
if *v != ty::Variance::Bivariant {
s.visit_with(self);
}
}
}

_ => {
ty.super_visit_with(self);
}
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1338,8 +1338,8 @@ impl<'tcx> OpaqueHiddenType<'tcx> {
// 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::AsyncFn(_) => map.collect(),
OpaqueTyOrigin::FnReturn(_) | OpaqueTyOrigin::TyAlias => {
OpaqueTyOrigin::FnReturn(_) | OpaqueTyOrigin::AsyncFn(_) => map.collect(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change is wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is that self.ty may contain a region outside of the member constraints, which trips the "non-defining opaque type in defining scope" error.

OpaqueTyOrigin::TyAlias => {
// Opaque types may only use regions that are bound. So for
// ```rust
// type Foo<'a, 'b, 'c> = impl Trait<'a> + 'b;
Expand Down
22 changes: 21 additions & 1 deletion compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,26 @@ pub fn relate_substs_with_variances<'tcx, R: TypeRelation<'tcx>>(
tcx.mk_substs(params)
}

#[instrument(level = "trace", skip(relation), ret)]
fn relate_opaque_item_substs<'tcx, R: TypeRelation<'tcx>>(
relation: &mut R,
def_id: DefId,
a_subst: SubstsRef<'tcx>,
b_subst: SubstsRef<'tcx>,
) -> RelateResult<'tcx, SubstsRef<'tcx>> {
let tcx = relation.tcx();
let variances = tcx.variances_of(def_id);
debug!(?variances);

let params = iter::zip(a_subst, b_subst).enumerate().map(|(i, (a, b))| {
let variance = variances[i];
let variance_info = ty::VarianceDiagInfo::default();
relation.relate_with_variance(variance, variance_info, a, b)
});

tcx.mk_substs(params)
}

impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> {
fn relate<R: TypeRelation<'tcx>>(
relation: &mut R,
Expand Down Expand Up @@ -561,7 +581,7 @@ pub fn super_relate_tys<'tcx, R: TypeRelation<'tcx>>(
(&ty::Opaque(a_def_id, a_substs), &ty::Opaque(b_def_id, b_substs))
if a_def_id == b_def_id =>
{
let substs = relate_substs(relation, a_substs, b_substs)?;
let substs = relate_opaque_item_substs(relation, a_def_id, a_substs, b_substs)?;
Ok(tcx.mk_opaque(a_def_id, substs))
}

Expand Down