Add overloading priority change warnings and documentation

Author: EugeneFlesselle

compiler/src/dotty/tools/dotc/ast/Trees.scala

@@ -1908,7 +1908,7 @@ object Trees {
               case MethodTpe(_, _, x: MethodType) => !x.isImplicitMethod
               case _ => true
             }}
-          val alternatives = ctx.typer.resolveOverloaded()(allAlts, proto, receiver.srcPos)
+          val alternatives = ctx.typer.resolveOverloaded(allAlts, proto, receiver.srcPos)
           assert(alternatives.size == 1,
             i"${if (alternatives.isEmpty) "no" else "multiple"} overloads available for " +
               i"$method on ${receiver.tpe.widenDealiasKeepAnnots} with targs: $targs%, %; args: $args%, %; expectedType: $expectedType." +

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

@@ -2112,15 +2112,71 @@ trait Applications extends Compatibility {
     }
   }
 
+  /** Overloadoing Priority Schemes, changing in `scala-3.7`.
+   *
+   *  Old Scheme, until 3.7:
+   *  - First, determines the _candidates_ (subset of the alternatives) by:
+   *    + looking at the first paramater clause
+   *  - Second, determine the best candidate by:
+   *    + doing narrowMostSpecific on the first argument list;
+   *    + if still ambigous and there is another parameter clause, then
+   *      restarts narrowMostSpecific by mapping the problem onto the next parameter clause,
+   *      but still considering all candidates.
+   *
+   *  New Scheme, from 3.7
+   *  - First, determines the candidates by:
+   *    + looking at first paramater clause;
+   *    + if still no determined and there is another parameter clause, then
+   *      looks at the next argument lists to narrow the candidates (not restarting from alts).
+   *      If that finds no alternative is applicable, fallback to the previous iteration
+   *      (see comment in narrowByNextParamClause for details).
+   *  - Second, determine the best candidate by, restoring all parameter clauses, and:
+   *    + doing narrowMostSpecific on the first argument list;
+   *    + if still ambigous and there is another parameter clause, then
+   *      continue narrowMostSpecific by mapping the problem onto the next parameter clause,
+   *      but only considering the alternatives found until now.
+   */
+  enum ResolveScheme:
+    case Old
+    case New
+    def isNewPriority: Boolean = this == New
+
+  /** Resolve overloaded alternative `alts`, given expected type `pt`. Determines the current
+   *  priority scheme and emits warnings for changes in resolution in migration version.
+   */
+  def resolveOverloaded(alts: List[TermRef], pt: Type, srcPos: SrcPos)(using Context): List[TermRef] =
+    record("resolveOverloaded")
+    lazy val oldRes = resolveOverloaded(resolveOverloaded1(ResolveScheme.Old))(alts, pt)
+    lazy val newRes = resolveOverloaded(resolveOverloaded1(ResolveScheme.New))(alts, pt)
+
+    val sv = Feature.sourceVersion
+    val isNewPriorityVersion = sv.isAtLeast(SourceVersion.`3.7`)
+    val isWarnPriorityChangeVersion = sv == SourceVersion.`3.7-migration`
+
+    def doWarn(oldChoice: String, newChoice: String): Unit = report.warning(
+      em"""Overloading resolution for ${err.expectedTypeStr(pt)} between alternatives
+          | ${alts map (_.info)}%\n %
+          |has changed.
+          |Previous choice          : $oldChoice
+          |New choice from Scala 3.7: $newChoice""", srcPos)
+
+    if isWarnPriorityChangeVersion then (oldRes, newRes) match
+      case (oldAlt :: Nil, newAlt :: Nil) if oldAlt != newAlt => doWarn(oldAlt.info.show, newAlt.info.show)
+      case (oldAlt :: Nil, Nil) => doWarn(oldAlt.info.show, "none")
+      case (Nil, newAlt :: Nil) => doWarn("none", newAlt.info.show)
+      case _ => // neither scheme has determined an alternative
+
+    if isNewPriorityVersion then newRes else oldRes
+  end resolveOverloaded
+
   /** Resolve overloaded alternative `alts`, given expected type `pt`.
    *  Two trials: First, without implicits or SAM conversions enabled. Then,
    *  if the first finds no eligible candidates, with implicits and SAM conversions enabled.
    *  Each trial applies the `resolve` parameter.
    */
   def resolveOverloaded
