Fix #6450: Don't drop apply alternatives in overloading resolution

[odersky]

The overloading resolution algorithm dropped alternatives reachable through applys
when testing for applicability. It did so in three situations

• when comparing the arities of alternatives
• when testing via isDirectlyApplicable
• when testing via isApplicable

The third problem stemmed from confusion between overloaded versions
of isApplicable (it's ironical that overzealous use of overloading
broke overloading resolution in the compiler!). The version for TermRefs
would not consider apply insertion, but the version for Types would.
This means that the overloading broke the Liskov substitution principle. If
a general type happened to be a TermRef the more specific isApplicable
version would kick in, which however did less than the original.

[odersky]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 0 running.

[dottybot]

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/6458/ to see the changes.

Benchmarks is based on merging with master (360dcad)

[anatoliykmetyuk]

Here's a minimised example of why ScalaCheck is failing:

trait A {
  def apply(x: Any): Int =  1
}

object B {
  def f(x: Any): Int = 2
  lazy val f = new A {}
  val x = f(null)
}

In Scala 2:

scala> trait A { def apply(x: Any): Int =  1 }; object B { def f(x: Any): Int = 2; lazy val f = new A {}; val x = f(null) }; B.x
<console>:13: error: ambiguous reference to overloaded definition,
both lazy value f in object B of type => A
and  method f in object B of type (x: Any)Int
match argument types (Null)
       trait A { def apply(x: Any): Int =  1 }; object B { def f(x: Any): Int = 2; lazy val f = new A {}; val x = f(null) };;

In this PR:

scala> trait A { def apply(x: Any): Int =  1 }; object B { def f(x: Any): Int = 2; lazy val f = new A {}; val x = f(null) }; B.x
// defined trait A
// defined object B
val res0: Int = 1

Another example:

trait A {
  def apply(x: Any): Int =  1
}

object B {
  def f(x: Any): A = new A {}
  lazy val f: A = f(null)
}

In Scala 2:

scala> trait A { def apply(x: Any): Int =  1 }; object B { def f(x: Any): A = new A {}; lazy val f: A = f(null) }; B.f
defined trait A
defined object B
res10: A = B$$anon$1@54ddd7e4

In this PR:

scala> trait A { def apply(x: Any): Int =  1 }; object B { def f(x: Any): A = new A {}; lazy val f: A = f(null) }; B.f
1 |trait A { def apply(x: Any): Int =  1 }; object B { def f(x: Any): A = new A {}; lazy val f: A = f(null) }; B.f
  |                                                                                                 ^^^^^^^
  |                                                             Found:    Int
  |                                                             Required: A

The second one is what's going on in ScalaCheck.

I'll see what can be done here.

[odersky]

@anatoliykmetyuk Thank you for the minimizations, which did highlight a problem. Unfortunately, even after fixing the problem, ScalaCheck still fails. Can you give it another try?

[anatoliykmetyuk]

Ok. This PR:

scala> trait A {
     |   def apply(x: AnyRef): Int
     | }
     |
     | object B {
     |   def f(e: Any): A = ???
     |   val f: A = f(null)
     | }
     |
7 |  val f: A = f(null)
  |             ^^^^^^^
  |             Found:    Int
  |             Required: A

scala> trait A {
     |   def apply(x: String): Int
     | }
     |
     | object B {
     |   def f(e: CharSequence): A = ???
     |   val f: A = f(null)
     | }
7 |  val f: A = f(null)
  |             ^^^^^^^
  |             Found:    Int
  |             Required: A

scala> trait A {
     |   def apply(x: String): Int
     | }
     |
     | object B {
     |   def f(e: Integer): A = ???
     |   val f: A = f(null)
     | }
7 |  val f: A = f(null)
  |             ^^^^^^^
  |             Found:    Int
  |             Required: A

But:

scala> trait A {
     |   def apply(x: Any): Int
     | }
     |
     | object B {
     |   def f(e: Any): A = ???
     |   val f: A = f(null)
     | }
     |
// defined trait A
// defined object B

scala> trait A {
     |   def apply(x: Any): Int
     | }
     |
     | object B {
     |   def f(e: AnyRef): A = ???
     |   val f: A = f(null)
     | }
     |
// defined trait A
// defined object B

scala> trait A {
     |   def apply(x: CharSequence): Int
     | }
     |
     | object B {
     |   def f(e: String): A = ???
     |   val f: A = f(null)
     | }
// defined trait A
// defined object B

Also:

scala> def f(x: Any) = 10
     | val f: Foo = f(1)   // Should show "Not found: type Foo" instead
