Skip to content

Improve partial unification handling #9523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 118 additions & 51 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice explanation!

* - 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 = {
Expand All @@ -860,13 +939,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
Expand Down Expand Up @@ -926,60 +1027,26 @@ 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 = {

/** 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
}

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)
} ||
fourthTry
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)
}
}

Expand Down Expand Up @@ -1043,8 +1110,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
}
Expand Down
10 changes: 9 additions & 1 deletion compiler/test/dotty/tools/vulpix/ParallelTesting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -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 =>
Expand Down
14 changes: 14 additions & 0 deletions tests/neg/i3452.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
10 changes: 10 additions & 0 deletions tests/neg/t2712-8.scala
Original file line number Diff line number Diff line change
@@ -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]]
}
2 changes: 1 addition & 1 deletion tests/pos/anykind.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}

Expand Down
6 changes: 3 additions & 3 deletions tests/pos/i6565.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
6 changes: 6 additions & 0 deletions tests/pos/i9478.scala
Original file line number Diff line number Diff line change
@@ -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)
}
18 changes: 18 additions & 0 deletions tests/pos/t2712-2b.scala
Original file line number Diff line number Diff line change
@@ -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]
}
9 changes: 9 additions & 0 deletions tests/pos/t2712-8.scala
Original file line number Diff line number Diff line change
@@ -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)
}
23 changes: 23 additions & 0 deletions tests/run/hk-alias-unification.scala
Original file line number Diff line number Diff line change
@@ -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
}
}