Change rules for given prioritization (#19300)

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.

Author: odersky

community-build/community-projects/shapeless-3

@@ -11,6 +11,7 @@ enum SourceVersion:
   case `3.3-migration`, `3.3`
   case `3.4-migration`, `3.4`
   case `3.5-migration`, `3.5`
+  case `3.6-migration`, `3.6`
   // !!! Keep in sync with scala.runtime.stdlibPatches.language !!!
   case `future-migration`, `future`
 

compiler/src/dotty/tools/dotc/config/SourceVersion.scala

@@ -41,6 +41,8 @@ object Mode {
   val Pattern: Mode = newMode(0, "Pattern")
   val Type: Mode = newMode(1, "Type")
 
+  val PatternOrTypeBits: Mode = Pattern | Type
+
   val ImplicitsEnabled: Mode = newMode(2, "ImplicitsEnabled")
   val InferringReturnType: Mode = newMode(3, "InferringReturnType")
 
@@ -101,16 +103,19 @@ object Mode {
    */
   val CheckBoundsOrSelfType: Mode = newMode(14, "CheckBoundsOrSelfType")
 
-  /** Use Scala2 scheme for overloading and implicit resolution */
-  val OldOverloadingResolution: Mode = newMode(15, "OldOverloadingResolution")
+  /** Use previous Scheme for implicit resolution. Currently significant
+   *  in 3.0-migration where we use Scala-2's scheme instead and in 3.5-migration
+   *  where we use the previous scheme up to 3.4 instead.
+   */
+  val OldImplicitResolution: Mode = newMode(15, "OldImplicitResolution")
 
   /** Treat CapturingTypes as plain AnnotatedTypes even in phase CheckCaptures.
-   *  Reuses the value of OldOverloadingResolution to save Mode bits.
-   *  This is OK since OldOverloadingResolution only affects implicit search, which
+   *  Reuses the value of OldImplicitResolution to save Mode bits.
+   *  This is OK since OldImplicitResolution only affects implicit search, which
    *  is done during phases Typer and Inlinig, and IgnoreCaptures only has an
    *  effect during phase CheckCaptures.
    */
-  val IgnoreCaptures = OldOverloadingResolution
+  val IgnoreCaptures = OldImplicitResolution
 
   /** Allow hk applications of type lambdas to wildcard arguments;
    *  used for checking that such applications do not normally arise
@@ -120,8 +125,6 @@ object Mode {
   /** Read original positions when unpickling from TASTY */
   val ReadPositions: Mode = newMode(17, "ReadPositions")
 
-  val PatternOrTypeBits: Mode = Pattern | Type
-
   /** We are elaborating the fully qualified name of a package clause.
    *  In this case, identifiers should never be imported.
    */
@@ -133,6 +136,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/core/Mode.scala

@@ -22,7 +22,7 @@ import ProtoTypes.*
 import Inferencing.*
 import reporting.*
 import Nullables.*, NullOpsDecorator.*
-import config.Feature
+import config.{Feature, SourceVersion}
 
 import collection.mutable
 import config.Printers.{overload, typr, unapp}
@@ -1709,6 +1709,12 @@ trait Applications extends Compatibility {
   /** Compare two alternatives of an overloaded call or an implicit search.
    *
    *  @param  alt1, alt2      Non-overloaded references indicating the two choices
+   *  @param  preferGeneral   When comparing two value types, prefer the more general one
+   *                          over the more specific one iff `preferGeneral` is true.
+   *                          `preferGeneral` is set to `true` when we compare two given values, since
+   *                          then we want the most general evidence that matches the target
+   *                          type. It is set to `false` for overloading resolution, when we want the
+   *                          most specific type instead.
    *  @return  1   if 1st alternative is preferred over 2nd
    *          -1   if 2nd alternative is preferred over 1st
    *           0   if neither alternative is preferred over the other
@@ -1724,27 +1730,26 @@ trait Applications extends Compatibility {
    *  an alternative that takes more implicit parameters wins over one
    *  that takes fewer.
    */
-  def compare(alt1: TermRef, alt2: TermRef)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) {
+  def compare(alt1: TermRef, alt2: TermRef, preferGeneral: Boolean = false)(using Context): Int = trace(i"compare($alt1, $alt2)", overload) {
     record("resolveOverloaded.compare")
 
-    /** Is alternative `alt1` with type `tp1` as specific as alternative
+    /** Is alternative `alt1` with type `tp1` as good as alternative
      *  `alt2` with type `tp2` ?
      *
-     *    1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as specific as `alt2`
+     *    1. A method `alt1` of type `(p1: T1, ..., pn: Tn)U` is as good as `alt2`
      *       if `alt1` is nullary or `alt2` is applicable to arguments (p1, ..., pn) of
      *       types T1,...,Tn. If the last parameter `pn` has a vararg type T*, then
      *       `alt1` must be applicable to arbitrary numbers of `T` parameters (which
      *       implies that it must be a varargs method as well).
      *    2. A polymorphic member of type [a1 >: L1 <: U1, ..., an >: Ln <: Un]T is as
-     *       specific as `alt2` of type `tp2` if T is as specific as `tp2` under the
+     *       good as `alt2` of type `tp2` if T is as good as `tp2` under the
      *       assumption that for i = 1,...,n each ai is an abstract type name bounded
      *       from below by Li and from above by Ui.
      *    3. A member of any other type `tp1` is:
-     *       a. always as specific as a method or a polymorphic method.
-     *       b. as specific as a member of any other type `tp2` if `tp1` is compatible
-     *          with `tp2`.
+     *       a. always as good as a method or a polymorphic method.
+     *       b. as good as a member of any other type `tp2` if `asGoodValueType(tp1, tp2) = true`
      */
-    def isAsSpecific(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) {
+    def isAsGood(alt1: TermRef, tp1: Type, alt2: TermRef, tp2: Type): Boolean = trace(i"isAsSpecific $tp1 $tp2", overload) {
       tp1 match
         case tp1: MethodType => // (1)
           tp1.paramInfos.isEmpty && tp2.isInstanceOf[LambdaType]
@@ -1766,69 +1771,94 @@ trait Applications extends Compatibility {
             fullyDefinedType(tp1Params, "type parameters of alternative", alt1.symbol.srcPos)
 
             val tparams = newTypeParams(alt1.symbol, tp1.paramNames, EmptyFlags, tp1.instantiateParamInfos(_))
-            isAsSpecific(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2)
+            isAsGood(alt1, tp1.instantiate(tparams.map(_.typeRef)), alt2, tp2)
           }
         case _ => // (3)
+          def isGiven(alt: TermRef) =
+            alt1.symbol.is(Given) && alt.symbol != defn.NotGivenClass
+          def compareValues(tp1: Type, tp2: Type)(using Context) =
+            isAsGoodValueType(tp1, tp2, isGiven(alt1), isGiven(alt2))
           tp2 match
             case tp2: MethodType => true // (3a)
             case tp2: PolyType if tp2.resultType.isInstanceOf[MethodType] => true // (3a)
             case tp2: PolyType => // (3b)
-              explore(isAsSpecificValueType(tp1, instantiateWithTypeVars(tp2)))
+              explore(compareValues(tp1, instantiateWithTypeVars(tp2)))
             case _ => // 3b)
-              isAsSpecificValueType(tp1, tp2)
+              compareValues(tp1, tp2)
     }
 
-    /** Test whether value type `tp1` is as specific as value type `tp2`.
-     *  Let's abbreviate this to `tp1 <:s tp2`.
-     *  Previously, `<:s` was the same as `<:`. This behavior is still
-     *  available under mode `Mode.OldOverloadingResolution`. The new behavior
-     *  is different, however. Here, `T <:s U` iff
+    /** Test whether value type `tp1` is as good as value type `tp2`.
+     *  Let's abbreviate this to `tp1 <:p tp2`. The behavior depends on the Scala version
+     *  and mode.
      *
-     *    flip(T) <: flip(U)
+     *   - In Scala 2, `<:p` was the same as `<:`. This behavior is still
+     *     available in 3.0-migration if mode `Mode.OldImplicitResolution` is turned on as well.
+     *     It is used to highlight differences between Scala 2 and 3 behavior.
      *
-     *  where `flip` changes covariant occurrences of contravariant type parameters to
-     *  covariant ones. Intuitively `<:s` means subtyping `<:`, except that all arguments
-     *  to contravariant parameters are compared as if they were covariant. E.g. given class
+     *   - In Scala 3.0-3.5, the behavior is as follows: `T <:p U` iff there is an impliit conversion
+     *     from `T` to `U`, or
      *
-     *     class Cmp[-X]
+     *        flip(T) <: flip(U)
      *
-     *  `Cmp[T] <:s Cmp[U]` if `T <: U`. On the other hand, non-variant occurrences
-     *  of parameters are not affected. So `T <: U` would imply `Set[Cmp[U]] <:s Set[Cmp[T]]`,
-     *  as usual, because `Set` is non-variant.
+     *     where `flip` changes covariant occurrences of contravariant type parameters to
+     *     covariant ones. Intuitively `<:p` means subtyping `<:`, except that all arguments
+     *     to contravariant parameters are compared as if they were covariant. E.g. given class
      *
-     *  This relation might seem strange, but it models closely what happens for methods.
-     *  Indeed, if we integrate the existing rules for methods into `<:s` we have now that
+     *         class Cmp[-X]
      *
-     *     (T)R  <:s  (U)R
+     *     `Cmp[T] <:p Cmp[U]` if `T <: U`. On the other hand, non-variant occurrences
+     *     of parameters are not affected. So `T <: U` would imply `Set[Cmp[U]] <:p Set[Cmp[T]]`,
+     *     as usual, because `Set` is non-variant.
      *
-     *  iff
+     *   - From Scala 3.6, `T <:p U` means `T <: U` or `T` convertible to `U`
+     *     for overloading resolution (when `preferGeneral is false), and the opposite relation
+     *     `U <: T` or `U convertible to `T` for implicit disambiguation between givens
+     *     (when `preferGeneral` is true). For old-style implicit values, the 3.4 behavior is kept.
+     *     If one of the alternatives is a given and the other is an implicit, the given wins.
      *
-     *     T => R  <:s  U => R
+     *   - In Scala 3.5 and Scala 3.6-migration, we issue a warning if the result under
+     *     Scala 3.6 differ wrt to the old behavior up to 3.5.
      *
-     *  Also: If a compared type refers to a given or its module class, use
+     *  Also and only for given resolution: If a compared type refers to a given or its module class, use
      *  the intersection of its parent classes instead.
      */
-    def isAsSpecificValueType(tp1: Type, tp2: Type)(using Context) =
-      if (ctx.mode.is(Mode.OldOverloadingResolution))
+    def isAsGoodValueType(tp1: Type, tp2: Type, alt1isGiven: Boolean, alt2isGiven: Boolean)(using Context): Boolean =
+      val oldResolution = ctx.mode.is(Mode.OldImplicitResolution)
+      if !preferGeneral || Feature.migrateTo3 && oldResolution then
+        // Normal specificity test for overloading resolution (where `preferGeneral` is false)
+        // and in mode Scala3-migration when we compare with the old Scala 2 rules.
         isCompatible(tp1, tp2)
-      else {
-        val flip = new TypeMap {
-          def apply(t: Type) = t match {
-            case t @ AppliedType(tycon, args) =>
-              def mapArg(arg: Type, tparam: TypeParamInfo) =
-                if (variance > 0 && tparam.paramVarianceSign < 0) defn.FunctionNOf(arg :: Nil, defn.UnitType)
-                else arg
-              mapOver(t.derivedAppliedType(tycon, args.zipWithConserve(tycon.typeParams)(mapArg)))
-            case _ => mapOver(t)
-          }
-        }
-        def prepare(tp: Type) = tp.stripTypeVar match {
+      else
+        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 Feature.sourceVersion.isAtMost(SourceVersion.`3.4`)
+            || oldResolution
+            || !alt1isGiven && !alt2isGiven
+        then
+          // Intermediate rules: better means specialize, but map all type arguments downwards
+          // These are enabled for 3.0-3.5, and for all comparisons between old-style implicits,
+          // and in 3.5 amd 3.6-migration when we compare with previous rules.
+          val flip = new TypeMap:
+            def apply(t: Type) = t match
+              case t @ AppliedType(tycon, args) =>
+                def mapArg(arg: Type, tparam: TypeParamInfo) =
+                  if (variance > 0 && tparam.paramVarianceSign < 0) defn.FunctionNOf(arg :: Nil, defn.UnitType)
+                  else arg
+                mapOver(t.derivedAppliedType(tycon, args.zipWithConserve(tycon.typeParams)(mapArg)))
+              case _ => mapOver(t)
+          (flip(tp1p) relaxed_<:< flip(tp2p)) || viewExists(tp1, tp2)
+        else
+          // New rules: better means generalize, givens always beat implicits
+          if alt1isGiven != alt2isGiven then alt1isGiven
+          else (tp2p relaxed_<:< tp1p) || viewExists(tp2, tp1)
+    end isAsGoodValueType
 
     /** Widen the result type of synthetic given methods from the implementation class to the
      *  type that's implemented. Example
@@ -1880,17 +1910,16 @@ trait Applications extends Compatibility {
     def comparePrefixes =
       val pre1 = widenPrefix(alt1)
       val pre2 = widenPrefix(alt2)
-      val winsPrefix1 = isAsSpecificValueType(pre1, pre2)
-      val winsPrefix2 = isAsSpecificValueType(pre2, pre1)
+      val winsPrefix1 = isCompatible(pre1, pre2)
+      val winsPrefix2 = isCompatible(pre2, pre1)
       if winsPrefix1 == winsPrefix2 then 0
       else if winsPrefix1 then 1
       else -1
 
     def compareWithTypes(tp1: Type, tp2: Type) =
       val ownerScore = compareOwner(alt1.symbol.maybeOwner, alt2.symbol.maybeOwner)
-
-      val winsType1 = isAsSpecific(alt1, tp1, alt2, tp2)
-      val winsType2 = isAsSpecific(alt2, tp2, alt1, tp1)
+      val winsType1 = isAsGood(alt1, tp1, alt2, tp2)
+      val winsType2 = isAsGood(alt2, tp2, alt1, tp1)
 
       overload.println(i"compare($alt1, $alt2)? $tp1 $tp2 $ownerScore $winsType1 $winsType2")
       if winsType1 && winsType2

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

@@ -531,7 +531,7 @@ object Implicits:
          |must be more specific than $target""" :: Nil
 
     override def msg(using Context) =
-      super.msg.append(i"\nThe expected type $target is not specific enough, so no search was attempted")
+      super.msg.append("\nThe expected type $target is not specific enough, so no search was attempted")
 
     override def toString = s"TooUnspecific"
   end TooUnspecific
@@ -1110,8 +1110,8 @@ trait Implicits:
           case result: SearchFailure if result.isAmbiguous =>
             val deepPt = pt.deepenProto
             if (deepPt ne pt) inferImplicit(deepPt, argument, span)
-            else if (migrateTo3 && !ctx.mode.is(Mode.OldOverloadingResolution))
-              withMode(Mode.OldOverloadingResolution)(inferImplicit(pt, argument, span)) match {
+            else if (migrateTo3 && !ctx.mode.is(Mode.OldImplicitResolution))
+              withMode(Mode.OldImplicitResolution)(inferImplicit(pt, argument, span)) match {
                 case altResult: SearchSuccess =>
                   report.migrationWarning(
                     result.reason.msg
@@ -1226,7 +1226,7 @@ trait Implicits:
     assert(argument.isEmpty || argument.tpe.isValueType || argument.tpe.isInstanceOf[ExprType],
         em"found: $argument: ${argument.tpe}, expected: $pt")
 
-    private def nestedContext() =
+    private def searchContext() =
       ctx.fresh.setMode(ctx.mode &~ Mode.ImplicitsEnabled)
 
     private def isCoherent = pt.isRef(defn.CanEqualClass)
@@ -1270,7 +1270,7 @@ trait Implicits:
       else
         val history = ctx.searchHistory.nest(cand, pt)
         val typingCtx =
-          nestedContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history)
+          searchContext().setNewTyperState().setFreshGADTBounds.setSearchHistory(history)
         val result = typedImplicit(cand, pt, argument, span)(using typingCtx)
         result match
           case res: SearchSuccess =>
@@ -1293,11 +1293,44 @@ trait Implicits:
        *  @return  a number > 0   if `alt1` is preferred over `alt2`
        *           a number < 0   if `alt2` is preferred over `alt1`
        *           0              if neither alternative is preferred over the other
+       *  The behavior depends on the source version
+       *      before 3.5: compare with preferGeneral = false
+       *             3.5: compare twice with preferGeneral = false and true, warning if result is different,
+       *                  return old result with preferGeneral = false
+       *   3.6-migration: compare twice with preferGeneral = false and true, warning if result is different,
+       *                  return new result with preferGeneral = true
+       *  3.6 and higher: compare with preferGeneral = true
+       *
        */
       def compareAlternatives(alt1: RefAndLevel, alt2: RefAndLevel): Int =
+        def comp(using Context) = explore(compare(alt1.ref, alt2.ref, preferGeneral = true))
         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))(using nestedContext())
+        else
+          var cmp = comp(using searchContext())
+          val sv = Feature.sourceVersion
+          if sv == SourceVersion.`3.5` || sv == SourceVersion.`3.6-migration` then
+            val prev = comp(using searchContext().addMode(Mode.OldImplicitResolution))
+            if cmp != prev then
+              def choice(c: Int) = c match
+                case -1 => "the second alternative"
+                case  1 => "the first alternative"
+                case _  => "none - it's ambiguous"
+              if sv == SourceVersion.`3.5` then
+                report.warning(
+                  em"""Given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref} will change
+                      |Current choice           : ${choice(prev)}
+                      |New choice from Scala 3.6: ${choice(cmp)}""", srcPos)
+                prev
+              else
+                report.warning(
+                  em"""Change in given search preference for $pt between alternatives ${alt1.ref} and ${alt2.ref}
+                      |Previous choice          : ${choice(prev)}
+                      |New choice from Scala 3.6: ${choice(cmp)}""", srcPos)
+                cmp
+            else cmp
+          else cmp
+      end compareAlternatives
 
       /** If `alt1` is also a search success, try to disambiguate as follows:
        *    - If alt2 is preferred over alt1, pick alt2, otherwise return an
@@ -1307,7 +1340,9 @@ trait Implicits:
         case alt1: SearchSuccess =>
           var diff = compareAlternatives(alt1, alt2)
           assert(diff <= 0)   // diff > 0 candidates should already have been eliminated in `rank`
-          if diff == 0 && alt2.isExtension then
+          if diff == 0 && alt1.ref =:= alt2.ref then
+            diff = 1 // See i12951 for a test where this happens
+          else if diff == 0 && alt2.isExtension then
             if alt1.isExtension then
               // Fall back: if both results are extension method applications,
               // compare the extension methods instead of their wrappers.
@@ -1333,8 +1368,8 @@ trait Implicits:
                     else
                       ctx.typerState
 
-                  diff = inContext(ctx.withTyperState(comparisonState)):
-                    compare(ref1, ref2)
+                  diff = inContext(searchContext().withTyperState(comparisonState)):
+                    compare(ref1, ref2, preferGeneral = true)
             else // alt1 is a conversion, prefer extension alt2 over it
               diff = -1
           if diff < 0 then alt2