From 1fb2082aadc12765f77047841f996d299b8cb031 Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 1 Aug 2020 22:08:58 +0200 Subject: [PATCH 1/6] Print remaining tests in case of timeout --- compiler/test/dotty/tools/vulpix/ParallelTesting.scala | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala index 37b9e852e030..7be747538852 100644 --- a/compiler/test/dotty/tools/vulpix/ParallelTesting.scala +++ b/compiler/test/dotty/tools/vulpix/ParallelTesting.scala @@ -15,6 +15,7 @@ import scala.io.Source import scala.util.{Random, Try, Failure => TryFailure, Success => TrySuccess, Using} import scala.util.control.NonFatal import scala.util.matching.Regex +import scala.collection.mutable.ListBuffer import dotc.{Compiler, Driver} import dotc.core.Contexts._ @@ -543,11 +544,18 @@ trait ParallelTesting extends RunnerOrchestration { self => } pool.shutdown() + if (!pool.awaitTermination(20, TimeUnit.MINUTES)) { + val remaining = new ListBuffer[TestSource] + filteredSources.lazyZip(eventualResults).foreach { (src, res) => + if (!res.isDone) + remaining += src + } + pool.shutdownNow() System.setOut(realStdout) System.setErr(realStderr) - throw new TimeoutException("Compiling targets timed out") + throw new TimeoutException(s"Compiling targets timed out, remaining targets: ${remaining.mkString(", ")}") } eventualResults.foreach { x => From 5fb13aad58cc554e7afc3b15f69d4d16698a1e5f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 11 Aug 2020 16:35:11 +0200 Subject: [PATCH 2/6] Don't dealias when matching type constructors Not only can this prevent some constraints from being inferred, it can lead to the wrong constraints being inferred due to partial unification, see comment. This change broke one test, in i6565.scala we had: type Lifted[A] = Err | A extension [O, U](o: Lifted[O]) def map(f: O => U): Lifted[U] = ??? val error: Err = Err() lazy val ok: Lifted[String] = { point("a").map(_ => if true then "foo" else error) // now fails because error does not have type String } Because `isMatchingApply` can now deal with aliased types, when typing the lambda we get the constraint `U <: String` from `Lifted[U] <: Lifted[String]`, so `Err` is no longer a subtype of the expected result type of the lambda. This can be fixed in a number of ways: I changed the test to use `flatMap` instead of `map`, but one could also remove the expected type, or replace it by `Lifted[Lifted[String]` or `Err | String`. I think the new behavior is arguably better since using type aliases now gives you more control on how type inference proceeds, even if it means that some things that used to typecheck don't anymore. This change is also necessary to get shapeless to compile after the following commit which makes partial unification work in more situation. --- .../dotty/tools/dotc/core/TypeComparer.scala | 26 +++++++++++++++++-- tests/pos/i6565.scala | 6 ++--- tests/run/hk-alias-unification.scala | 23 ++++++++++++++++ 3 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 tests/run/hk-alias-unification.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 1ceb4d5428ab..29706b79fb50 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -860,13 +860,35 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi */ def isMatchingApply(tp1: Type): Boolean = tp1 match { case AppliedType(tycon1, args1) => - def loop(tycon1: Type, args1: List[Type]): Boolean = tycon1.dealiasKeepRefiningAnnots match { + // We intentionally do not dealias `tycon1` or `tycon2` here. + // `TypeApplications#appliedTo` already takes care of dealiasing type + // constructors when this can be done without affecting type + // inference, doing it here would not only prevent code from compiling + // but could also result in the wrong thing being inferred later, for example + // in `tests/run/hk-alias-unification.scala` we end up checking: + // + // Foo[?F, ?T] <:< Foo[[X] =>> (X, String), Int] + // + // Naturally, we'd like to infer: + // + // ?F := [X] => (X, String) + // + // but if we dealias `Foo` then we'll end up trying to check: + // + // ErasedFoo[?F[?T]] <:< ErasedFoo[(Int, String)] + // + // Because of partial unification, this will succeed, but will produce the constraint: + // + // ?F := [X] =>> (Int, X) + // + // Which is not what we wanted! + def loop(tycon1: Type, args1: List[Type]): Boolean = tycon1 match { case tycon1: TypeParamRef => (tycon1 == tycon2 || canConstrain(tycon1) && isSubType(tycon1, tycon2)) && isSubArgs(args1, args2, tp1, tparams) case tycon1: TypeRef => - tycon2.dealiasKeepRefiningAnnots match { + tycon2 match { case tycon2: TypeRef => val tycon1sym = tycon1.symbol val tycon2sym = tycon2.symbol diff --git a/tests/pos/i6565.scala b/tests/pos/i6565.scala index 0672724724fd..194c7723c45e 100644 --- a/tests/pos/i6565.scala +++ b/tests/pos/i6565.scala @@ -8,10 +8,10 @@ extension [O, U](o: Lifted[O]) def flatMap(f: O => Lifted[U]): Lifted[U] = ??? val error: Err = Err() -lazy val ok: Lifted[String] = { // ok despite map returning a union - point("a").map(_ => if true then "foo" else error) // ok +lazy val ok: Lifted[String] = { + point("a").flatMap(_ => if true then "foo" else error) } lazy val nowAlsoOK: Lifted[String] = { - point("a").flatMap(_ => point("b").map(_ => if true then "foo" else error)) + point("a").flatMap(_ => point("b").flatMap(_ => if true then "foo" else error)) } diff --git a/tests/run/hk-alias-unification.scala b/tests/run/hk-alias-unification.scala new file mode 100644 index 000000000000..0b392d7648b2 --- /dev/null +++ b/tests/run/hk-alias-unification.scala @@ -0,0 +1,23 @@ +trait Bla[T] +object Bla { + implicit def blaInt: Bla[Int] = new Bla[Int] {} + implicit def blaString: Bla[String] = new Bla[String] { + assert(false, "I should not be summoned!") + } +} + +trait ErasedFoo[FT] +object Test { + type Foo[F[_], T] = ErasedFoo[F[T]] + type Foo2[F[_], T] = Foo[F, T] + + def mkFoo[F[_], T](implicit gen: Bla[T]): Foo[F, T] = new Foo[F, T] {} + def mkFoo2[F[_], T](implicit gen: Bla[T]): Foo2[F, T] = new Foo2[F, T] {} + + def main(args: Array[String]): Unit = { + val a: Foo[[X] =>> (X, String), Int] = mkFoo + val b: Foo2[[X] =>> (X, String), Int] = mkFoo + val c: Foo[[X] =>> (X, String), Int] = mkFoo2 + } +} + From 968332e44c026113c9c397d58cfdea7043686a8d Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sat, 1 Aug 2020 22:08:58 +0200 Subject: [PATCH 3/6] 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. --- .../dotty/tools/dotc/core/TypeComparer.scala | 114 +++++++++++++----- tests/neg/i3452.scala | 14 +++ tests/pos/anykind.scala | 2 +- tests/pos/i9478.scala | 6 + 4 files changed, 103 insertions(+), 33 deletions(-) create mode 100644 tests/pos/i9478.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 29706b79fb50..eb01f7e3243c 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -849,6 +849,85 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi case _ => isSubType(pre1, pre2) + /** Compare `tycon[args]` with `other := otherTycon[otherArgs]`, via `>:>` if fromBelow is true, `<:<` otherwise + * (we call this relationship `~:~` in the rest of this comment). + * + * This method works by: + * + * 1. Choosing an appropriate type constructor `adaptedTycon` + * 2. Constraining `tycon` such that `tycon ~:~ adaptedTycon` + * 3. Recursing on `adaptedTycon[args] ~:~ other` + * + * So, how do we pick `adaptedTycon`? When `args` and `otherArgs` have the + * same length the answer is simply: + * + * adaptedTycon := otherTycon + * + * But we also handle having `args.length < otherArgs.length`, in which + * case we need to make up a type constructor of the right kind. For + * example, if `fromBelow = false` and we're comparing: + * + * ?F[A] <:< Either[String, B] where `?F <: [X] =>> Any` + * + * we will choose: + * + * adaptedTycon := [X] =>> Either[String, X] + * + * this allows us to constrain: + * + * ?F <: adaptedTycon + * + * and then recurse on: + * + * adaptedTycon[A] <:< Either[String, B] + * + * In general, given: + * + * - k := args.length + * - d := otherArgs.length - k + * + * `adaptedTycon` will be: + * + * [T_0, ..., T_k-1] =>> otherTycon[otherArgs(0), ..., otherArgs(d-1), T_0, ..., T_k-1] + * + * where `T_n` has the same bounds as `otherTycon.typeParams(d+n)` + * + * Historical note: this strategy is known in Scala as "partial unification" + * (even though the type constructor variable isn't actually unified but only + * has one of its bounds constrained), for background see: + * - The infamous SI-2712: https://github.com/scala/bug/issues/2712 + * - The PR against Scala 2.12 implementing -Ypartial-unification: https://github.com/scala/scala/pull/5102 + * - Some explanations on how this impacts API design: https://gist.github.com/djspiewak/7a81a395c461fd3a09a6941d4cd040f2 + */ + def compareAppliedTypeParamRef(tycon: TypeParamRef, args: List[Type], other: AppliedType, fromBelow: Boolean): Boolean = + def directionalIsSubType(tp1: Type, tp2: Type): Boolean = + if fromBelow then isSubType(tp2, tp1) else isSubType(tp1, tp2) + def directionalRecur(tp1: Type, tp2: Type): Boolean = + if fromBelow then recur(tp2, tp1) else recur(tp1, tp2) + + val otherTycon = other.tycon + val otherArgs = other.args + + val d = otherArgs.length - args.length + d >= 0 && { + val tparams = tycon.typeParams + val remainingTparams = otherTycon.typeParams.drop(d) + variancesConform(remainingTparams, tparams) && { + val adaptedTycon = + if d > 0 then + HKTypeLambda(remainingTparams.map(_.paramName))( + tl => remainingTparams.map(remainingTparam => + tl.integrate(remainingTparams, remainingTparam.paramInfo).bounds), + tl => otherTycon.appliedTo( + otherArgs.take(d) ++ tl.paramRefs)) + else + otherTycon + (assumedTrue(tycon) || directionalIsSubType(tycon, adaptedTycon.ensureLambdaSub)) && + directionalRecur(adaptedTycon.appliedTo(args), other) + } + } + end compareAppliedTypeParamRef + /** Subtype test for the hk application `tp2 = tycon2[args2]`. */ def compareAppliedType2(tp2: AppliedType, tycon2: Type, args2: List[Type]): Boolean = { @@ -952,38 +1031,9 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi * or fallback to fourthTry. */ def canInstantiate(tycon2: TypeParamRef): Boolean = { - - /** Let - * - * `tparams_1, ..., tparams_k-1` be the type parameters of the rhs - * `tparams1_1, ..., tparams1_n-1` be the type parameters of the constructor of the lhs - * `args1_1, ..., args1_n-1` be the type arguments of the lhs - * `d = n - k` - * - * Returns `true` iff `d >= 0` and `tycon2` can be instantiated to - * - * [tparams1_d, ... tparams1_n-1] -> tycon1[args_1, ..., args_d-1, tparams_d, ... tparams_n-1] - * - * such that the resulting type application is a supertype of `tp1`. - */ def appOK(tp1base: Type) = tp1base match { case tp1base: AppliedType => - var tycon1 = tp1base.tycon - val args1 = tp1base.args - val tparams1all = tycon1.typeParams - val lengthDiff = tparams1all.length - tparams.length - lengthDiff >= 0 && { - val tparams1 = tparams1all.drop(lengthDiff) - variancesConform(tparams1, tparams) && { - if (lengthDiff > 0) - tycon1 = HKTypeLambda(tparams1.map(_.paramName))( - tl => tparams1.map(tparam => tl.integrate(tparams, tparam.paramInfo).bounds), - tl => tp1base.tycon.appliedTo(args1.take(lengthDiff) ++ - tparams1.indices.toList.map(tl.paramRefs(_)))) - (assumedTrue(tycon2) || isSubType(tycon1.ensureLambdaSub, tycon2)) && - recur(tp1, tycon1.appliedTo(args2)) - } - } + compareAppliedTypeParamRef(tycon2, args2, tp1base, fromBelow = true) case _ => false } @@ -1065,8 +1115,8 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi tycon1 match { case param1: TypeParamRef => def canInstantiate = tp2 match { - case AppliedType(tycon2, args2) => - isSubType(param1, tycon2.ensureLambdaSub) && isSubArgs(args1, args2, tp1, tycon2.typeParams) + case tp2base: AppliedType => + compareAppliedTypeParamRef(param1, args1, tp2base, fromBelow = false) case _ => false } diff --git a/tests/neg/i3452.scala b/tests/neg/i3452.scala index 1b16ff3d4d56..1a439338f4d6 100644 --- a/tests/neg/i3452.scala +++ b/tests/neg/i3452.scala @@ -6,8 +6,22 @@ object Test { implicit def case1[F[_]](implicit t: => TC[F[Any]]): TC[Tuple2K[[_] =>> Any, F, Any]] = ??? implicit def case2[A, F[_]](implicit r: TC[F[Any]]): TC[A] = ??? + // Disabled because it leads to an infinite loop in implicit search + // this is probably the same issue as https://github.com/lampepfl/dotty/issues/9568 + // implicitly[TC[Int]] // was: error +} + +object Test1 { + case class Tuple2K[H[_], T[_], X](h: H[X], t: T[X]) + + trait TC[A] + + implicit def case1[F[_]](implicit t: TC[F[Any]]): TC[Tuple2K[[_] =>> Any, F, Any]] = ??? + implicit def case2[A, F[_]](implicit r: TC[F[Any]]): TC[A] = ??? + implicitly[TC[Int]] // error } + object Test2 { trait TC[A] diff --git a/tests/pos/anykind.scala b/tests/pos/anykind.scala index 2e40ae070e13..356fdffa460b 100644 --- a/tests/pos/anykind.scala +++ b/tests/pos/anykind.scala @@ -56,11 +56,11 @@ object Test { object Kinder extends KinderLowerImplicits { type Aux[MA, M0 <: AnyKind, Args0 <: HList] = Kinder[MA] { type M = M0; type Args = Args0 } - 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 } 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 } } trait KinderLowerImplicits { + 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 } implicit def kinder0[A]: Kinder.Aux[A, A, HNil] = new Kinder[A] { type M = A; type Args = HNil } } diff --git a/tests/pos/i9478.scala b/tests/pos/i9478.scala new file mode 100644 index 000000000000..33d748e72e9b --- /dev/null +++ b/tests/pos/i9478.scala @@ -0,0 +1,6 @@ +class Foo[T[_, _], F[_], A, B](val fa: T[F[A], F[B]]) + +object Test { + def x[T[_, _]](tmab: T[Either[Int, String], Either[Int, Int]]) = + new Foo(tmab) +} From cf849a69937d9b1369234bfc090c306e55db1e4c Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 10 Aug 2020 09:23:13 +0200 Subject: [PATCH 4/6] Remove extraneous fourthTry If `canInstantiate` fails, we fallback to `compareLower` which itself can fallback to `fourthTry`, so there's no need to fallback to `fourhTry` inside `canInstantiate` itself. --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index eb01f7e3243c..4b764a9ccc53 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1027,8 +1027,7 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi /** `param2` can be instantiated to a type application prefix of the LHS * or to a type application prefix of one of the LHS base class instances - * and the resulting type application is a supertype of `tp1`, - * or fallback to fourthTry. + * and the resulting type application is a supertype of `tp1`. */ def canInstantiate(tycon2: TypeParamRef): Boolean = { def appOK(tp1base: Type) = tp1base match { @@ -1050,8 +1049,7 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi false } liftToBase(tp1w.baseClasses) - } || - fourthTry + } } } From 6c63f50d4d9c9db0e30049a557a8454c0f9a940e Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 10 Aug 2020 09:35:14 +0200 Subject: [PATCH 5/6] Generalize base class handling --- .../dotty/tools/dotc/core/TypeComparer.scala | 25 ++++++++----------- tests/pos/t2712-8.scala | 9 +++++++ 2 files changed, 20 insertions(+), 14 deletions(-) create mode 100644 tests/pos/t2712-8.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 4b764a9ccc53..aac7562c31d9 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1036,20 +1036,17 @@ class TypeComparer(using val comparerCtx: Context) extends ConstraintHandling wi case _ => false } - tp1.widen match { - case tp1w: AppliedType => appOK(tp1w) - case tp1w => - tp1w.typeSymbol.isClass && { - val classBounds = tycon2.classSymbols - def liftToBase(bcs: List[ClassSymbol]): Boolean = bcs match { - case bc :: bcs1 => - classBounds.exists(bc.derivesFrom) && appOK(nonExprBaseType(tp1, bc)) - || liftToBase(bcs1) - case _ => - false - } - liftToBase(tp1w.baseClasses) - } + val tp1w = tp1.widen + appOK(tp1w) || tp1w.typeSymbol.isClass && { + val classBounds = tycon2.classSymbols + def liftToBase(bcs: List[ClassSymbol]): Boolean = bcs match { + case bc :: bcs1 => + classBounds.exists(bc.derivesFrom) && appOK(nonExprBaseType(tp1, bc)) + || liftToBase(bcs1) + case _ => + false + } + liftToBase(tp1w.baseClasses) } } diff --git a/tests/pos/t2712-8.scala b/tests/pos/t2712-8.scala new file mode 100644 index 000000000000..4180c2f69da7 --- /dev/null +++ b/tests/pos/t2712-8.scala @@ -0,0 +1,9 @@ +class Two[A, B] +class One[A] extends Two[A, A] + +object Test { + def foo[F[_, _]](x: F[Int, Int]) = x + + val t: One[Int] = ??? + foo(t) +} From 64e8797a97d98499ca0fd08ca532a1b903546bae Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Mon, 10 Aug 2020 09:58:57 +0200 Subject: [PATCH 6/6] Add remaining SI-2712 testcases from Scala 2 It turns out that neg/t2712-2.scala actually works in Dotty due to better type inference. --- tests/neg/t2712-8.scala | 10 ++++++++++ tests/pos/t2712-2b.scala | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 tests/neg/t2712-8.scala create mode 100644 tests/pos/t2712-2b.scala diff --git a/tests/neg/t2712-8.scala b/tests/neg/t2712-8.scala new file mode 100644 index 000000000000..363bcf71b0a6 --- /dev/null +++ b/tests/neg/t2712-8.scala @@ -0,0 +1,10 @@ +object Test extends App { + class L[A] + class Quux0[B, CC[_]] + class Quux[C] extends Quux0[C, L] + + def foo[D[_]](x: D[D[Boolean]]) = ??? + def bar: Quux[Int] = ??? + + foo(bar) // error: Found: Test.Quux[Int] Required: D[D[Boolean]] +} diff --git a/tests/pos/t2712-2b.scala b/tests/pos/t2712-2b.scala new file mode 100644 index 000000000000..3b4084898db8 --- /dev/null +++ b/tests/pos/t2712-2b.scala @@ -0,0 +1,18 @@ +package test + +class X1 +class X2 +class X3 + +trait One[A] +trait Two[A, B] + +class Foo extends Two[X1, X2] with One[X3] +object Test { + def test1[M[_], A](x: M[A]): M[A] = x + + val foo = new Foo + + test1(foo): One[X3] // fails in Scala 2 with partial unification enabled, works in Dotty + test1(foo): Two[X1, X2] +}