Merge pull request #9065 from dotty-staging/fix-#9058b

Fix #9058: Make implicit search preference relation transitive

Author: anatoliykmetyuk

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

@@ -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

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

@@ -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`.
@@ -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)