mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Rollup merge of #121479 - lcnr:fix-generalize, r=compiler-errors

Browse Source 
fix generalizer unsoundness

I ended up getting confused while trying to flip the variances when flipping the order. Should be all right now.

This is only exploitable when generalizing if the `ambient_variance` of the relation is `Contravariant`. This can currently only be the case in the NLL generalizer which only rarely generalizes, causing us to miss this regression. Very much an issue with #121462 however.
This commit is contained in:
Matthias Krüger 2024-02-23 09:42:12 +01:00 committed by GitHub
commit 7ae95b266e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 33 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
compiler/rustc_borrowck/src/type_check/relate_tys.rs
View File

@ -123,7 +123,11 @@ fn relate_opaques(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, ()>
// `handle_opaque_type` cannot handle subtyping, so to support subtyping
// we instead eagerly generalize here. This is a bit of a mess but will go
// away once we're using the new solver.
let mut enable_subtyping = |ty, ty_is_expected| {
//
// Given `opaque rel B`, we create a new infer var `ty_vid` constrain it
// by using `ty_vid rel B` and then finally and end by equating `ty_vid` to
// the opaque.
let mut enable_subtyping = |ty, opaque_is_expected| {
let ty_vid = infcx.next_ty_var_id_in_universe(
TypeVariableOrigin {
kind: TypeVariableOriginKind::MiscVariable,
@ -132,7 +136,7 @@ fn relate_opaques(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, ()>
ty::UniverseIndex::ROOT,
);
let variance = if ty_is_expected {
let variance = if opaque_is_expected {
self.ambient_variance
} else {
self.ambient_variance.xform(ty::Contravariant)
@ -140,7 +144,7 @@ fn relate_opaques(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, ()>
self.type_checker.infcx.instantiate_ty_var(
self,
ty_is_expected,
opaque_is_expected,
ty_vid,
variance,
ty,
@ -149,8 +153,8 @@ fn relate_opaques(&mut self, a: Ty<'tcx>, b: Ty<'tcx>) -> RelateResult<'tcx, ()>
};
let (a, b) = match (a.kind(), b.kind()) {
(&ty::Alias(ty::Opaque, ..), _) => (a, enable_subtyping(b, false)?),
(_, &ty::Alias(ty::Opaque, ..)) => (enable_subtyping(a, true)?, b),
(&ty::Alias(ty::Opaque, ..), _) => (a, enable_subtyping(b, true)?),
(_, &ty::Alias(ty::Opaque, ..)) => (enable_subtyping(a, false)?, b),
_ => unreachable!(
"expected at least one opaque type in `relate_opaques`, got {a} and {b}."
),

42
compiler/rustc_infer/src/infer/relate/generalize.rs
View File

@ -26,13 +26,13 @@ impl<'tcx> InferCtxt<'tcx> {
/// This is *not* expected to be used anywhere except for an implementation of
/// `TypeRelation`. Do not use this, and instead please use `At::eq`, for all
/// other usecases (i.e. setting the value of a type var).
#[instrument(level = "debug", skip(self, relation, target_is_expected))]
#[instrument(level = "debug", skip(self, relation))]
pub fn instantiate_ty_var<R: ObligationEmittingRelation<'tcx>>(
&self,
relation: &mut R,
target_is_expected: bool,
target_vid: ty::TyVid,
ambient_variance: ty::Variance,
instantiation_variance: ty::Variance,
source_ty: Ty<'tcx>,
) -> RelateResult<'tcx, ()> {
debug_assert!(self.inner.borrow_mut().type_variables().probe(target_vid).is_unknown());
@ -46,7 +46,7 @@ pub fn instantiate_ty_var<R: ObligationEmittingRelation<'tcx>>(
//
// We then relate `generalized_ty <: source_ty`,adding constraints like `'x: '?2` and `?1 <: ?3`.
let Generalization { value_may_be_infer: generalized_ty, has_unconstrained_ty_var } =
self.generalize(relation.span(), target_vid, ambient_variance, source_ty)?;
self.generalize(relation.span(), target_vid, instantiation_variance, source_ty)?;
// Constrain `b_vid` to the generalized type `generalized_ty`.
if let &ty::Infer(ty::TyVar(generalized_vid)) = generalized_ty.kind() {
@ -73,7 +73,7 @@ pub fn instantiate_ty_var<R: ObligationEmittingRelation<'tcx>>(
// the alias can be normalized to something which does not
// mention `?0`.
if self.next_trait_solver() {
let (lhs, rhs, direction) = match ambient_variance {
let (lhs, rhs, direction) = match instantiation_variance {
ty::Variance::Invariant => {
(generalized_ty.into(), source_ty.into(), AliasRelationDirection::Equate)
}
@ -106,22 +106,28 @@ pub fn instantiate_ty_var<R: ObligationEmittingRelation<'tcx>>(
}
}
} else {
// HACK: make sure that we `a_is_expected` continues to be
// correct when relating the generalized type with the source.
// NOTE: The `instantiation_variance` is not the same variance as
// used by the relation. When instantiating `b`, `target_is_expected`
// is flipped and the `instantion_variance` is also flipped. To
// constrain the `generalized_ty` while using the original relation,
// we therefore only have to flip the arguments.
//
// ```ignore (not code)
// ?a rel B
// instantiate_ty_var(?a, B) # expected and variance not flipped
// B' rel B
// ```
// or
// ```ignore (not code)
// A rel ?b
// instantiate_ty_var(?b, A) # expected and variance flipped
// A rel A'
// ```
if target_is_expected == relation.a_is_expected() {
relation.relate_with_variance(
ambient_variance,
ty::VarianceDiagInfo::default(),
generalized_ty,
source_ty,
)?;
relation.relate(generalized_ty, source_ty)?;
} else {
relation.relate_with_variance(
ambient_variance.xform(ty::Contravariant),
ty::VarianceDiagInfo::default(),
source_ty,
generalized_ty,
)?;
debug!("flip relation");
relation.relate(source_ty, generalized_ty)?;
}
}