From aeb7f43b5a0592be7ab25bc52ffa2c78d019baef Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 9 Mar 2020 17:00:37 +0100 Subject: [PATCH 1/5] Fix #4561: Refactor overloading resolution refactor overloading resolution to make handling of type arguments more regular and systematic. --- .../dotty/tools/dotc/typer/Applications.scala | 99 ++++++++++--------- .../dotty/tools/dotc/typer/ProtoTypes.scala | 13 ++- tests/pos/i4561.scala | 10 ++ 3 files changed, 68 insertions(+), 54 deletions(-) create mode 100644 tests/pos/i4561.scala diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 44cd5b47c420..68f128d9877b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1544,7 +1544,7 @@ trait Applications extends Compatibility { * 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 +1590,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 +1645,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 +1719,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 +1749,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, Nil, args, resultType) ) if (alts2.isEmpty && !ctx.isAfterTyper) alts.filter(alt => - isApplicableMethodRef(alt, targs, args, resultType, keepConstraint = false) + isApplicableMethodRef(alt, Nil, 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 +1770,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,7 +1830,7 @@ 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) @@ -1850,13 +1838,30 @@ trait Applications extends Compatibility { 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 diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 37b4cce5ff5a..7380f788cccd 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -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 diff --git a/tests/pos/i4561.scala b/tests/pos/i4561.scala new file mode 100644 index 000000000000..6e843b9ec656 --- /dev/null +++ b/tests/pos/i4561.scala @@ -0,0 +1,10 @@ +object abc { + trait Test0 + trait Test1 { def apply(f: Int => Int): Unit } + + def v: Test0 = ??? + def v[T]: Test1 = ??? + //def v[T](x: String): Test0 = ??? + + v[Int]{ v => v } +} \ No newline at end of file From 13719405d28ddaacb0a05d9855819ed81526d8b8 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 9 Mar 2020 17:08:44 +0100 Subject: [PATCH 2/5] Don't pass type arguments in applicable tests --- .../dotty/tools/dotc/typer/Applications.scala | 36 +++++++++---------- .../dotty/tools/dotc/typer/ProtoTypes.scala | 2 +- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 68f128d9877b..073300441af6 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/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.widenTermRefExpr, 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,20 +1241,20 @@ 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 @@ -1261,13 +1262,12 @@ trait Applications extends Compatibility { 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,9 +1538,7 @@ 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. */ @@ -1749,11 +1747,11 @@ trait Applications extends Compatibility { def narrowByTrees(alts: List[TermRef], args: List[Tree], resultType: Type): List[TermRef] = { val alts2 = alts.filter(alt => - isDirectlyApplicableMethodRef(alt, Nil, args, resultType) + isDirectlyApplicableMethodRef(alt, args, resultType) ) if (alts2.isEmpty && !ctx.isAfterTyper) alts.filter(alt => - isApplicableMethodRef(alt, Nil, args, resultType, keepConstraint = false) + isApplicableMethodRef(alt, args, resultType, keepConstraint = false) ) else alts2 diff --git a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala b/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala index 7380f788cccd..d707f49a4bf7 100644 --- a/compiler/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/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 = From 9a02f8ea6ce4b74b80a4258f85353c2224f43f52 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 9 Mar 2020 22:03:00 +0100 Subject: [PATCH 3/5] Fix unit test --- .../test/dotty/tools/languageserver/SignatureHelpTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala b/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala index 85dc484c95f4..e23100cdcf4f 100644 --- a/language-server/test/dotty/tools/languageserver/SignatureHelpTest.scala +++ b/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 = { From a3b66462945a31fa94973382259340f96c733d6f Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Tue, 10 Mar 2020 08:59:00 +0100 Subject: [PATCH 4/5] Fix auto-suggest breakage and expand test --- .../dotty/tools/dotc/typer/Applications.scala | 2 +- tests/pos/i4561.scala | 22 ++++++++++++++----- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 073300441af6..6a0862eb172f 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -651,7 +651,7 @@ trait Applications extends Compatibility { * argument trees. */ class ApplicableToTrees(methRef: TermRef, args: List[Tree], resultType: Type)(implicit ctx: Context) - extends TestApplication(methRef, methRef.widenTermRefExpr, args, resultType) { + 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) diff --git a/tests/pos/i4561.scala b/tests/pos/i4561.scala index 6e843b9ec656..0eb9b8c63976 100644 --- a/tests/pos/i4561.scala +++ b/tests/pos/i4561.scala @@ -1,10 +1,20 @@ -object abc { +object abc: trait Test0 - trait Test1 { def apply(f: Int => Int): Unit } + trait Test1[T]: + def apply(f: T => T): Unit + def apply(s: String): Unit def v: Test0 = ??? - def v[T]: Test1 = ??? - //def v[T](x: String): Test0 = ??? + def v[T]: Test1[Int] = ??? - v[Int]{ v => v } -} \ No newline at end of file + 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) From c075f8b1f5f16b9f73160b600b10e4c0d3abb89d Mon Sep 17 00:00:00 2001 From: Anatolii Kmetiuk Date: Thu, 12 Mar 2020 14:23:54 +0100 Subject: [PATCH 5/5] Typo fix in tests/pos/i4561.scala --- tests/pos/i4561.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/pos/i4561.scala b/tests/pos/i4561.scala index 0eb9b8c63976..5a51cb668a37 100644 --- a/tests/pos/i4561.scala +++ b/tests/pos/i4561.scala @@ -16,5 +16,5 @@ object abc: v(x => x + 1) w[Int](_ + 1) - // w[_ + 1) // error: `+` is not a member of Any + // w(_ + 1) // error: `+` is not a member of Any w[Int]("x")(_ + 1)