From fa2921bdca25960fbe68ad5fa14410f254a6c69e Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 22 Feb 2024 22:18:43 +0100 Subject: [PATCH 1/2] woops, soundly generalizing is hard I ended up getting confused while trying to flip the variances when flipping the order. Should be all right now --- .../src/type_check/relate_tys.rs | 14 ++++--- .../src/infer/relate/generalize.rs | 42 +++++++++++-------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_borrowck/src/type_check/relate_tys.rs b/compiler/rustc_borrowck/src/type_check/relate_tys.rs index dd355c3525c..61b803ea38d 100644 --- a/compiler/rustc_borrowck/src/type_check/relate_tys.rs +++ b/compiler/rustc_borrowck/src/type_check/relate_tys.rs @@ -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}." ), diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index 90f10a0eba9..1494730978b 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -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>( &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>( // // 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>( // 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>( } } } 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 + // ?a rel B + // instantiate_ty_var(?a, B) # expected and variance not flipped + // B' rel B + // ``` + // or + // ```ignore + // 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)?; } } From dabacb7431dbca5ea337ed5a456ecf756e2533af Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 22 Feb 2024 23:10:46 +0100 Subject: [PATCH 2/2] fix CI --- compiler/rustc_infer/src/infer/relate/generalize.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_infer/src/infer/relate/generalize.rs b/compiler/rustc_infer/src/infer/relate/generalize.rs index 1494730978b..eecdbbe5179 100644 --- a/compiler/rustc_infer/src/infer/relate/generalize.rs +++ b/compiler/rustc_infer/src/infer/relate/generalize.rs @@ -112,13 +112,13 @@ pub fn instantiate_ty_var>( // constrain the `generalized_ty` while using the original relation, // we therefore only have to flip the arguments. // - // ```ignore + // ```ignore (not code) // ?a rel B // instantiate_ty_var(?a, B) # expected and variance not flipped // B' rel B // ``` // or - // ```ignore + // ```ignore (not code) // A rel ?b // instantiate_ty_var(?b, A) # expected and variance flipped // A rel A'