Skip to content

Commit 3bcd253

Browse files
committed
Fix specifity comparison for extensions in polymorphic givens
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).
1 parent be1b6a1 commit 3bcd253

File tree

4 files changed

+77
-4
lines changed

4 files changed

+77
-4
lines changed

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,13 @@ class TyperState() {
183183

184184
/** Integrate the constraints from `that` into this TyperState.
185185
*
186-
* @pre If `that` is committable, it must not contain any type variable which
186+
* @pre If `this` and `that` are committable, `that` must not contain any type variable which
187187
* does not exist in `this` (in other words, all its type variables must
188188
* be owned by a common parent of `this` and `that`).
189189
*/
190-
def mergeConstraintWith(that: TyperState)(using Context): Unit =
190+
def mergeConstraintWith(that: TyperState)(using Context): this.type =
191+
if this eq that then return this
192+
191193
that.ensureNotConflicting(constraint)
192194

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

224227
for tl <- constraint.domainLambdas do
225228
if constraint.isRemovable(tl) then constraint = constraint.remove(tl)
229+
this
226230
end mergeConstraintWith
227231

228232
/** Take ownership of `tvar`.

compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,29 @@ trait Implicits:
11771177
// compare the extension methods instead of their wrappers.
11781178
def stripExtension(alt: SearchSuccess) = methPart(stripApply(alt.tree)).tpe
11791179
(stripExtension(alt1), stripExtension(alt2)) match
1180-
case (ref1: TermRef, ref2: TermRef) => diff = compare(ref1, ref2)
1180+
case (ref1: TermRef, ref2: TermRef) =>
1181+
// ref1 and ref2 might refer to type variables owned by
1182+
// alt1.tstate and alt2.tstate respectively, to compare the
1183+
// alternatives correctly we need a TyperState that includes
1184+
// constraints from both sides, see
1185+
// tests/*/extension-specificity2.scala for test cases.
1186+
val constraintsIn1 = alt1.tstate.constraint ne ctx.typerState.constraint
1187+
val constraintsIn2 = alt2.tstate.constraint ne ctx.typerState.constraint
1188+
def exploreState(alt: SearchSuccess): TyperState =
1189+
alt.tstate.fresh(committable = false)
1190+
val comparisonState =
1191+
if constraintsIn1 && constraintsIn2 then
1192+
exploreState(alt1).mergeConstraintWith(alt2.tstate)
1193+
else if constraintsIn1 then
1194+
exploreState(alt1)
1195+
else if constraintsIn2 then
1196+
exploreState(alt2)
1197+
else
1198+
ctx.typerState
1199+
1200+
diff = inContext(ctx.withTyperState(comparisonState)) {
1201+
compare(ref1, ref2)
1202+
}
11811203
case _ =>
11821204
if diff < 0 then alt2
11831205
else if diff > 0 then alt1
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
trait Bla1[A]:
2+
extension (x: A) def foo(y: A): Int
3+
trait Bla2[A]:
4+
extension (x: A) def foo(y: A): Int
5+
6+
def test =
7+
given bla1[T <: Int]: Bla1[T] = ???
8+
given bla2[S <: Int]: Bla2[S] = ???
9+
10+
1.foo(2) // error: never extension is more specific than the other
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
trait Foo[F[_]]:
2+
extension [A](fa: F[A])
3+
def foo[B](fb: F[B]): Int
4+
5+
def test1 =
6+
// Simplified from https://github.com/typelevel/spotted-leopards/issues/2
7+
given listFoo: Foo[List] with
8+
extension [A](fa: List[A])
9+
def foo[B](fb: List[B]): Int = 1
10+
11+
given functionFoo[T]: Foo[[A] =>> T => A] with
12+
extension [A](fa: T => A)
13+
def foo[B](fb: T => B): Int = 2
14+
15+
val x = List(1, 2).foo(List(3, 4))
16+
assert(x == 1, x)
17+
18+
def test2 =
19+
// This test case would fail if we used `wildApprox` on the method types
20+
// instead of using the correct typer state.
21+
trait Bar1[A]:
22+
extension (x: A => A) def bar(y: A): Int
23+
trait Bar2:
24+
extension (x: Int => 1) def bar(y: Int): Int
25+
26+
given bla1[T]: Bar1[T] with
27+
extension (x: T => T) def bar(y: T): Int = 1
28+
given bla2: Bar2 with
29+
extension (x: Int => 1) def bar(y: Int): Int = 2
30+
31+
val f: Int => 1 = x => 1
32+
val x = f.bar(1)
33+
assert(x == 2, x)
34+
35+
@main def Test =
36+
test1
37+
test2

0 commit comments

Comments
 (0)