Take expected result type into account for overloading resolution

[odersky]

Previously, the expected result type of a FunProto type was ignored and taken into
account only in case of ambiguities. run/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 resolution 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 resolution:

A method alternative is applicable if ... (as before), and if its result type
is compatible 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.

Review by @adriaanm

[odersky]

There's a download failure for jline 2.12.jar. What to do?

[DarkDimius]

/rebuild

[adriaanm]

I wonder whether resolveOverloaded could do adaptByResult's work from the start? What's the rationale for doing this afterwards?

[odersky]

@adriaanm It would be much slower. I tried that first and it looked like at least 4 times blow-up in compilation times.I was surprised at that drastic result, and don't really have an explanation for the magnitude of it. But in principle it makes sense: When doing overloading resolution, we want to prune the search space as quickly as possible. Results of overloaded methods tend to be similar, so checking them first does not prune much. Better to concentrate on the arguments first.

[adriaanm]

Ok, would be good to include this as a comment to avoid someone slowing down the compiler if they happened to implement what I was wondering about :-)

[adriaanm]

One last concern: are there other calls to resolveOverloaded that need similar post-processing? If so, it would still make sense to create a separate method that combines resolveOverloaded and adaptByResult.

[retronym]

What alternative of Array.apply is selected after this change? In Scalac, we fall back to the generic version ([T](xs: T*)(implicit evidence$2: scala.reflect.ClassTag[T])Array[T]).

% cat sandbox/test.scala
class Test {
  val x: Array[Byte] = Array(1, 2)
}
% qscalac -Xprint:typer sandbox/test.scala
[[syntax trees at end of                     typer]] // test.scala
package <empty> {
  class Test extends scala.AnyRef {
    def <init>(): Test = {
      Test.super.<init>();
      ()
    };
    private[this] val x: Array[Byte] = scala.Array.apply[Byte](1, 2)((ClassTag.Byte: scala.reflect.ClassTag[Byte]));
    <stable> <accessor> def x: Array[Byte] = Test.this.x
  }
}

This comes from the the way that inferMethodAlternative tries to resolve a winner 1) with and then 2) without the expected type.

[retronym]

See also https://issues.scala-lang.org/browse/SI-8230, in which I've extract a standalone test case to mimic the overloads and associated implicit conversions of Array#apply, which leads to this puzzler:

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
  }
}

I traced out the type inference here: scala/scala@2.11.x...retronym:ticket/8230

[retronym]

Overall, I think any intentional divergence between the Scala and dotty type checker around overloading/inference needs to be strongly motivated and thoroughly tested. While it can be tricky to reverse engineer some of the workings of the current system, it does represent "the devil we know".

[odersky]

@retronym I verified that dottys resolution is the same as scalac's. t8230a.scala is a crasher after the typer. See #670.