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

Relate alias ty with variance #116219

Merged
merged 3 commits into from
Oct 11, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/equate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ impl<'tcx> TypeRelation<'tcx> for Equate<'_, '_, 'tcx> {
// performing trait matching (which then performs equality
// unification).

relate::relate_args(self, a_arg, b_arg)
relate::relate_args_invariantly(self, a_arg, b_arg)
}

fn relate_with_variance<T: Relate<'tcx>>(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/generalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ where
// Avoid fetching the variance if we are in an invariant
// context; no need, and it can induce dependency cycles
// (e.g., #41849).
relate::relate_args(self, a_subst, b_subst)
relate::relate_args_invariantly(self, a_subst, b_subst)
} else {
let tcx = self.tcx();
let opt_variances = tcx.variances_of(item_def_id);
Expand Down
65 changes: 24 additions & 41 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use crate::ty::error::{ExpectedFound, TypeError};
use crate::ty::{self, Expr, ImplSubject, Term, TermKind, Ty, TyCtxt, TypeFoldable};
use crate::ty::{GenericArg, GenericArgKind, GenericArgsRef};
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_hir::def_id::DefId;
use rustc_target::spec::abi;
use std::iter;
Expand Down Expand Up @@ -134,7 +135,7 @@ pub fn relate_type_and_mut<'tcx, R: TypeRelation<'tcx>>(
}

#[inline]
pub fn relate_args<'tcx, R: TypeRelation<'tcx>>(
pub fn relate_args_invariantly<'tcx, R: TypeRelation<'tcx>>(
relation: &mut R,
a_arg: GenericArgsRef<'tcx>,
b_arg: GenericArgsRef<'tcx>,
Expand Down Expand Up @@ -273,7 +274,20 @@ impl<'tcx> Relate<'tcx> for ty::AliasTy<'tcx> {
if a.def_id != b.def_id {
Err(TypeError::ProjectionMismatched(expected_found(relation, a.def_id, b.def_id)))
} else {
let args = relation.relate(a.args, b.args)?;
let args = match relation.tcx().def_kind(a.def_id) {
DefKind::OpaqueTy => relate_args_with_variances(
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it be fine to use variances_of for all other aliases as well? at this point you could use relation.relate_item_args which avoids any lookup in Equate (and needs to call variances_of regardless if we're not in Equate)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think variances_of is impl'd for projections. I guess I could impl it, but that may have other perf implications.

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, alternatively, instead of using the DefKind, have the Ty::relate provide the AliasKind to the substs relate.

do we need ProjectionTy to implement Relate itself, or would an inherent method relate which also takes the AliasKind be possible?

Copy link
Member Author

Choose a reason for hiding this comment

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

do we need ProjectionTy to implement Relate itself, or would an inherent method relate which also takes the AliasKind be possible?

This is pretty difficult, because in order to invoke a relation, you need something that implements Self: ToTrace. That requires Self: Relate. I guess I could poke a hole through with something like trace, but even then, it doesn't seem really ergonomic. Or I could separate out ToTrace from Relate or something...

This also makes this code significantly uglier:

fn assemble_subst_relate_candidate(
&mut self,
param_env: ty::ParamEnv<'tcx>,
alias_lhs: ty::AliasTy<'tcx>,
alias_rhs: ty::AliasTy<'tcx>,
direction: ty::AliasRelationDirection,
) -> QueryResult<'tcx> {
self.probe_misc_candidate("args relate").enter(|ecx| {
match direction {
ty::AliasRelationDirection::Equate => {
ecx.eq(param_env, alias_lhs, alias_rhs)?;
}
ty::AliasRelationDirection::Subtype => {
ecx.sub(param_env, alias_lhs, alias_rhs)?;
}
}
ecx.evaluate_added_goals_and_make_canonical_response(Certainty::Yes)
})
}

Since we can't just use the generic eq and sub methods on EvalCtxt.

relation,
a.def_id,
relation.tcx().variances_of(a.def_id),
a.args,
b.args,
false, // do not fetch `type_of(a_def_id)`, as it will cause a cycle
)?,
DefKind::AssocTy | DefKind::AssocConst | DefKind::TyAlias => {
Copy link
Member

Choose a reason for hiding this comment

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

What about lazy type aliases? Shouldn't they be related with variances, too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered this, but it shouldn't matter in practice, since they're always normalizable.

This only really concerns things that must be considered as aliases.

relate_args_invariantly(relation, a.args, b.args)?
}
def => bug!("unknown alias DefKind: {def:?}"),
};
Ok(relation.tcx().mk_alias_ty(a.def_id, args))
}
}
Expand Down Expand Up @@ -315,7 +329,7 @@ impl<'tcx> Relate<'tcx> for ty::TraitRef<'tcx> {
if a.def_id != b.def_id {
Err(TypeError::Traits(expected_found(relation, a.def_id, b.def_id)))
} else {
let args = relate_args(relation, a.args, b.args)?;
let args = relate_args_invariantly(relation, a.args, b.args)?;
Ok(ty::TraitRef::new(relation.tcx(), a.def_id, args))
}
}
Expand All @@ -331,7 +345,7 @@ impl<'tcx> Relate<'tcx> for ty::ExistentialTraitRef<'tcx> {
if a.def_id != b.def_id {
Err(TypeError::Traits(expected_found(relation, a.def_id, b.def_id)))
} else {
let args = relate_args(relation, a.args, b.args)?;
let args = relate_args_invariantly(relation, a.args, b.args)?;
Ok(ty::ExistentialTraitRef { def_id: a.def_id, args })
}
}
Expand Down Expand Up @@ -449,7 +463,7 @@ pub fn structurally_relate_tys<'tcx, R: TypeRelation<'tcx>>(
// All Generator types with the same id represent
// the (anonymous) type of the same generator expression. So
// all of their regions should be equated.
let args = relation.relate(a_args, b_args)?;
let args = relate_args_invariantly(relation, a_args, b_args)?;
Ok(Ty::new_generator(tcx, a_id, args, movability))
}

