From 54835b6fb19ab0758c7503fb6f0e990ee4c25491 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Wed, 17 Jun 2015 18:00:21 +0200 Subject: [PATCH 1/5] Take expected result type into account more often for overloading resolution Previously, the expected result type of a FunProto type was ignored and taken into account only in case of ambiguities. arrayclone-new.scala shows that this is not enough. In a case like val x: Array[Byte] = Array(1, 2) we typed 1, 2 to be Int, so overloading resulution would give the Array.apply of type (Int, Int*)Array[Int]. But that's a dead end, since Array[Int] is not a subtype of Array[Byte]. This commit proposes the following modified rule for overloading resulution: A method alternative is applicable if ... (as before), and if its result type is copmpatible with the expected type of the method application. The commit does not pre-select alternatives based on comparing with the expected result type. I tried that but it slowed down typechecking by a factor of at least 4. Instead, we proceed as usual, ignoring the result type except in case of ambiguities, but check whether the result of overloading resolution has a compatible result type. If that's not the case, we filter all alternatives for result type compatibility and try again. --- src/dotty/tools/dotc/typer/Applications.scala | 40 +++++++++++++++++++ src/dotty/tools/dotc/typer/ProtoTypes.scala | 2 +- src/dotty/tools/dotc/typer/Typer.scala | 2 +- tests/{pending => }/run/arrayclone-new.scala | 3 +- 4 files changed, 44 insertions(+), 3 deletions(-) rename tests/{pending => }/run/arrayclone-new.scala (98%) diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index 9e9acf97a2a1..c162abd5f1d0 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -1039,6 +1039,46 @@ trait Applications extends Compatibility { self: Typer => } } + /** If the `chosen` alternative has a result type incompatible with the expected result + * type `pt`, run overloading resolution again on all alternatives that do match `pt`. + * If the latter succeeds with a single alternative, return it, otherwise + * fallback to `chosen`. + */ + def adaptByResult(alts: List[TermRef], chosen: TermRef, pt: Type)(implicit ctx: Context) = + if (ctx.isAfterTyper) chosen + else { + def nestedCtx = ctx.fresh.setExploreTyperState + pt match { + case pt: FunProto if !resultConforms(chosen, pt.resultType)(nestedCtx) => + alts.filter(alt => + (alt ne chosen) && resultConforms(alt, pt.resultType)(nestedCtx)) match { + case Nil => chosen + case alt2 :: Nil => alt2 + case alts2 => + resolveOverloaded(alts2, pt) match { + case alt2 :: Nil => alt2 + case _ => chosen + } + } + case _ => chosen + } + } + + /** Is `alt` a method or polytype whose result type after the first value parameter + * section conforms to the expected type `resultType`? If `resultType` + * is a `IgnoredProto`, pick the underlying type instead. + */ + private def resultConforms(alt: Type, resultType: Type)(implicit ctx: Context): Boolean = resultType match { + case IgnoredProto(ignored) => resultConforms(alt, ignored) + case _: ValueType => + alt.widen match { + case tp: PolyType => resultConforms(constrained(tp).resultType, resultType) + case tp: MethodType => constrainResult(tp.resultType, resultType) + case _ => true + } + case _ => true + } + private def harmonizeWith[T <: AnyRef](ts: List[T])(tpe: T => Type, adapt: (T, Type) => T)(implicit ctx: Context): List[T] = { def numericClasses(ts: List[T], acc: Set[Symbol]): Set[Symbol] = ts match { case t :: ts1 => diff --git a/src/dotty/tools/dotc/typer/ProtoTypes.scala b/src/dotty/tools/dotc/typer/ProtoTypes.scala index c7efe45b7b23..9a012c30e0b0 100644 --- a/src/dotty/tools/dotc/typer/ProtoTypes.scala +++ b/src/dotty/tools/dotc/typer/ProtoTypes.scala @@ -46,7 +46,7 @@ object ProtoTypes { * fits the given expected result type. */ def constrainResult(mt: Type, pt: Type)(implicit ctx: Context): Boolean = pt match { - case _: FunProto => + case pt: FunProto => mt match { case mt: MethodType => mt.isDependent || constrainResult(mt.resultType, pt.resultType) diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index 2bdd0d19714e..ce0a181c34e1 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -1238,7 +1238,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit def expectedStr = err.expectedTypeStr(pt) resolveOverloaded(alts, pt) match { case alt :: Nil => - adapt(tree.withType(alt), pt, original) + adapt(tree.withType(adaptByResult(alts, alt, pt)), pt, original) case Nil => def noMatches = errorTree(tree, diff --git a/tests/pending/run/arrayclone-new.scala b/tests/run/arrayclone-new.scala similarity index 98% rename from tests/pending/run/arrayclone-new.scala rename to tests/run/arrayclone-new.scala index 5f09cd73a1e4..8f66d1b31243 100644 --- a/tests/pending/run/arrayclone-new.scala +++ b/tests/run/arrayclone-new.scala @@ -1,4 +1,4 @@ -import scala.reflect.{ClassTag, classTag} +import scala.reflect.ClassTag object Test extends dotty.runtime.LegacyApp{ BooleanArrayClone; @@ -106,3 +106,4 @@ object PolymorphicArrayClone{ testIt(mangled.it, 0, 1); } + From aef3d5734e27898bd7e06a7f940509ce14ef684c Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 19 Jun 2015 13:19:54 +0200 Subject: [PATCH 2/5] Always take results into account for resolveOverloading Previously, this was not done when resolving through tpd. Also, improve comments to explain why we picked the "check afterwards" strategy. Finally, refactor so that the new logic is all inside resolveOverloaded. --- src/dotty/tools/dotc/typer/Applications.scala | 99 ++++++++++--------- src/dotty/tools/dotc/typer/Typer.scala | 2 +- 2 files changed, 54 insertions(+), 47 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index c162abd5f1d0..ec516c56ce7d 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -966,6 +966,53 @@ trait Applications extends Compatibility { self: Typer => def narrowByTypes(alts: List[TermRef], argTypes: List[Type], resultType: Type): List[TermRef] = alts filter (isApplicable(_, argTypes, resultType)) + /** Is `alt` a method or polytype whose result type after the first value parameter + * section conforms to the expected type `resultType`? If `resultType` + * is a `IgnoredProto`, pick the underlying type instead. + */ + def resultConforms(alt: Type, resultType: Type)(implicit ctx: Context): Boolean = resultType match { + case IgnoredProto(ignored) => resultConforms(alt, ignored) + case _: ValueType => + alt.widen match { + case tp: PolyType => resultConforms(constrained(tp).resultType, resultType) + case tp: MethodType => constrainResult(tp.resultType, resultType) + case _ => true + } + case _ => true + } + + /** If the `chosen` alternative has a result type incompatible with the expected result + * type `pt`, run overloading resolution again on all alternatives that do match `pt`. + * If the latter succeeds with a single alternative, return it, otherwise + * fallback to `chosen`. + * + * Note this order of events is done for speed. One might be tempted to + * preselect alternatives by result type. But is slower, because it discriminates + * less. The idea is when searching for a best solution, as is the case in overloading + * resolution, we should first try criteria which are cheap and which have a high + * probability of pruning the search. result type comparisons are neither cheap nor + * do they prune much, on average. + */ + def adaptByResult(alts: List[TermRef], chosen: TermRef) = + if (ctx.isAfterTyper) chosen + else { + def nestedCtx = ctx.fresh.setExploreTyperState + pt match { + case pt: FunProto if !resultConforms(chosen, pt.resultType)(nestedCtx) => + alts.filter(alt => + (alt ne chosen) && resultConforms(alt, pt.resultType)(nestedCtx)) match { + case Nil => chosen + case alt2 :: Nil => alt2 + case alts2 => + resolveOverloaded(alts2, pt) match { + case alt2 :: Nil => alt2 + case _ => chosen + } + } + case _ => chosen + } + } + val candidates = pt match { case pt @ FunProto(args, resultType, _) => val numArgs = args.length @@ -1029,56 +1076,16 @@ trait Applications extends Compatibility { self: Typer => } if (isDetermined(candidates)) candidates else narrowMostSpecific(candidates) match { - case result @ (alt1 :: alt2 :: _) => -// overload.println(i"ambiguous $alt1 $alt2") + case Nil => Nil + case alt :: Nil => adaptByResult(candidates, alt) :: Nil + case alts => +// overload.println(i"ambiguous $alts%, %") val deepPt = pt.deepenProto - if (deepPt ne pt) resolveOverloaded(alts, deepPt, targs) - else result - case result => - result + if (deepPt ne pt) resolveOverloaded(candidates, deepPt, targs) + else alts } } - /** If the `chosen` alternative has a result type incompatible with the expected result - * type `pt`, run overloading resolution again on all alternatives that do match `pt`. - * If the latter succeeds with a single alternative, return it, otherwise - * fallback to `chosen`. - */ - def adaptByResult(alts: List[TermRef], chosen: TermRef, pt: Type)(implicit ctx: Context) = - if (ctx.isAfterTyper) chosen - else { - def nestedCtx = ctx.fresh.setExploreTyperState - pt match { - case pt: FunProto if !resultConforms(chosen, pt.resultType)(nestedCtx) => - alts.filter(alt => - (alt ne chosen) && resultConforms(alt, pt.resultType)(nestedCtx)) match { - case Nil => chosen - case alt2 :: Nil => alt2 - case alts2 => - resolveOverloaded(alts2, pt) match { - case alt2 :: Nil => alt2 - case _ => chosen - } - } - case _ => chosen - } - } - - /** Is `alt` a method or polytype whose result type after the first value parameter - * section conforms to the expected type `resultType`? If `resultType` - * is a `IgnoredProto`, pick the underlying type instead. - */ - private def resultConforms(alt: Type, resultType: Type)(implicit ctx: Context): Boolean = resultType match { - case IgnoredProto(ignored) => resultConforms(alt, ignored) - case _: ValueType => - alt.widen match { - case tp: PolyType => resultConforms(constrained(tp).resultType, resultType) - case tp: MethodType => constrainResult(tp.resultType, resultType) - case _ => true - } - case _ => true - } - private def harmonizeWith[T <: AnyRef](ts: List[T])(tpe: T => Type, adapt: (T, Type) => T)(implicit ctx: Context): List[T] = { def numericClasses(ts: List[T], acc: Set[Symbol]): Set[Symbol] = ts match { case t :: ts1 => diff --git a/src/dotty/tools/dotc/typer/Typer.scala b/src/dotty/tools/dotc/typer/Typer.scala index ce0a181c34e1..2bdd0d19714e 100644 --- a/src/dotty/tools/dotc/typer/Typer.scala +++ b/src/dotty/tools/dotc/typer/Typer.scala @@ -1238,7 +1238,7 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit def expectedStr = err.expectedTypeStr(pt) resolveOverloaded(alts, pt) match { case alt :: Nil => - adapt(tree.withType(adaptByResult(alts, alt, pt)), pt, original) + adapt(tree.withType(alt), pt, original) case Nil => def noMatches = errorTree(tree, From 631f7c1c9195c91e47403c9b1582f5f998ed2e66 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 19 Jun 2015 14:35:21 +0200 Subject: [PATCH 3/5] Refine result type checking New test case: array-overload.scala. Failed before, succeeds now. --- src/dotty/tools/dotc/typer/Applications.scala | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/dotty/tools/dotc/typer/Applications.scala b/src/dotty/tools/dotc/typer/Applications.scala index ec516c56ce7d..c45db4ccc827 100644 --- a/src/dotty/tools/dotc/typer/Applications.scala +++ b/src/dotty/tools/dotc/typer/Applications.scala @@ -915,7 +915,9 @@ trait Applications extends Compatibility { self: Typer => }} def narrowMostSpecific(alts: List[TermRef])(implicit ctx: Context): List[TermRef] = track("narrowMostSpecific") { - (alts: @unchecked) match { + alts match { + case Nil => alts + case _ :: Nil => alts case alt :: alts1 => def winner(bestSoFar: TermRef, alts: List[TermRef]): TermRef = alts match { case alt :: alts1 => @@ -993,9 +995,7 @@ trait Applications extends Compatibility { self: Typer => * probability of pruning the search. result type comparisons are neither cheap nor * do they prune much, on average. */ - def adaptByResult(alts: List[TermRef], chosen: TermRef) = - if (ctx.isAfterTyper) chosen - else { + def adaptByResult(alts: List[TermRef], chosen: TermRef) = { def nestedCtx = ctx.fresh.setExploreTyperState pt match { case pt: FunProto if !resultConforms(chosen, pt.resultType)(nestedCtx) => @@ -1074,14 +1074,19 @@ trait Applications extends Compatibility { self: Typer => case pt => alts filter (normalizedCompatible(_, pt)) } - if (isDetermined(candidates)) candidates - else narrowMostSpecific(candidates) match { + narrowMostSpecific(candidates) match { case Nil => Nil - case alt :: Nil => adaptByResult(candidates, alt) :: Nil + case alt :: Nil => + adaptByResult(alts, alt) :: Nil + // why `alts` and not `candidates`? pos/array-overload.scala gives a test case. + // Here, only the Int-apply is a candidate, but it is not compatible with the result + // type. Picking the Byte-apply as the only result-compatible solution then forces + // the arguments (which are constants) to be adapted to Byte. If we had picked + // `candidates` instead, no solution would have been found. case alts => // overload.println(i"ambiguous $alts%, %") val deepPt = pt.deepenProto - if (deepPt ne pt) resolveOverloaded(candidates, deepPt, targs) + if (deepPt ne pt) resolveOverloaded(alts, deepPt, targs) else alts } } From faa7078fffb868c22f783388df0a6d80f64ee0c1 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 19 Jun 2015 14:49:43 +0200 Subject: [PATCH 4/5] Add failing test for #670. --- tests/pending/pos/t8230a.scala | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/pending/pos/t8230a.scala diff --git a/tests/pending/pos/t8230a.scala b/tests/pending/pos/t8230a.scala new file mode 100644 index 000000000000..405aa86f552c --- /dev/null +++ b/tests/pending/pos/t8230a.scala @@ -0,0 +1,26 @@ +trait Arr[T] +object Arr { + def apply[T](xs: T): Arr[T] = null + def apply(x: Long) : Arr[Long] = null +} + +object I { + implicit def arrToTrav[T] (a: Arr[T]) : Traversable[T] = null + implicit def longArrToTrav(a: Arr[Long]): Traversable[Long] = null +} + +object Test { + def foo(t: Traversable[Any]) = {} + + object Okay { + Arr("1") + + import I.{ arrToTrav, longArrToTrav } + foo(Arr("2")) + } + + object Fail { + import I.arrToTrav + foo(Arr("3")) // found String, expected Long + } +} From 0bbc85895807aa4df05f46b1f0794a8c2fc4d095 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 19 Jun 2015 15:31:16 +0200 Subject: [PATCH 5/5] Add test that succeeds --- tests/pos/array-overload.scala | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 tests/pos/array-overload.scala diff --git a/tests/pos/array-overload.scala b/tests/pos/array-overload.scala new file mode 100644 index 000000000000..737ea0ec735a --- /dev/null +++ b/tests/pos/array-overload.scala @@ -0,0 +1,8 @@ +class Test { + object A { + def apply(x: Byte, xs: Byte*): Array[Byte] = ??? + def apply(x: Int, xs: Int*): Array[Int] = ??? + } + + val x: Array[Byte] = A.apply(1, 2) +}