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

Author: lcnr

compiler/rustc_borrowck/src/type_check/relate_tys.rs

@@ -123,7 +123,11 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, '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,15 +136,15 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, '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)
             };
 
             self.type_checker.infcx.instantiate_ty_var(
                 self,
-                ty_is_expected,
+                opaque_is_expected,
                 ty_vid,
                 variance,
                 ty,
@@ -149,8 +153,8 @@ impl<'me, 'bccx, 'tcx> NllTypeRelating<'me, 'bccx, '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}."
             ),

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<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 @@ impl<'tcx> InferCtxt<'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 @@ impl<'tcx> InferCtxt<'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 @@ impl<'tcx> InferCtxt<'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
+            // ?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)?;
             }
         }