-      (resolve: (List[TermRef], Type) => Context ?=> List[TermRef] = resolveOverloaded1)
-      (alts: List[TermRef], pt: Type, srcPos: SrcPos = NoSourcePosition)(using Context): List[TermRef] =
-    record("resolveOverloaded")
+      (resolve: (List[TermRef], Type) => Context ?=> List[TermRef])
+      (alts: List[TermRef], pt: Type)(using Context): List[TermRef] =
 
     /** Is `alt` a method or polytype whose result type after the first value parameter
      *  section conforms to the expected type `resultType`? If `resultType`
@@ -2157,7 +2213,7 @@ trait Applications extends Compatibility {
           case Nil => chosen
           case alt2 :: Nil => alt2
           case alts2 =>
-            resolveOverloaded(resolve)(alts2, pt, srcPos) match {
+            resolveOverloaded(resolve)(alts2, pt) match {
               case alt2 :: Nil => alt2
               case _ => chosen
             }
@@ -2224,7 +2280,7 @@ trait Applications extends Compatibility {
    *  It might be called twice from the public `resolveOverloaded` method, once with
    *  implicits and SAM conversions enabled, and once without.
    */
-  private def resolveOverloaded1(alts: List[TermRef], pt: Type)(using Context): List[TermRef] =
+  private def resolveOverloaded1(scheme: ResolveScheme)(alts: List[TermRef], pt: Type)(using Context): List[TermRef] =
   trace(i"resolve over $alts%, %, pt = $pt", typr, show = true):
     record(s"resolveOverloaded1", alts.length)
 
