Change rules for given prioritization

Consider the following program:

```scala
class A
class B extends A
class C extends A

given A = A()
given B = B()
given C = C()

def f(using a: A, b: B, c: C) =
  println(a.getClass)
  println(b.getClass)
  println(c.getClass)

@main def Test = f
```
With the current rules, this would fail with an ambiguity error between B and C when
trying to synthesize the A parameter. This is a problem without an easy remedy.

We can fix this problem by flipping the priority for implicit arguments. Instead of
requiring an argument to be most _specific_, we now require it to be most _general_
while still conforming to the formal parameter.

There are three justifications for this change, which at first glance seems quite drastic:

 - It gives us a natural way to deal with inheritance triangles like the one in the code above.
   Such triangles are quite common.
 - Intuitively, we want to get the closest possible match between required formal parameter type and
   synthetisized argument. The "most general" rule provides that.
 - We already do a crucial part of this. Namely, with current rules we interpolate all
   type variables in an implicit argument downwards, no matter what their variance is.
   This makes no sense in theory, but solves hairy problems with contravariant typeclasses
   like `Comparable`. Instead of this hack, we now do something more principled, by
   flipping the direction everywhere, preferring general over specific, instead of just
   flipping contravariant type parameters.

# Conflicts:
#	compiler/src/dotty/tools/dotc/typer/Implicits.scala

Author: odersky

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

