Skip to content

Commit

Permalink
Rollup merge of rust-lang#116219 - compiler-errors:relate-alias-ty-wi…
Browse files Browse the repository at this point in the history
…th-variance, r=lcnr

Relate alias ty with variance

In the new solver, turns out that the subst-relate branch of the alias-relate predicate was relating args invariantly even for opaques, which have variance 💀.

This change is a bit more invasive, but I'd rather not special-case it [here](https://github.com/rust-lang/rust/blob/aeaa5c30e5c9041264a2e8314b68ad84c2dc3169/compiler/rustc_trait_selection/src/solve/alias_relate.rs#L171-L190) and then have it break elsewhere. I'm doing a perf run to see if the extra call to `def_kind` is that expensive, if it is, I'll reconsider.

r? `@lcnr`
  • Loading branch information
matthiaskrgr committed Oct 11, 2023
2 parents 5aa23be + be29d22 commit 9e7daf6
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 43 deletions.
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(
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 => {
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());
}

0 comments on commit 9e7daf6

Please sign in to comment.