Skip to content

Commit 968332e

Browse files
committed
Fix #9478: Improve partial unification handling
So far, like Scala 2, given: Either[Int, String] <:< ?F[String] we were able to infer: ?F >: [X] =>> Either[Int, X] However, in the inverse situation: ?F[String] <:< Either[Int, String] Scala 2, but not Dotty, was able to infer: ?F <: [X] =>> Either[Int, X] This commit fixes this by generalizing the partial unification logic to be run both in `compareAppliedType2` and `compareAppliedType1`, this broke a few tests: - In `anykind.scala`, `kinder1` and `kinder2` became ambiguous, this is fixed by moving `kinder2` in a lower priority base trait. - One of the implicit search in tests/neg/i3452.scala now goes into an infinite loop, I've disabled it for now. The issue seems to be related to by-name implicits as the divergence checker works when the by-name is removed from one of the implicit parameter, I've added the non-by-name version in the same file. I've also been able to reproduce the same symptoms on master and filed #9568.
1 parent 5fb13aa commit 968332e

File tree

4 files changed

+103
-33
lines changed

4 files changed

+103
-33
lines changed

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

Lines changed: 82 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,85 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi
849849
case _ =>
850850
isSubType(pre1, pre2)
851851

852+
/** Compare `tycon[args]` with `other := otherTycon[otherArgs]`, via `>:>` if fromBelow is true, `<:<` otherwise
853+
* (we call this relationship `~:~` in the rest of this comment).
854+
*
855+
* This method works by:
856+
*
857+
* 1. Choosing an appropriate type constructor `adaptedTycon`
858+
* 2. Constraining `tycon` such that `tycon ~:~ adaptedTycon`
859+
* 3. Recursing on `adaptedTycon[args] ~:~ other`
860+
*
861+
* So, how do we pick `adaptedTycon`? When `args` and `otherArgs` have the
862+
* same length the answer is simply:
863+
*
864+
* adaptedTycon := otherTycon
865+
*
866+
* But we also handle having `args.length < otherArgs.length`, in which
867+
* case we need to make up a type constructor of the right kind. For
868+
* example, if `fromBelow = false` and we're comparing:
869+
*
870+
* ?F[A] <:< Either[String, B] where `?F <: [X] =>> Any`
871+
*
872+
* we will choose:
873+
*
874+
* adaptedTycon := [X] =>> Either[String, X]
875+
*
876+
* this allows us to constrain:
877+
*
878+
* ?F <: adaptedTycon
879+
*
880+
* and then recurse on:
881+
*
882+
* adaptedTycon[A] <:< Either[String, B]
883+
*
884+
* In general, given:
885+
*
886+
* - k := args.length
887+
* - d := otherArgs.length - k
888+
*
889+
* `adaptedTycon` will be:
890+
*
891+
* [T_0, ..., T_k-1] =>> otherTycon[otherArgs(0), ..., otherArgs(d-1), T_0, ..., T_k-1]
892+
*
893+
* where `T_n` has the same bounds as `otherTycon.typeParams(d+n)`
894+
*
895+
* Historical note: this strategy is known in Scala as "partial unification"
896+
* (even though the type constructor variable isn't actually unified but only
897+
* has one of its bounds constrained), for background see:
898+
* - The infamous SI-2712: https://github.com/scala/bug/issues/2712
899+
* - The PR against Scala 2.12 implementing -Ypartial-unification: https://github.com/scala/scala/pull/5102
900+
* - Some explanations on how this impacts API design: https://gist.github.com/djspiewak/7a81a395c461fd3a09a6941d4cd040f2
901+
*/
902+
def compareAppliedTypeParamRef(tycon: TypeParamRef, args: List[Type], other: AppliedType, fromBelow: Boolean): Boolean =
903+
def directionalIsSubType(tp1: Type, tp2: Type): Boolean =
904+
if fromBelow then isSubType(tp2, tp1) else isSubType(tp1, tp2)
905+
def directionalRecur(tp1: Type, tp2: Type): Boolean =
906+
if fromBelow then recur(tp2, tp1) else recur(tp1, tp2)
907+
908+
val otherTycon = other.tycon
909+
val otherArgs = other.args
910+
911+
val d = otherArgs.length - args.length
912+
d >= 0 && {
913+
val tparams = tycon.typeParams
914+
val remainingTparams = otherTycon.typeParams.drop(d)
915+
variancesConform(remainingTparams, tparams) && {
916+
val adaptedTycon =
917+
if d > 0 then
918+
HKTypeLambda(remainingTparams.map(_.paramName))(
919+
tl => remainingTparams.map(remainingTparam =>
920+
tl.integrate(remainingTparams, remainingTparam.paramInfo).bounds),
921+
tl => otherTycon.appliedTo(
922+
otherArgs.take(d) ++ tl.paramRefs))
923+
else
924+
otherTycon
925+
(assumedTrue(tycon) || directionalIsSubType(tycon, adaptedTycon.ensureLambdaSub)) &&
926+
directionalRecur(adaptedTycon.appliedTo(args), other)
927+
}
928+
}
929+
end compareAppliedTypeParamRef
930+
852931
/** Subtype test for the hk application `tp2 = tycon2[args2]`.
853932
*/
854933
def compareAppliedType2(tp2: AppliedType, tycon2: Type, args2: List[Type]): Boolean = {
@@ -952,38 +1031,9 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi
9521031
* or fallback to fourthTry.
9531032
*/
9541033
def canInstantiate(tycon2: TypeParamRef): Boolean = {
955-
956-
/** Let
957-
*
958-
* `tparams_1, ..., tparams_k-1` be the type parameters of the rhs
959-
* `tparams1_1, ..., tparams1_n-1` be the type parameters of the constructor of the lhs
960-
* `args1_1, ..., args1_n-1` be the type arguments of the lhs
961-
* `d = n - k`
962-
*
963-
* Returns `true` iff `d >= 0` and `tycon2` can be instantiated to
964-
*
965-
* [tparams1_d, ... tparams1_n-1] -> tycon1[args_1, ..., args_d-1, tparams_d, ... tparams_n-1]
966-
*
967-
* such that the resulting type application is a supertype of `tp1`.
968-
*/
9691034
def appOK(tp1base: Type) = tp1base match {
9701035
case tp1base: AppliedType =>
971-
var tycon1 = tp1base.tycon
972-
val args1 = tp1base.args
973-
val tparams1all = tycon1.typeParams
974-
val lengthDiff = tparams1all.length - tparams.length
975-
lengthDiff >= 0 && {
976-
val tparams1 = tparams1all.drop(lengthDiff)
977-
variancesConform(tparams1, tparams) && {
978-
if (lengthDiff > 0)
979-
tycon1 = HKTypeLambda(tparams1.map(_.paramName))(
980-
tl => tparams1.map(tparam => tl.integrate(tparams, tparam.paramInfo).bounds),
981-
tl => tp1base.tycon.appliedTo(args1.take(lengthDiff) ++
982-
tparams1.indices.toList.map(tl.paramRefs(_))))
983-
(assumedTrue(tycon2) || isSubType(tycon1.ensureLambdaSub, tycon2)) &&
984-
recur(tp1, tycon1.appliedTo(args2))
985-
}
986-
}
1036+
compareAppliedTypeParamRef(tycon2, args2, tp1base, fromBelow = true)
9871037
case _ => false
9881038
}
9891039

@@ -1065,8 +1115,8 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi
10651115
tycon1 match {
10661116
case param1: TypeParamRef =>
10671117
def canInstantiate = tp2 match {
1068-
case AppliedType(tycon2, args2) =>
1069-
isSubType(param1, tycon2.ensureLambdaSub) && isSubArgs(args1, args2, tp1, tycon2.typeParams)
1118+
case tp2base: AppliedType =>
1119+
compareAppliedTypeParamRef(param1, args1, tp2base, fromBelow = false)
10701120
case _ =>
10711121
false
10721122
}

tests/neg/i3452.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,22 @@ object Test {
66
implicit def case1[F[_]](implicit t: => TC[F[Any]]): TC[Tuple2K[[_] =>> Any, F, Any]] = ???
77
implicit def case2[A, F[_]](implicit r: TC[F[Any]]): TC[A] = ???
88

9+
// Disabled because it leads to an infinite loop in implicit search
10+
// this is probably the same issue as https://github.com/lampepfl/dotty/issues/9568
11+
// implicitly[TC[Int]] // was: error
12+
}
13+
14+
object Test1 {
15+
case class Tuple2K[H[_], T[_], X](h: H[X], t: T[X])
16+
17+
trait TC[A]
18+
19+
implicit def case1[F[_]](implicit t: TC[F[Any]]): TC[Tuple2K[[_] =>> Any, F, Any]] = ???
20+
implicit def case2[A, F[_]](implicit r: TC[F[Any]]): TC[A] = ???
21+
922
implicitly[TC[Int]] // error
1023
}
24+
1125
object Test2 {
1226
trait TC[A]
1327

tests/pos/anykind.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,11 @@ object Test {
5656
object Kinder extends KinderLowerImplicits {
5757
type Aux[MA, M0 <: AnyKind, Args0 <: HList] = Kinder[MA] { type M = M0; type Args = Args0 }
5858

59-
implicit def kinder2[M0[_, _], A0, B0]: Kinder.Aux[M0[A0, B0], M0, A0 :: B0 :: HNil] = new Kinder[M0[A0, B0]] { type M[t, u] = M0[t, u]; type Args = A0 :: B0 :: HNil }
6059
implicit def kinder1[M0[_], A0]: Kinder.Aux[M0[A0], M0, A0 :: HNil] = new Kinder[M0[A0]] { type M[t] = M0[t]; type Args = A0 :: HNil }
6160
}
6261

6362
trait KinderLowerImplicits {
63+
implicit def kinder2[M0[_, _], A0, B0]: Kinder.Aux[M0[A0, B0], M0, A0 :: B0 :: HNil] = new Kinder[M0[A0, B0]] { type M[t, u] = M0[t, u]; type Args = A0 :: B0 :: HNil }
6464
implicit def kinder0[A]: Kinder.Aux[A, A, HNil] = new Kinder[A] { type M = A; type Args = HNil }
6565
}
6666

tests/pos/i9478.scala

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
class Foo[T[_, _], F[_], A, B](val fa: T[F[A], F[B]])
2+
3+
object Test {
4+
def x[T[_, _]](tmab: T[Either[Int, String], Either[Int, Int]]) =
5+
new Foo(tmab)
6+
}

0 commit comments

Comments
 (0)