Expand All @@ -459,15 +473,15 @@ pub fn structurally_relate_tys<'tcx, R: TypeRelation<'tcx>>(
// All GeneratorWitness types with the same id represent
// the (anonymous) type of the same generator expression. So
// all of their regions should be equated.
let args = relation.relate(a_args, b_args)?;
let args = relate_args_invariantly(relation, a_args, b_args)?;
Ok(Ty::new_generator_witness(tcx, a_id, args))
}

(&ty::Closure(a_id, a_args), &ty::Closure(b_id, b_args)) if a_id == b_id => {
// All Closure types with the same id represent
// the (anonymous) type of the same closure expression. So
// all of their regions should be equated.
let args = relation.relate(a_args, b_args)?;
let args = relate_args_invariantly(relation, a_args, b_args)?;
Ok(Ty::new_closure(tcx, a_id, &args))
}

Expand Down Expand Up @@ -536,24 +550,6 @@ pub fn structurally_relate_tys<'tcx, R: TypeRelation<'tcx>>(
Ok(Ty::new_fn_ptr(tcx, fty))
}

// The args of opaque types may not all be invariant, so we have
// to treat them separately from other aliases.
(
&ty::Alias(ty::Opaque, ty::AliasTy { def_id: a_def_id, args: a_args, .. }),
&ty::Alias(ty::Opaque, ty::AliasTy { def_id: b_def_id, args: b_args, .. }),
) if a_def_id == b_def_id => {
let opt_variances = tcx.variances_of(a_def_id);
let args = relate_args_with_variances(
relation,
a_def_id,
opt_variances,
a_args,
b_args,
false, // do not fetch `type_of(a_def_id)`, as it will cause a cycle
)?;
Ok(Ty::new_opaque(tcx, a_def_id, args))
}

// Alias tend to mostly already be handled downstream due to normalization.
(&ty::Alias(a_kind, a_data), &ty::Alias(b_kind, b_data)) => {
let alias_ty = relation.relate(a_data, b_data)?;
Expand Down Expand Up @@ -709,7 +705,7 @@ impl<'tcx> Relate<'tcx> for ty::ClosureArgs<'tcx> {
a: ty::ClosureArgs<'tcx>,
b: ty::ClosureArgs<'tcx>,
) -> RelateResult<'tcx, ty::ClosureArgs<'tcx>> {
let args = relate_args(relation, a.args, b.args)?;
let args = relate_args_invariantly(relation, a.args, b.args)?;
Ok(ty::ClosureArgs { args })
}
}
Expand All @@ -720,7 +716,7 @@ impl<'tcx> Relate<'tcx> for ty::GeneratorArgs<'tcx> {
a: ty::GeneratorArgs<'tcx>,
b: ty::GeneratorArgs<'tcx>,
) -> RelateResult<'tcx, ty::GeneratorArgs<'tcx>> {
let args = relate_args(relation, a.args, b.args)?;
let args = relate_args_invariantly(relation, a.args, b.args)?;
Ok(ty::GeneratorArgs { args })
}
}
Expand All @@ -731,7 +727,7 @@ impl<'tcx> Relate<'tcx> for GenericArgsRef<'tcx> {
a: GenericArgsRef<'tcx>,
b: GenericArgsRef<'tcx>,
) -> RelateResult<'tcx, GenericArgsRef<'tcx>> {
relate_args(relation, a, b)
relate_args_invariantly(relation, a, b)
}
}

Expand Down Expand Up @@ -835,19 +831,6 @@ impl<'tcx> Relate<'tcx> for Term<'tcx> {
}
}

impl<'tcx> Relate<'tcx> for ty::ProjectionPredicate<'tcx> {
fn relate<R: TypeRelation<'tcx>>(
relation: &mut R,
a: ty::ProjectionPredicate<'tcx>,
b: ty::ProjectionPredicate<'tcx>,
) -> RelateResult<'tcx, ty::ProjectionPredicate<'tcx>> {
Ok(ty::ProjectionPredicate {
projection_ty: relation.relate(a.projection_ty, b.projection_ty)?,
term: relation.relate(a.term, b.term)?,
})
}
}

///////////////////////////////////////////////////////////////////////////
// Error handling

Expand Down
14 changes: 14 additions & 0 deletions tests/ui/impl-trait/in-trait/opaque-variances.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// check-pass
// compile-flags: -Ztrait-solver=next

fn foo<'a: 'a>(x: &'a Vec<i32>) -> impl Sized {
()
}

fn main() {
// in NLL, we want to make sure that the `'a` subst of `foo` does not get
// related between `x` and the RHS of the assignment. That would require
// that the temp is live for the lifetime of the variable `x`, which of
// course is not necessary since `'a` is not captured by the RPIT.
let x = foo(&Vec::new());
}
Loading