Fix ranking logic

Author: odersky

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

@@ -1369,8 +1369,13 @@ trait Implicits:
                       |New choice from Scala 3.7: ${choice(cmp)}""")
                 cmp
             else cmp max prev
-              // When ranking, we keep the better of cmp and prev, which ends up retaining a candidate
-              // if it is retained in either version.
+              // When ranking, alt1 is always the new candidate and alt2 is the
+              // solution found previously. We keep the candidate if the outcome is 0
+              // (ambiguous) or 1 (first wins). Or, when ranking in healImplicit we keep the
+              // candidate only if the outcome is 1. In both cases, keeping the better
+              // of `cmp` and `prev` means we keep candidates that could match
+              // in either scheme. This means that subsequent disambiguation
+              // comparisons will record a warning if cmp != prev.
           else cmp
       end compareAlternatives
 
@@ -1416,7 +1421,15 @@ trait Implicits:
           if diff < 0 then alt2
           else if diff > 0 then alt1
           else SearchFailure(new AmbiguousImplicits(alt1, alt2, pt, argument), span)
-        case _: SearchFailure => alt2
+        case fail: SearchFailure =>
+          fail.reason match
+            case ambi: AmbiguousImplicits =>
+              if compareAlternatives(ambi.alt1, alt2) < 0 &&
+                compareAlternatives(ambi.alt2, alt2) < 0
+              then alt2
+              else alt1
+            case _ =>
+              alt2
 
       /** Try to find a best matching implicit term among all the candidates in `pending`.
        *  @param pending   The list of candidates that remain to be tested
@@ -1621,7 +1634,7 @@ trait Implicits:
             throw ex
 
       val sorted = sort(eligible)
-      val result = sorted match
+      val res = sorted match
         case first :: rest =>
           val firstIsImplicit = first.ref.symbol.is(Implicit)
           if rest.exists(_.ref.symbol.is(Implicit) != firstIsImplicit) then
@@ -1638,11 +1651,11 @@ trait Implicits:
 
       // Issue all priority change warnings that can affect the result
       val shownWarnings = priorityChangeWarnings.toList.collect:
-        case (critical, msg) if result.found.exists(critical.contains(_)) =>
+        case (critical, msg) if res.found.exists(critical.contains(_)) =>
           msg
-      result match
-        case result: SearchFailure =>
-          result.reason match
+      res match
+        case res: SearchFailure =>
+          res.reason match
             case ambi: AmbiguousImplicits =>
               // Make warnings part of error message because otherwise they are suppressed when
               // the error is emitted.
@@ -1652,7 +1665,7 @@ trait Implicits:
       for msg <- shownWarnings do
         report.warning(msg, srcPos)
 
-      result
+      res
     end searchImplicit
 
     def isUnderSpecifiedArgument(tp: Type): Boolean =

tests/pos/i15264.scala

@@ -1,3 +1,4 @@
+import language.`3.7`
 object priority:
     // lower number = higher priority
     class Prio0 extends Prio1

tests/warn/i15264.scala

@@ -0,0 +1,56 @@
+// Note: No check file for this test since the precise warning messages are non-deterministic
+import language.`3.7-migration`
+object priority:
+    // lower number = higher priority
+    class Prio0 extends Prio1
+    object Prio0 { given Prio0() }
+
+    class Prio1 extends Prio2
+    object Prio1 { given Prio1() }
+
+    class Prio2
+    object Prio2 { given Prio2() }
+
+object repro:
+    // analogous to cats Eq, Hash, Order:
+    class A[V]
+    class B[V] extends A[V]
+    class C[V] extends A[V]
+
+    class Q[V]
+
+    object context:
+        // prios work here, which is cool
+        given[V](using priority.Prio0): C[V] = new C[V]
+        given[V](using priority.Prio1): B[V] = new B[V]
+        given[V](using priority.Prio2): A[V] = new A[V]
+
+    object exports:
+        // so will these exports
+        export context.given
+
+    // if you import these don't import from 'context' above
+    object qcontext:
+        // base defs, like what you would get from cats
+        given ga: A[Int] = new B[Int] // added so that we don't get an ambiguity in test2
+        given gb: B[Int] = new B[Int]
+        given gc: C[Int] = new C[Int]
+
+        // these seem like they should work but don't
+        given gcq[V](using p0: priority.Prio0)(using c: C[V]): C[Q[V]] = new C[Q[V]]
+        given gbq[V](using p1: priority.Prio1)(using b: B[V]): B[Q[V]] = new B[Q[V]]
+        given gaq[V](using p2: priority.Prio2)(using a: A[V]): A[Q[V]] = new A[Q[V]]
+
+object test1:
+    import repro.*
+    import repro.exports.given
+
+    // these will work
+    val a = summon[A[Int]] // warn
+
+
+object test2:
+    import repro.*
+    import repro.qcontext.given
+
+    val a = summon[A[Q[Int]]] // warn