Tweak tparam unification to work with lambda cleanup

[dwijnand]

Fixes #21981

[odersky]

It looks reasonable to me. @smarter WDYT? You wrote the original code.

[smarter]

I can reproduce the problem with the change in this PR if I change the receiver of map2 to be a tuple of type parameters (K1, K2) instead of a single parameter K:

trait Ops[F[_], A]:
  def map0[B](f0: A => B): F[B] = ???

trait Functor1[G[_]]

trait Functor2[H[_]]:
  extension [C1, C2](hc: H[(C1, C2)])
    def map2[D](f1: (C1, C2) => D): H[D]

trait Ref[I[_], +E]

final class Cov[+F]

class Test:
  given [J[_]](using J: Functor1[J]): Functor2[J] with
    extension [K1, K2](jk: J[(K1, K2)])
      def map2[L](f2: (K1, K2) => L): J[L] = ???

  def t1[
    M[_[t]],
    N[_],
  ](using N: Functor1[N]): Unit =

    val x3: Ops[N, M[[t] =>> Ref[N, t]]] = ???

    val x2: N[(M[N], M[[t] =>> Ref[N, t]])] = x3
      .map0 { refs             => (???, refs) }
      .map2 { case (not, refs) => (???, refs) }

The root cause of the regression is that the removal of type variables in https://github.com/scala/scala3/pull/21466/files#diff-73257963d8a6d79cd4878ae4ff164e33c36629f77821886ffc7cea2e2e38af39R78 does not check that the removed type variables are not in use somewhere. I spent some time looking into it but couldn't find a robust way to do so (maybe a version of Inferencing#interpolateTypeVars that doesn't require a tree argument could be used on pt in normalizedCompatible to do this clean-up?), so I think for now it'd be best to just revert #21466

[smarter]

I think we should still consider a revert of #21466 for the time being.

[smarter]

The check tvar1.instanceOpt == tvar accounts for that, but it doesn't account for B := Foo[K]. Here's a test case with B := Contra[K]:

case class Inv[T](x: T)
class Contra[-ContraParam](x: ContraParam)

trait Ops[F[_], A]:
  def map0[B](f0: A => Contra[B]): F[B] = ???

trait Functor1[G[_]]

trait Functor2[H[_]]

trait Ref[I[_], +E]

class Test:
  given [J[_]](using J: Functor1[J]): Functor2[J] with
    extension [K](jk: J[Contra[K]])
      def map2[L](f2: K => L): J[L] = ???

  def t1[
    M[_[t]],
    N[_],
  ](using N: Functor1[N]): Unit =

    val x3: Ops[N, M[[t] =>> Ref[N, t]]] = ???

    val x2: N[(M[N], M[[t] =>> Ref[N, t]])] = x3
      .map0 { refs             => Contra[Contra[(Nothing, M[[t] =>> Ref[N, t]])]](???) }
      .map2 { case (not, refs) => (???, refs) }

Now this could probably be fixed by checking tvar.occursIn(tvar1.instanceOpt) but at this point I'd started getting worried about time complexity: we're already inside two nested loops over all type variables and now we're traversing a type (in fact this is the traversal that dependsOn avoids for uninstantiated type variables).

[dwijnand]

Now this could probably be fixed by checking tvar.occursIn(tvar1.instanceOpt) but at this point I'd started getting worried about time complexity: we're already inside two nested loops over all type variables and now we're traversing a type (in fact this is the traversal that dependsOn avoids for uninstantiated type variables).

Thanks for all the review @smarter, it's been great feedback.

Do you have another idea how we can deal with the original issue (the extra lambda polluting the constraint) if we revert #21466?

[smarter]

Do you have another idea how we can deal with the original issue (the extra lambda polluting the constraint)

In Inferencing.interpolateTypeVars, we look for type variables among tp and pt using Inferencing.variances:

scala3/compiler/src/dotty/tools/dotc/typer/Inferencing.scala

Line 674 in 399d008

val vs = variances(tp, pt)

and variables which appear non-variantly are left uninstantiated (except if we can rely on their level to ensure instantiating them won't affect anything else in the constraint, cf

scala3/compiler/src/dotty/tools/dotc/typer/Inferencing.scala

Lines 723 to 729 in 399d008

	if !cmp.levelOK(tvar.nestingLevel, ctx.nestingLevel) then
	// Invariant: The type of a tree whose enclosing scope is level
	// N only contains type variables of level <= N.
	typr.println(i"instantiate nonvariant $tvar of level ${tvar.nestingLevel} to a type variable of level <= ${ctx.nestingLevel}, $constraint")
	cmp.atLevel(ctx.nestingLevel, tvar.origin)
	else
	typr.println(i"no interpolation for nonvariant $tvar in $state")

).

In tests/pos/i21981.scala if I call Inferencing.variances(tp, pt), then K doesn't get picked up, but I change the definition of variances to not skip part of pt:

diff --git compiler/src/dotty/tools/dotc/typer/Inferencing.scala compiler/src/dotty/tools/dotc/typer/Inferencing.scala
index 2ebcd96d5bd..96634cdb1ac 100644
--- compiler/src/dotty/tools/dotc/typer/Inferencing.scala
+++ compiler/src/dotty/tools/dotc/typer/Inferencing.scala
@@ -586,7 +586,7 @@ object Inferencing {
       if (vmap1 eq vmap) vmap else propagate(vmap1)
     }

-    propagate(accu(accu(VarianceMap.empty, tp), pt.finalResultType))
+    propagate(accu(accu(VarianceMap.empty, tp), pt))
   }

   /** Run the transformation after dealiasing but return the original type if it was a no-op. */

Then we do get K appearing non-variantly, which should be enough to prevent us from instantiating (ideally, we should refactor interpolateTypeVars to be callable without a tree instead of replicating part of its logic elsewhere).

I don't think I had a good reason for using pt.finalResultType here in variances, but I haven't tried to see what happens if we do get rid of it.

[dwijnand]

I don't think I had a good reason for using pt.finalResultType here in variances, but I haven't tried to see what happens if we do get rid of it.

Thanks for the suggestion. It looks like it works... until I encountered tests/pos-custom-args/captures/i16116.scala. In the code:

val fr = cpsAsync[Future] {
   val y = asyncPlus(1,Future successful 2).asInstanceOf[Int]
   y+1
}

We end up looking for the implicit CpsMonad[Future] that cpsAsync needs, but if weconsume all of pt we end up type-checking that a CpsTransform[Future] is needed, before makeContextualFunction gets a chance to introduce it into the context...