Skip to content

Fix #9058: Make implicit search preference relation transitive #9065

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 2 commits into from
May 28, 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
49 changes: 43 additions & 6 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1347,16 +1347,53 @@ trait Applications extends Compatibility {
* @return 1 if `sym1` properly derives from `sym2`
* -1 if `sym2` properly derives from `sym1`
* 0 otherwise
* Module classes also inherit the relationship from their companions.
* Module classes also inherit the relationship from their companions. This means,
* if no direct derivation exists between `sym1` and `sym2` also perform the following
* tests:
* - If both sym1 and sym1 are module classes that have companion classes, compare the companions
* - If sym1 is a module class with a companion, and sym2 is a normal class or trait, compare
* the companion with sym2.
*
* Note that this makes `compareOwner(_, _) > 0` not transitive! For instance:
*
* class A extends B
* object A
* class B
* object B extends C
*
* Then compareOwner(A$, B$) = 1 and compareOwner(B$, C) == 1, but
* compareOwner(A$, C) == 0.
*/
def compareOwner(sym1: Symbol, sym2: Symbol)(using Context): Int =
if (sym1 == sym2) 0
else if (sym1 isSubClass sym2) 1
else if (sym2 isSubClass sym1) -1
else if (sym2.is(Module)) compareOwner(sym1, sym2.companionClass)
else if (sym1.is(Module)) compareOwner(sym1.companionClass, sym2)
if sym1 == sym2 then 0
else if sym1.isSubClass(sym2) then 1
else if sym2.isSubClass(sym1) then -1
else if sym1.is(Module) then
compareOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
else 0

/** A version of `compareOwner` that is transitive, to be used in sorting
* It would be nice if we could use this as the general method for comparing
* owners, but unfortunately this does not compile all existsing code.
* An example is `enrich-gentraversable.scala`. Here we have
*
* trait BuildFrom...
* object BuildFrom extends BuildFromLowPriority1
*
* and we need to pick an implicit in BuildFrom over BuildFromLowPriority1
* the rules in `compareOwner` give us that, but the rules in `isSubOwner`
* don't.
* So we need to stick with `compareOwner` for backwards compatibility, even
* though it is arguably broken. We can still use `isSubOwner` for sorting
* since that is just a performance optimization, so if the two methods
* don't agree sometimes that's OK.
*/
def isSubOwner(sym1: Symbol, sym2: Symbol)(using Context): Boolean =
if sym1.is(Module) && sym1.companionClass.exists then
isSubOwner(sym1.companionClass, if sym2.is(Module) then sym2.companionClass else sym2)
else
sym1 != sym2 && sym1.isSubClass(sym2)

/** Compare to alternatives of an overloaded call or an implicit search.
*
* @param alt1, alt2 Non-overloaded references indicating the two choices
Expand Down
19 changes: 14 additions & 5 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1169,14 +1169,11 @@ trait Implicits { self: Typer =>
if (level1 < level2) return false
val sym1 = cand1.ref.symbol
val sym2 = cand2.ref.symbol
val ownerScore = compareOwner(sym1.maybeOwner, sym2.maybeOwner)
if (ownerScore > 0) return true
if (ownerScore < 0) return false
val arity1 = sym1.info.firstParamTypes.length
val arity2 = sym2.info.firstParamTypes.length
if (arity1 < arity2) return true
if (arity1 > arity2) return false
false
isSubOwner(sym1, sym2)
}

/** Sort list of implicit references according to `prefer`.
Expand All @@ -1190,7 +1187,19 @@ trait Implicits { self: Typer =>
if (prefer(e2, e1)) e2 :: e1 :: Nil
else eligible
case _ =>
eligible.sortWith(prefer)
try eligible.sortWith(prefer)
catch case ex: IllegalArgumentException =>
// diagnostic to see what went wrong
for
e1 <- eligible
e2 <- eligible
if prefer(e1, e2)
e3 <- eligible
if prefer(e2, e3) && !prefer(e1, e3)
do
val es = List(e1, e2, e3)
println(i"transitivity violated for $es%, %\n ${es.map(_.implicitRef.underlyingRef.symbol.showLocated)}%, %")
throw ex
}

rank(sort(eligible), NoMatchingImplicitsFailure, Nil)
Expand Down