@@ -2414,7 +2470,7 @@ trait Applications extends Compatibility {
             val alts3 = narrowByTrees(alts2, pt.typedArgs(normArg(alts2, _, _)), resultType)
 
             resultType.deepenProto match
-              case resultType: FunOrPolyProto =>
+              case resultType: FunOrPolyProto if scheme.isNewPriority =>
                 narrowByNextParamClause(resolveCandidates)(alts3, pt.typedArgs(), resultType)
                   .fallbackTo(alts3) // see comment in narrowByNextParamClause
               case _ =>
@@ -2472,35 +2528,31 @@ trait Applications extends Compatibility {
           val deepPt = pt.deepenProto
           deepPt match
             case pt @ FunProto(_, resType: FunOrPolyProto) =>
-              narrowByNextParamClause(resolveOverloaded1)(found, pt.typedArgs(), resType)
+              val alts1 = if scheme.isNewPriority then found else candidates
+              narrowByNextParamClause(resolveOverloaded1(scheme))(alts1, pt.typedArgs(), resType)
             case _ =>
               // prefer alternatives that need no eta expansion
               val noCurried = alts.filterConserve(!resultIsMethod(_))
               val noCurriedCount = noCurried.length
               if noCurriedCount == 1 then
                 noCurried
               else if noCurriedCount > 1 && noCurriedCount < alts.length then
-                resolveOverloaded1(noCurried, pt)
+                resolveOverloaded1(scheme)(noCurried, pt)
               else
                 // prefer alternatves that match without default parameters
                 val noDefaults = alts.filterConserve(!_.symbol.hasDefaultParams)
                 val noDefaultsCount = noDefaults.length
                 if noDefaultsCount == 1 then
                   noDefaults
                 else if noDefaultsCount > 1 && noDefaultsCount < alts.length then
-                  resolveOverloaded1(noDefaults, pt)
+                  resolveOverloaded1(scheme)(noDefaults, pt)
                 else if deepPt ne pt then
                   // try again with a deeper known expected type
-                  resolveOverloaded1(alts, deepPt)
+                  resolveOverloaded1(scheme)(alts, deepPt)
                 else
                   candidates
     end resolveOverloaded2
 
-    // First, we find the candidates by considering all parameter clauses.
-    // Second, we determine the most specific again by considering all parameter clauses;
-    // but restarting from the 1st argument list.
-    // In both cases, considering subsequent argument lists only narrows the set of alternatives
-    // (i.e. we do retry from the complete list of alternative mapped onto there next param clause).
     val candidates = resolveCandidates(alts, pt)
     record("resolveOverloaded.narrowedApplicable", candidates.length)
     resolveOverloaded2(candidates, pt)

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

@@ -4108,7 +4108,7 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
       def altRef(alt: SingleDenotation) = TermRef(ref.prefix, ref.name, alt)
       val alts = altDenots.map(altRef)
 
-      resolveOverloaded()(alts, pt, tree.srcPos) match
+      resolveOverloaded(alts, pt, tree.srcPos) match
         case alt :: Nil =>
           readaptSimplified(tree.withType(alt))
         case Nil =>

tests/neg/multiparamlist-overload-3.6.check

@@ -1,27 +1,7 @@
--- [E007] Type Mismatch Error: tests/neg/multiparamlist-overload-3.6.scala:33:21 ---------------------------------------
-33 |    val r = f(new B)(new A) // error since resolves to R2 in 3.7 (and 3.8) as expected
+-- [E007] Type Mismatch Error: tests/neg/multiparamlist-overload-3.6.scala:41:21 ---------------------------------------
+41 |    val r = f(new B)(new A) // error: resolves to: R2 in 3.6, R1 in 3.7
    |                     ^^^^^
    |                     Found:    A
    |                     Required: B
    |
    | longer explanation available when compiling with `-explain`
--- Warning: tests/neg/multiparamlist-overload-3.6.scala:20:10 ----------------------------------------------------------
-20 |  val r = f(new B)(new C) // resolves to: R1 in 3.7, R3 in 3.8
-   |          ^
-   |          Overloading resolution for arguments (B)(C) between alternatives
-   |           (x: B)(y: B): R3
-   |           (x: B)(y: A): R2
-   |           (x: A)(y: C): R1
-   |          will change.
-   |          Current choice           : (x: A)(y: C): R1
-   |          New choice from Scala 3.7: (x: B)(y: B): R3
--- Warning: tests/neg/multiparamlist-overload-3.6.scala:40:12 ----------------------------------------------------------
-40 |    val r = f(new B)(new A) // resolves to: R1 in 3.7, R2 in 3.8 as in Part1
-   |            ^
-   |            Overloading resolution for arguments (B)(A) between alternatives
-   |             (x: B)(y: C): R3
-   |             (x: B)(y: B): R2
-   |             (x: A)(y: A): R1
-   |            will change.
-   |            Current choice           : (x: A)(y: A): R1
-   |            New choice from Scala 3.7: none

tests/neg/multiparamlist-overload-3.6.scala

@@ -1,4 +1,4 @@
-import scala.language.`3.7`
+import scala.language.`3.6`
 
 class A
 class B extends A
@@ -8,36 +8,37 @@ class R1
 class R2
 class R3
 
-// The alternatives are ordered from most genereal to most specific in each test,
+// In each test, the alternatives are defined from most genereal to most specific,
 // with respect to a lexicographic ordering by parameter list.
-
+// In each of the cases tested here, scala-3.7 resolves to the most specific applicable alternative.
+// See tests/pos/multiparamlist-overload-3.7.scala for the comparison.
 
 object Test1:
   def f(x: A)(y: C) = new R1
   def f(x: B)(y: A) = new R2
   def f(x: B)(y: B) = new R3
 
-  val r = f(new B)(new C) // resolves to: R1 in 3.7, R3 in 3.8
+  val r = f(new B)(new C) // all alternatives are applicable; resolves to: R1 in 3.6, R3 in 3.7
   val _: R1 = r
 end Test1
 
-
 object Test2:
   // R1 is the only applicable alternative in both parts
-  // but it is only resolved to in Part2 by adding (an unapplicable) R3
+  // but (in 3.7) it is only resolved to in Part1 by adding (an unapplicable) R3
 
   object Part1:
     def f(x: A)(y: A) = new R1
     def f(x: B)(y: B) = new R2
+    def f(x: B)(y: C) = new R3
 
-    val r = f(new B)(new A) // error since resolves to R2 in 3.7 (and 3.8) as expected
+    val r = f(new B)(new A) // resolves to: R1 in 3.6, R1 in 3.7
+    val _: R1 = r
 
   object Part2:
     def f(x: A)(y: A) = new R1
     def f(x: B)(y: B) = new R2
-    def f(x: B)(y: C) = new R3
 
-    val r = f(new B)(new A) // resolves to: R1 in 3.7, R2 in 3.8 as in Part1
-    val _: R1 = r
+    val r = f(new B)(new A) // error: resolves to: R2 in 3.6, R1 in 3.7
+    val _: R2 = r
 
 end Test2

tests/neg/multiparamlist-overload-3.7.check

@@ -19,4 +19,5 @@ class Matchers:
 
 class Test extends Matchers:
   def test(): Unit =
-    "hello world" should endWith ("world") // warn: overloading resolution change
+    "hello world" should endWith ("world") // no overloading resolution change
+    // related to tests/warn/multiparamlist-overload-3.7.scala

tests/neg/multiparamlist-overload-3.7.scala

@@ -0,0 +1,19 @@
+-- Warning: tests/warn/multiparamlist-overload-3.7.scala:21:10 ---------------------------------------------------------
+21 |  val r = f(new B)(new C) // warn: all alternatives are applicable; resolves to: R1 in 3.6, R3 in 3.7
+   |          ^
+   |          Overloading resolution for arguments (B)(C) between alternatives
+   |           (x: B)(y: B): R3
+   |           (x: B)(y: A): R2
+   |           (x: A)(y: C): R1
+   |          has changed.
+   |          Previous choice          : (x: A)(y: C): R1
+   |          New choice from Scala 3.7: (x: B)(y: B): R3
+-- Warning: tests/warn/multiparamlist-overload-3.7.scala:41:12 ---------------------------------------------------------
+41 |    val r = f(new B)(new A) // warn: resolves to: R2 in 3.6, R1 in 3.7
+   |            ^
+   |            Overloading resolution for arguments (B)(A) between alternatives
+   |             (x: B)(y: B): R2
+   |             (x: A)(y: A): R1
+   |            has changed.
+   |            Previous choice          : (x: B)(y: B): R2
+   |            New choice from Scala 3.7: (x: A)(y: A): R1

tests/warn/scalatest-overload-3.7.scala renamed to tests/pos/scalatest-overload-3.7.scala

@@ -0,0 +1,44 @@
+import scala.language.`3.7-migration`
+
+class A
+class B extends A
+class C extends B
+
+class R1
+class R2
+class R3
+
+// In each test, the alternatives are defined from most genereal to most specific,
+// with respect to a lexicographic ordering by parameter list.
+// In each of the cases tested here, scala-3.7 resolves to the most specific applicable alternative.
+// See tests/neg/multiparamlist-overload-3.6.scala for the comparison.
+
+object Test1:
+  def f(x: A)(y: C) = new R1
+  def f(x: B)(y: A) = new R2
+  def f(x: B)(y: B) = new R3
+
+  val r = f(new B)(new C) // warn: all alternatives are applicable; resolves to: R1 in 3.6, R3 in 3.7
+  val _: R3 = r
+end Test1
+
+object Test2:
+  // R1 is the only applicable alternative in both parts
+  // but (in 3.7) it is only resolved to in Part1 by adding (an unapplicable) R3
+
+  object Part1:
+    def f(x: A)(y: A) = new R1
+    def f(x: B)(y: B) = new R2
+    def f(x: B)(y: C) = new R3
+
+    val r = f(new B)(new A) // resolves to: R1 in 3.6, R1 in 3.7
+    val _: R1 = r
+
+  object Part2:
+    def f(x: A)(y: A) = new R1
+    def f(x: B)(y: B) = new R2
+
+    val r = f(new B)(new A) // warn: resolves to: R2 in 3.6, R1 in 3.7
+    val _: R1 = r
+
+end Test2