Exception in thread "main" java.lang.StackOverflowError
	at dotty.tools.dotc.config.ScalaSettings.YnoDecodeStacktraces(ScalaSettings.scala:164)
	at dotty.tools.dotc.core.handleRecursive$.apply(TypeErrors.scala:90)
	at dotty.tools.dotc.core.Types$Type.findMember(Types.scala:724)
	at dotty.tools.dotc.core.Types$Type.memberBasedOnFlags(Types.scala:545)
	at dotty.tools.dotc.core.Types$Type.member(Types.scala:530)
	at dotty.tools.dotc.typer.Applications.onMethod(Applications.scala:1210)
	at dotty.tools.dotc.typer.Applications.sizeFits$2(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.sizeFits$1$$anonfun$1(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.onMethod$$anonfun$1(Applications.scala:1210)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.hasAltWith(Denotations.scala:752)
	at dotty.tools.dotc.typer.Applications.onMethod(Applications.scala:1210)
	at dotty.tools.dotc.typer.Applications.sizeFits$2(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.sizeFits$1$$anonfun$1(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.onMethod$$anonfun$1(Applications.scala:1210)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.hasAltWith(Denotations.scala:752)
	at dotty.tools.dotc.typer.Applications.onMethod(Applications.scala:1210)
	at dotty.tools.dotc.typer.Applications.sizeFits$2(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.sizeFits$1$$anonfun$1(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.onMethod$$anonfun$1(Applications.scala:1210)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.hasAltWith(Denotations.scala:752)
	at dotty.tools.dotc.typer.Applications.onMethod(Applications.scala:1210)
	at dotty.tools.dotc.typer.Applications.sizeFits$2(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.sizeFits$1$$anonfun$1(Applications.scala:1580)
	at dotty.tools.dotc.typer.Applications.onMethod$$anonfun$1(Applications.scala:1210)
	at dotty.tools.dotc.core.Denotations$SingleDenotation.hasAltWith(Denotations.scala:752)
	at dotty.tools.dotc.typer.Applications.onMethod(Applications.scala:1210)
...

[odersky]

Thanks for the trouble shooting @anatoliykmetyuk! That was a huge help to fix this.

[anatoliykmetyuk]

It seems this one still has corner cases.

This:

scala> trait A {
     |   def apply(x: String): A
     | }
     |
     | object B {
     |   def f(e: Integer): A = ???
     |   val f: A = ???
     |   val g: A = f(null)
     | }
     |
// defined trait A
// defined object B

Scala 2:

scala> trait A {
     |   def apply(x: String): A
     | }
defined trait A

scala>

scala> object B {
     |   def f(e: Integer): A = ???
     |   val f: A = ???
     |   val g: A = f(null)
     | }
<console>:15: error: ambiguous reference to overloaded definition,
both value f in object B of type => A
and  method f in object B of type (e: Integer)A
match argument types (Null) and expected result type A
         val g: A = f(null)

Also:

trait A {
  def apply(x: String): A = {println(2); null}
}

object B {
  def f(e: Integer): A = {println(1); null}
  val f: A = f(null)
}

object Main extends App {
  println(B.f)
}

When compiled (successfully) and run with this PR, the result is weird:

sbt:dotty> dotr Main
[warn] Multiple main classes detected.  Run 'show discoveredMainClasses' to see the list
Exception in thread "main" java.lang.ExceptionInInitializerError
	at Main$.<init>(Sample.scala:11)
	at Main$.<clinit>(Sample.scala)
	at Main.main(Sample.scala)
Caused by: java.lang.NullPointerException
	at B$.<init>(Sample.scala:7)
	at B$.<clinit>(Sample.scala)
	... 3 more

In Scala 2:

scala> trait A {
     |   def apply(x: String): A = {println(2); null}
     | }
defined trait A

scala>

scala> object B {
     |   def f(e: Integer): A = {println(1); null}
     |   val f: A = f(null)
     | }
<console>:14: error: ambiguous reference to overloaded definition,
both value f in object B of type => A
and  method f in object B of type (e: Integer)A
match argument types (Null) and expected result type A
         val f: A = f(null)

My guess is that the problem may be that isAsSpecific only does deep investigation for MethodType and PolyType. For any other, including the value f, it assumes it to be as specific as any other method type by default...

[odersky]

@anatoliykmetyuk I found a better scheme. Should be much clearer now.

[anatoliykmetyuk]

LGTM. Treat everything uniformly as early as possible – elegant solution!

[anatoliykmetyuk]

@odersky do you think we should rebase this before merging to keep the history clean?

[odersky]

It was rebased at some point already. So should be OK to merge.