Merge pull request #8484 from dotty-staging/fix-#4561

Fix #4561: Refactor overloading resolution

Author: anatoliykmetyuk

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

@@ -650,8 +650,8 @@ trait Applications extends Compatibility {
   /** Subclass of Application for applicability tests with type arguments and value
    *  argument trees.
    */
-  class ApplicableToTrees(methRef: TermRef, targs: List[Type], args: List[Tree], resultType: Type)(implicit ctx: Context)
-  extends TestApplication(methRef, methRef.widen.appliedTo(targs), args, resultType) {
+  class ApplicableToTrees(methRef: TermRef, args: List[Tree], resultType: Type)(implicit ctx: Context)
+  extends TestApplication(methRef, methRef.widen, args, resultType) {
     def argType(arg: Tree, formal: Type): Type = normalize(arg.tpe, formal)
     def treeToArg(arg: Tree): Tree = arg
     def isVarArg(arg: Tree): Boolean = tpd.isWildcardStarArg(arg)
@@ -662,7 +662,8 @@ trait Applications extends Compatibility {
   /** Subclass of Application for applicability tests with type arguments and value
    * argument trees.
    */
-  class ApplicableToTreesDirectly(methRef: TermRef, targs: List[Type], args: List[Tree], resultType: Type)(implicit ctx: Context) extends ApplicableToTrees(methRef, targs, args, resultType)(ctx) {
+  class ApplicableToTreesDirectly(methRef: TermRef, args: List[Tree], resultType: Type)(implicit ctx: Context)
+  extends ApplicableToTrees(methRef, args, resultType)(ctx) {
     override def argOK(arg: TypedArg, formal: Type): Boolean =
       argType(arg, formal) relaxed_<:< formal.widenExpr
   }
@@ -1240,34 +1241,33 @@ trait Applications extends Compatibility {
   def typedUnApply(tree: untpd.UnApply, selType: Type)(implicit ctx: Context): UnApply =
     throw new UnsupportedOperationException("cannot type check an UnApply node")
 
-  /** Is given method reference applicable to type arguments `targs` and argument trees `args`?
+  /** Is given method reference applicable to argument trees `args`?
    *  @param  resultType   The expected result type of the application
    */
-  def isApplicableMethodRef(methRef: TermRef, targs: List[Type], args: List[Tree], resultType: Type, keepConstraint: Boolean)(implicit ctx: Context): Boolean = {
+  def isApplicableMethodRef(methRef: TermRef, args: List[Tree], resultType: Type, keepConstraint: Boolean)(implicit ctx: Context): Boolean = {
     def isApp(implicit ctx: Context): Boolean =
-      new ApplicableToTrees(methRef, targs, args, resultType).success
+      new ApplicableToTrees(methRef, args, resultType).success
     if (keepConstraint) isApp else ctx.test(isApp)
   }
 
-  /** Is given method reference applicable to type arguments `targs` and argument trees `args` without inferring views?
+  /** Is given method reference applicable to argument trees `args` without inferring views?
     *  @param  resultType   The expected result type of the application
     */
-  def isDirectlyApplicableMethodRef(methRef: TermRef, targs: List[Type], args: List[Tree], resultType: Type)(implicit ctx: Context): Boolean =
-    ctx.test(new ApplicableToTreesDirectly(methRef, targs, args, resultType).success)
+  def isDirectlyApplicableMethodRef(methRef: TermRef, args: List[Tree], resultType: Type)(implicit ctx: Context): Boolean =
+    ctx.test(new ApplicableToTreesDirectly(methRef, args, resultType).success)
 
   /** Is given method reference applicable to argument types `args`?
    *  @param  resultType   The expected result type of the application
    */
   def isApplicableMethodRef(methRef: TermRef, args: List[Type], resultType: Type)(implicit ctx: Context): Boolean =
     ctx.test(new ApplicableToTypes(methRef, args, resultType).success)
 
-  /** Is given type applicable to type arguments `targs` and argument trees `args`,
-   *  possibly after inserting an `apply`?
+  /** Is given type applicable to argument trees `args`, possibly after inserting an `apply`?
    *  @param  resultType   The expected result type of the application
    */
-  def isApplicableType(tp: Type, targs: List[Type], args: List[Tree], resultType: Type, keepConstraint: Boolean)(implicit ctx: Context): Boolean =
-    onMethod(tp, targs.nonEmpty || args.nonEmpty) {
-      isApplicableMethodRef(_, targs, args, resultType, keepConstraint)
+  def isApplicableType(tp: Type, args: List[Tree], resultType: Type, keepConstraint: Boolean)(implicit ctx: Context): Boolean =
+    onMethod(tp, args.nonEmpty) {
+      isApplicableMethodRef(_, args, resultType, keepConstraint)
     }
 
   /** Is given type applicable to argument types `args`, possibly after inserting an `apply`?
@@ -1538,13 +1538,11 @@ trait Applications extends Compatibility {
     }
   }
 
-  /** Resolve overloaded alternative `alts`, given expected type `pt` and
-   *  possibly also type argument `targs` that need to be applied to each alternative
-   *  to form the method type.
+  /** 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.
    */
-  def resolveOverloaded(alts: List[TermRef], pt: Type)(implicit ctx: Context): List[TermRef] = {
+  def resolveOverloaded(alts: List[TermRef], pt: Type)(implicit ctx: Context): List[TermRef] =
     record("resolveOverloaded")
 
     /** Is `alt` a method or polytype whose result type after the first value parameter
@@ -1590,15 +1588,26 @@ trait Applications extends Compatibility {
       case _ => chosen
     }
 
-    def resolve(alts: List[TermRef]) = {
-      var found = resolveOverloaded(alts, pt, Nil)(ctx.retractMode(Mode.ImplicitsEnabled))
-      if (found.isEmpty && ctx.mode.is(Mode.ImplicitsEnabled))
-        found = resolveOverloaded(alts, pt, Nil)
-      found match {
+    def resolve(alts: List[TermRef]): List[TermRef] =
+      pt match
+        case pt: FunProto =>
+          if pt.isUsingApply then
+            val alts0 = alts.filterConserve { alt =>
+              val mt = alt.widen.stripPoly
+              mt.isImplicitMethod || mt.isContextualMethod
+            }
+            if alts0 ne alts then return resolve(alts0)
+          else if alts.exists(_.widen.stripPoly.isContextualMethod) then
+            return resolveMapped(alts, alt => stripImplicit(alt.widen), pt)
+        case _ =>
+
+      var found = resolveOverloaded1(alts, pt)(using ctx.retractMode(Mode.ImplicitsEnabled))
+      if found.isEmpty && ctx.mode.is(Mode.ImplicitsEnabled) then
+        found = resolveOverloaded1(alts, pt)
+      found match
         case alt :: Nil => adaptByResult(alt, alts) :: Nil
         case _ => found
-      }
-    }
+    end resolve
 
     /** Try an apply method, if
      *   - the result is applied to value arguments and alternative is not a method, or
@@ -1634,15 +1643,16 @@ trait Applications extends Compatibility {
       resolve(expanded).map(retract)
     }
     else resolve(alts)
-  }
+  end resolveOverloaded
 
   /** This private version of `resolveOverloaded` does the bulk of the work of
-   *  overloading resolution, but does not do result adaptation. It might be
-   *  called twice from the public `resolveOverloaded` method, once with
+   *  overloading resolution, but does neither result adaptation nor apply insertion.
+   *  It might be called twice from the public `resolveOverloaded` method, once with
    *  implicits and SAM conversions enabled, and once without.
    */
-  private def resolveOverloaded(alts: List[TermRef], pt: Type, targs: List[Type])(implicit ctx: Context): List[TermRef] =
-    record("resolveOverloaded/2")
+  private def resolveOverloaded1(alts: List[TermRef], pt: Type)(implicit ctx: Context): List[TermRef] =
+    trace(i"resolve over $alts%, %, pt = $pt", typr, show = true) {
+    record("resolveOverloaded1")
 
     def isDetermined(alts: List[TermRef]) = alts.isEmpty || alts.tail.isEmpty
 
@@ -1707,22 +1717,6 @@ trait Applications extends Compatibility {
       case _ => arg
     end normArg
 
-    /** Resolve overloading by mapping to a different problem where each alternative's
-     *  type is mapped with `f`, alternatives with non-existing types are dropped, and the
-     *  expected type is `pt`. Map the results back to the original alternatives.
-     */
-    def resolveMapped(alts: List[TermRef], f: TermRef => Type, pt: Type): List[TermRef] =
-      val reverseMapping = alts.flatMap { alt =>
-        val t = f(alt)
-        if t.exists then
-          Some((TermRef(NoPrefix, alt.symbol.asTerm.copy(info = t)), alt))
-        else
-          None
-      }
-      val mapped = reverseMapping.map(_._1)
-      overload.println(i"resolve mapped: $mapped")
-      resolveOverloaded(mapped, pt, targs).map(reverseMapping.toMap)
-
     val candidates = pt match {
       case pt @ FunProto(args, resultType) =>
         val numArgs = args.length
@@ -1753,25 +1747,16 @@ trait Applications extends Compatibility {
 
         def narrowByTrees(alts: List[TermRef], args: List[Tree], resultType: Type): List[TermRef] = {
           val alts2 = alts.filter(alt =>
-            isDirectlyApplicableMethodRef(alt, targs, args, resultType)
+            isDirectlyApplicableMethodRef(alt, args, resultType)
           )
           if (alts2.isEmpty && !ctx.isAfterTyper)
             alts.filter(alt =>
-              isApplicableMethodRef(alt, targs, args, resultType, keepConstraint = false)
+              isApplicableMethodRef(alt, args, resultType, keepConstraint = false)
             )
           else
             alts2
         }
 
-        if pt.isUsingApply then
-          val alts0 = alts.filterConserve { alt =>
-            val mt = alt.widen.stripPoly
-            mt.isImplicitMethod || mt.isContextualMethod
-          }
-          if alts0 ne alts then return resolveOverloaded(alts0, pt, targs)
-        else if alts.exists(_.widen.stripPoly.isContextualMethod) then
-          return resolveMapped(alts, alt => stripImplicit(alt.widen), pt)
-
         val alts1 = narrowBySize(alts)
         //ctx.log(i"narrowed by size: ${alts1.map(_.symbol.showDcl)}%, %")
         if isDetermined(alts1) then alts1
@@ -1783,9 +1768,10 @@ trait Applications extends Compatibility {
             pretypeArgs(alts2, pt)
             narrowByTrees(alts2, pt.typedArgs(normArg(alts2, _, _)), resultType)
 
-      case pt @ PolyProto(targs1, pt1) if targs.isEmpty =>
-        val alts1 = alts.filter(pt.isMatchedBy(_))
-        resolveOverloaded(alts1, pt1, targs1.tpes)
+      case pt @ PolyProto(targs1, pt1) =>
+        val alts1 = alts.filter(pt.canInstantiate)
+        if isDetermined(alts1) then alts1
+        else resolveMapped(alts1, _.widen.appliedTo(targs1.tpes), pt1)
 
       case defn.FunctionOf(args, resultType, _, _) =>
         narrowByTypes(alts, args, resultType)
@@ -1842,21 +1828,38 @@ trait Applications extends Compatibility {
           if noCurriedCount == 1 then
             noCurried
           else if noCurriedCount > 1 && noCurriedCount < alts.length then
-            resolveOverloaded(noCurried, pt, targs)
+            resolveOverloaded1(noCurried, pt)
           else
             // prefer alternatves that match without default parameters
             val noDefaults = alts.filter(!_.symbol.hasDefaultParams)
             val noDefaultsCount = noDefaults.length
             if noDefaultsCount == 1 then
               noDefaults
             else if noDefaultsCount > 1 && noDefaultsCount < alts.length then
-              resolveOverloaded(noDefaults, pt, targs)
+              resolveOverloaded1(noDefaults, pt)
             else if deepPt ne pt then
               // try again with a deeper known expected type
-              resolveOverloaded(alts, deepPt, targs)
+              resolveOverloaded1(alts, deepPt)
             else
               candidates
-  end resolveOverloaded
+    }
+  end resolveOverloaded1
+
+  /** Resolve overloading by mapping to a different problem where each alternative's
+   *  type is mapped with `f`, alternatives with non-existing types are dropped, and the
+   *  expected type is `pt`. Map the results back to the original alternatives.
+   */
+  def resolveMapped(alts: List[TermRef], f: TermRef => Type, pt: Type)(using Context): List[TermRef] =
+    val reverseMapping = alts.flatMap { alt =>
+      val t = f(alt)
+      if t.exists then
+        Some((TermRef(NoPrefix, alt.symbol.asTerm.copy(info = t)), alt))
+      else
+        None
+    }
+    val mapped = reverseMapping.map(_._1)
+    overload.println(i"resolve mapped: $mapped")
+    resolveOverloaded(mapped, pt).map(reverseMapping.toMap)
 
   /** Try to typecheck any arguments in `pt` that are function values missing a
    *  parameter type. If the formal parameter types corresponding to a closure argument

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

@@ -260,7 +260,7 @@ object ProtoTypes {
       def isPoly(tree: Tree) = tree.tpe.widenSingleton.isInstanceOf[PolyType]
       // See remark in normalizedCompatible for why we can't keep the constraint
       // if one of the arguments has a PolyType.
-      typer.isApplicableType(tp, Nil, args, resultType, keepConstraint && !args.exists(isPoly))
+      typer.isApplicableType(tp, args, resultType, keepConstraint && !args.exists(isPoly))
     }
 
     def derivedFunProto(args: List[untpd.Tree] = this.args, resultType: Type, typer: Typer = this.typer): FunProto =
@@ -454,13 +454,12 @@ object ProtoTypes {
 
     override def resultType(implicit ctx: Context): Type = resType
 
-    override def isMatchedBy(tp: Type, keepConstraint: Boolean)(implicit ctx: Context): Boolean = {
-      def isInstantiatable(tp: Type) = tp.widen match {
-        case tp: PolyType => tp.paramNames.length == targs.length
-        case _ => false
-      }
-      isInstantiatable(tp) || tp.member(nme.apply).hasAltWith(d => isInstantiatable(d.info))
-    }
+    def canInstantiate(tp: Type)(using Context) = tp.widen match
+      case tp: PolyType => tp.paramNames.length == targs.length
+      case _ => false
+
+    override def isMatchedBy(tp: Type, keepConstraint: Boolean)(implicit ctx: Context): Boolean =
+      canInstantiate(tp) || tp.member(nme.apply).hasAltWith(d => canInstantiate(d.info))
 
     def derivedPolyProto(targs: List[Tree], resultType: Type): PolyProto =
       if ((targs eq this.targs) && (resType eq this.resType)) this

language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala

@@ -159,7 +159,7 @@ class SignatureHelpTest {
            }""".withSource
       .signatureHelp(m1, List(sig0, sig1), None, 0)
       .signatureHelp(m2, List(sig0, sig1), None, 0)
-      .signatureHelp(m3, List(), Some(1), 1)  // TODO: investigate we do not get help at $m3
+      .signatureHelp(m3, List(sig0, sig1), Some(1), 1)
   }
 
   @Test def multipleParameterLists: Unit = {

tests/pos/i4561.scala

@@ -0,0 +1,20 @@
+object abc:
+  trait Test0
+  trait Test1[T]:
+    def apply(f: T => T): Unit
+    def apply(s: String): Unit
+
+  def v: Test0 = ???
+  def v[T]: Test1[Int] = ???
+
+  def w: Test0 = ???
+  def w[T]: Test1[T] = ???
+  def w[T](x: String): Test1[T] = ???
+
+  v[Any](x => x)
+  val v1: Test0 = v
+  v(x => x + 1)
+
+  w[Int](_ + 1)
+  // w(_ + 1)      // error: `+` is not a member of Any
+  w[Int]("x")(_ + 1)