@@ -133,6 +133,8 @@ object Mode {
   /** We are typing the body of an inline method */
   val InlineableBody: Mode = newMode(21, "InlineableBody")
 
+  val NewGivenRules: Mode = newMode(22, "NewGivenRules")
+
   /** We are synthesizing the receiver of an extension method */
   val SynthesizeExtMethodReceiver: Mode = newMode(23, "SynthesizeExtMethodReceiver")
 

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

@@ -1640,6 +1640,9 @@ trait Applications extends Compatibility {
   def compare(alt1: TermRef, alt2: TermRef, preferGeneral: Boolean = false)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) {
     record("resolveOverloaded.compare")
 
+    val newGivenRules =
+      ctx.mode.is(Mode.NewGivenRules) && alt1.symbol.is(Given)
+
     /** Is alternative `alt1` with type `tp1` as specific as alternative
      *  `alt2` with type `tp2` ?
      *
@@ -1726,7 +1729,7 @@ trait Applications extends Compatibility {
         // Normal specificity test for overloading resultion (where `preferGeneral` is false)
         // and in mode Scala3-migration when we compare with the old Scala 2 rules.
         isCompatible(tp1, tp2)
-      else {
+      else
         val flip = new TypeMap {
           def apply(t: Type) = t match {
             case t @ AppliedType(tycon, args) =>
@@ -1737,13 +1740,20 @@ trait Applications extends Compatibility {
             case _ => mapOver(t)
           }
         }
-        def prepare(tp: Type) = tp.stripTypeVar match {
+
+        def prepare(tp: Type) = tp.stripTypeVar match
           case tp: NamedType if tp.symbol.is(Module) && tp.symbol.sourceModule.is(Given) =>
-            flip(tp.widen.widenToParents)
-          case _ => flip(tp)
-        }
-        (prepare(tp1) relaxed_<:< prepare(tp2)) || viewExists(tp1, tp2)
-      }
+            tp.widen.widenToParents
+          case _ =>
+            tp
+
+        val tp1p = prepare(tp1)
+        val tp2p = prepare(tp2)
+        if newGivenRules then
+          (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1)
+        else
+          (flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2)
+    end isAsSpecificValueType
 
     /** Widen the result type of synthetic given methods from the implementation class to the
      *  type that's implemented. Example
@@ -1776,7 +1786,7 @@ trait Applications extends Compatibility {
 
     def compareWithTypes(tp1: Type, tp2: Type) = {
       val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
-      def winsType1 = isAsSpecific(alt1, tp1, alt2, tp2)
+      val winsType1 = isAsSpecific(alt1, tp1, alt2, tp2)
       def winsType2 = isAsSpecific(alt2, tp2, alt1, tp1)
 
       overload.println(i"compare($alt1, $alt2)? $tp1 $tp2 $ownerScore $winsType1 $winsType2")

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

@@ -1291,7 +1291,12 @@ trait Implicits:
       def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int =
         if alt1.ref eq alt2.ref then 0
         else if alt1.level != alt2.level then alt1.level - alt2.level
-        else explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext())
+        else
+          val was = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext())
+          val now = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))(using searchContext().addMode(Mode.NewGivenRules))
+          if was != now then
+            println(i"change in preference for $pt between ${alt1.ref} and ${alt2.ref}, was: $was, now: $now at $srcPos")
+          now
 
       /** If `alt1` is also a search success, try to disambiguate as follows:
        *    - If alt2 is preferred over alt1, pick alt2, otherwise return an

tests/neg/i15264.scala

@@ -0,0 +1,58 @@
+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 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]]
+
+object test2:
+    import repro.*
+    import repro.qcontext.given
+
+    // This one will fail as ambiguous - prios aren't having an effect.
+    // Priorities indeed don't have an effect if the result is already decided
+    // without using clauses, they onyl act as a tie breaker.
+    // With the new resolution rules, it's ambiguous since we pick `gaq` for
+    // summon, and that needs an A[Int], but there are only the two competing choices
+    // qb and qc.
+    val a = summon[A[Q[Int]]] // error: ambiguous between qb and qc for A[Int]

tests/pos/i15264.scala

@@ -30,6 +30,7 @@ object repro:
     // 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]
 
@@ -45,9 +46,9 @@ object test1:
     // these will work
     val a = summon[A[Int]]
 
+
 object test2:
     import repro.*
     import repro.qcontext.given
 
-    // this one will fail as ambiguous - prios aren't having an effect
-    val a = summon[A[Q[Int]]]
+    val a = summon[A[Q[Int]]]

tests/run/given-triangle.check

@@ -0,0 +1,3 @@
+class A
+class B
+class C

tests/run/given-triangle.scala

@@ -0,0 +1,14 @@
+class A
+class B extends A
+class C extends A
+
+given A = A()
+given B = B()
+given C = C()
+
+def f(using a: A, b: B, c: C) =
+  println(a.getClass)
+  println(b.getClass)
+  println(c.getClass)
+
+@main def Test = f

tests/run/implicit-specifity.scala

@@ -38,5 +38,5 @@ object Test extends App {
   assert(Show[Int] == 0)
   assert(Show[String] == 1)
   assert(Show[Generic] == 1)   // showGen loses against fallback due to longer argument list
-  assert(Show[Generic2] == 2)  // ... but the opaque type intersection trick works.
+  assert(Show[Generic2] == 1)  // ... and the opaque type intersection trick no longer works with new resolution rules.
 }

tests/run/implied-for.scala

@@ -20,7 +20,7 @@ object Test extends App {
   val x2: T = t
   val x3: D[Int] = d
 
-  assert(summon[T].isInstanceOf[B])
+  assert(summon[T].isInstanceOf[T])
   assert(summon[D[Int]].isInstanceOf[D[_]])
 }
 

tests/run/implied-priority.scala

@@ -72,16 +72,16 @@ def test2a = {
 }
 
 /* If that solution is not applicable, we can define an override by refining the
- * result type of the given instance, e.g. like this:
+ * result type of all lower-priority instances, e.g. like this:
  */
 object Impl3 {
-  given t1[T]: E[T]("low")
+  trait LowPriority // A marker trait to indicate a lower priority
+  given t1[T]: E[T]("low") with LowPriority
 }
 
 object Override {
-  trait HighestPriority  // A marker trait to indicate a higher priority
 
-  given over[T]: E[T]("hi") with HighestPriority()
+  given over[T]: E[T]("hi") with {}
 }
 
 def test3 = {
@@ -90,7 +90,7 @@ def test3 = {
 
   { import Override.given
     import Impl3.given
-    assert(summon[E[String]].str == "hi") // `over` takes priority since its result type is a subtype of t1's.
+    assert(summon[E[String]].str == "hi", summon[E[String]].str) // `Impl3` takes priority since its result type is a subtype of t1's.
   }
 }