Skip to content

Commit

Permalink
Fix specifity comparison for extensions in polymorphic givens
Browse files Browse the repository at this point in the history
When we compare polymorphic methods for specificity, we replace their
type parameters by type variables constrained in the current
context (see isAsSpecific), but for extension methods in polymorphic
givens, the comparison was done with a constraint set that does not
include the type parameters of the givens, this lead to ambiguity errors
as experienced in
typelevel/spotted-leopards#2.

We fix this by ensuring the TyperState we use for the comparison
contains any additional constraint specific to the TyperState of either
alternative. This required generalizing TyperState#mergeConstraintWith
to handle `this` not being committable (because in that case `this` does
not need to take ownership of the type variables owned by `that`,
therefore `that` itself is allowed to be committable).
  • Loading branch information
smarter authored and olsdavis committed Apr 4, 2022
1 parent 384d6d9 commit a5915c7
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 4 deletions.
10 changes: 7 additions & 3 deletions compiler/src/dotty/tools/dotc/core/TyperState.scala
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,13 @@ class TyperState() {

/** Integrate the constraints from `that` into this TyperState.
*
* @pre If `that` is committable, it must not contain any type variable which
* @pre If `this` and `that` are committable, `that` must not contain any type variable which
* does not exist in `this` (in other words, all its type variables must
* be owned by a common parent of `this` and `that`).
*/
def mergeConstraintWith(that: TyperState)(using Context): Unit =
def mergeConstraintWith(that: TyperState)(using Context): this.type =
if this eq that then return this

that.ensureNotConflicting(constraint)

val comparingCtx = ctx.withTyperState(this)
Expand All @@ -198,7 +200,8 @@ class TyperState() {
// Integrate the type lambdas from `other`
constraint.contains(tl) || other.isRemovable(tl) || {
val tvars = tl.paramRefs.map(other.typeVarOfParam(_)).collect { case tv: TypeVar => tv }
tvars.foreach(tvar => if !tvar.inst.exists && !isOwnedAnywhere(this, tvar) then includeVar(tvar))
if this.isCommittable then
tvars.foreach(tvar => if !tvar.inst.exists && !isOwnedAnywhere(this, tvar) then includeVar(tvar))
typeComparer.addToConstraint(tl, tvars)
}) &&
// Integrate the additional constraints on type variables from `other`
Expand All @@ -223,6 +226,7 @@ class TyperState() {

for tl <- constraint.domainLambdas do
if constraint.isRemovable(tl) then constraint = constraint.remove(tl)
this
end mergeConstraintWith

/** Take ownership of `tvar`.
Expand Down
24 changes: 23 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,29 @@ trait Implicits:
// compare the extension methods instead of their wrappers.
def stripExtension(alt: SearchSuccess) = methPart(stripApply(alt.tree)).tpe
(stripExtension(alt1), stripExtension(alt2)) match
case (ref1: TermRef, ref2: TermRef) => diff = compare(ref1, ref2)
case (ref1: TermRef, ref2: TermRef) =>
// ref1 and ref2 might refer to type variables owned by
// alt1.tstate and alt2.tstate respectively, to compare the
// alternatives correctly we need a TyperState that includes
// constraints from both sides, see
// tests/*/extension-specificity2.scala for test cases.
val constraintsIn1 = alt1.tstate.constraint ne ctx.typerState.constraint
val constraintsIn2 = alt2.tstate.constraint ne ctx.typerState.constraint
def exploreState(alt: SearchSuccess): TyperState =
alt.tstate.fresh(committable = false)
val comparisonState =
if constraintsIn1 && constraintsIn2 then
exploreState(alt1).mergeConstraintWith(alt2.tstate)
else if constraintsIn1 then
exploreState(alt1)
else if constraintsIn2 then
exploreState(alt2)
else
ctx.typerState

diff = inContext(ctx.withTyperState(comparisonState)) {
compare(ref1, ref2)
}
case _ =>
if diff < 0 then alt2
else if diff > 0 then alt1
Expand Down
10 changes: 10 additions & 0 deletions tests/neg/extension-specificity2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
trait Bla1[A]:
extension (x: A) def foo(y: A): Int
trait Bla2[A]:
extension (x: A) def foo(y: A): Int

def test =
given bla1[T <: Int]: Bla1[T] = ???
given bla2[S <: Int]: Bla2[S] = ???

1.foo(2) // error: never extension is more specific than the other
37 changes: 37 additions & 0 deletions tests/run/extension-specificity2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
trait Foo[F[_]]:
extension [A](fa: F[A])
def foo[B](fb: F[B]): Int

def test1 =
// Simplified from https://github.com/typelevel/spotted-leopards/issues/2
given listFoo: Foo[List] with
extension [A](fa: List[A])
def foo[B](fb: List[B]): Int = 1

given functionFoo[T]: Foo[[A] =>> T => A] with
extension [A](fa: T => A)
def foo[B](fb: T => B): Int = 2

val x = List(1, 2).foo(List(3, 4))
assert(x == 1, x)

def test2 =
// This test case would fail if we used `wildApprox` on the method types
// instead of using the correct typer state.
trait Bar1[A]:
extension (x: A => A) def bar(y: A): Int
trait Bar2:
extension (x: Int => 1) def bar(y: Int): Int

given bla1[T]: Bar1[T] with
extension (x: T => T) def bar(y: T): Int = 1
given bla2: Bar2 with
extension (x: Int => 1) def bar(y: Int): Int = 2

val f: Int => 1 = x => 1
val x = f.bar(1)
assert(x == 2, x)

@main def Test =
test1
test2

0 comments on commit a5915c7

Please sign in to comment.