Skip to content

Commit 333531a

Browse files
committed
Split owner-based comparisons for implicit search into two
It seems there is no way to compare owners for implicit disambiguation that is both transitive and that compiles most existing code. So the fix is to use two similar comparison methods: One for disambiguation and the other for pre-sorting.
1 parent 8524295 commit 333531a

File tree

2 files changed

+38
-8
lines changed

2 files changed

+38
-8
lines changed

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

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1347,20 +1347,53 @@ trait Applications extends Compatibility {
13471347
* @return 1 if `sym1` properly derives from `sym2`
13481348
* -1 if `sym2` properly derives from `sym1`
13491349
* 0 otherwise
1350-
* Module classes inherit the relationship from their companions. This means:
1350+
* Module classes also inherit the relationship from their companions. This means,
1351+
* if no direct derivation exists between `sym1` and `sym2` also perform the following
1352+
* tests:
13511353
* - If both sym1 and sym1 are module classes that have companion classes, compare the companions
13521354
* - If sym1 is a module class with a companion, and sym2 is a normal class or trait, compare
13531355
* the companion with sym2.
1354-
* Going beyond that risks making compareOwner non-transitive.
1356+
*
1357+
* Note that this makes `compareOwner(_, _) > 0` not transitive! For instance:
1358+
*
1359+
* class A extends B
1360+
* object A
1361+
* class B
1362+
* object B extends C
1363+
*
1364+
* Then compareOwner(A$, B$) = 1 and compareOwner(B$, C) == 1, but
1365+
* compareOwner(A$, C) == 0.
13551366
*/
13561367
def compareOwner(sym1: Symbol, sym2: Symbol)(using Context): Int =
13571368
if sym1 == sym2 then 0
1358-
else if sym1.is(Module) && sym1.companionClass.exists then
1359-
compareOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
13601369
else if sym1.isSubClass(sym2) then 1
13611370
else if sym2.isSubClass(sym1) then -1
1371+
else if sym1.is(Module) then
1372+
compareOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
13621373
else 0
13631374

1375+
/** A version of `compareOwner` that is transitive, to be used in sorting
1376+
* It would be nice if we could use this as the general method for comparing
1377+
* owners, but unfortunately this does not compile all existsing code.
1378+
* An example is `enrich-gentraversable.scala`. Here we have
1379+
*
1380+
* trait BuildFrom...
1381+
* object BuildFrom extends BuildFromLowPriority1
1382+
*
1383+
* and we need to pick an implicit in BuildFrom over BuildFromLowPriority1
1384+
* the rules in `compareOwner` give us that, but the rules in `isSubOwner`
1385+
* don't.
1386+
* So we need to stick with `compareOwner` for backwards compatibility, even
1387+
* though it is arguably broken. We can still use `isSubOwner` for sorting
1388+
* since that is just a performance optimization, so if the two methods
1389+
* don't agree sometimes that's OK.
1390+
*/
1391+
def isSubOwner(sym1: Symbol, sym2: Symbol)(using Context): Boolean =
1392+
if sym1.is(Module) && sym1.companionClass.exists then
1393+
isSubOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
1394+
else
1395+
sym1 != sym2 && sym1.isSubClass(sym2)
1396+
13641397
/** Compare to alternatives of an overloaded call or an implicit search.
13651398
*
13661399
* @param alt1, alt2 Non-overloaded references indicating the two choices

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

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1173,10 +1173,7 @@ trait Implicits { self: Typer =>
11731173
val arity2 = sym2.info.firstParamTypes.length
11741174
if (arity1 < arity2) return true
11751175
if (arity1 > arity2) return false
1176-
val ownerScore = compareOwner(sym1.maybeOwner, sym2.maybeOwner)
1177-
if (ownerScore > 0) return true
1178-
if (ownerScore < 0) return false
1179-
false
1176+
isSubOwner(sym1, sym2)
11801177
}
11811178

11821179
/** Sort list of implicit references according to `prefer`.

0 commit comments

Comments
 (0)