Add overloading support for case-closures

[odersky]

case-closures (which are represented as Match nodes) have a known
arity just like other function literals. So shape analysis for
overloading resolution should apply to them as well.

The change is completely analogous to

https://github.com/scala/scala/pull/5698/files

Review by @szeiger?

[julienrf]

This is the use case we are interested in for the collections. Do you have an idea why it fails?

[odersky]

Note: The test file in this PR contains 4 cases (lines 32-35) which compile under nsc but not under dotc. This has nothing to do with the current fix. Instead it reveals some pre-existing differences in overloading resolution. From what I could see dotc's results here seem to conform to spec. Essentially, both the MySeq member

def map1[U](f: T => U): MySeq[U] = new MySeq[U]

and the MyMap member

def map1[C, D](f: ((A, B)) => (C, D)): MyMap[C, D] = new MyMap[C, D]

survive shape analysis and therefore a typed argument is needed. Maybe I have overlooked something here? I could not figure out why this passes in nsc. On the other hand, the maps experimental branch in

scala/collection-strawman#26

compiles OK now with dotc after the present PR even if we uncomment the critical definition of xs1Bis.

[szeiger]

I believe this works in nsc due to extending scala/scala#5307 to Match arguments but AFAICT you have done the same here in isFunArg. The applicable overloads are the tupled versions

class MySeq[T] {
  def map1[U](f: T => U): MySeq[U] = new MySeq[U]
}

class MyMap[A, B] extends MySeq[(A, B)] {
  def map1[C, D](f: ((A, B)) => (C, D)): MyMap[C, D] = new MyMap[C, D]
}

For MyMap[Int, String] the types get instantiated as:

  def map1[U](f: ((Int, String)) => U): MySeq[U] = new MySeq[U]
  def map1[C, D](f: ((Int, String)) => (C, D)): MyMap[C, D] = new MyMap[C, D]

The HOF parameter types of the applicable matches are LUBbed and therefore the Match is typechecked with the expected type ((Int, String)) => ?.

[julienrf]

Essentially, both the MySeq member

def map1[U](f: T => U): MySeq[U] = new MySeq[U]

and the MyMap member

def map1[C, D](f: ((A, B)) => (C, D)): MyMap[C, D] = new MyMap[C, D]

survive shape analysis and therefore a typed argument is needed. Maybe I have overlooked something here? I could not figure out why this passes in nsc. On the other hand, the maps experimental branch in scala/collection-strawman#26 compiles OK now with dotc after the present PR even if we uncomment the critical definition of xs1Bis.

Actually, I think the strawman will be updated so that the map method of Map will have the same signature as the above map1. So, we need to fix this issue.

[odersky]

Agreed, the instantiations in question are:

def map1[U](f: ((Int, String)) => U): MySeq[U] = new MySeq[U]
def map1[C, D](f: ((Int, String)) => (C, D)): MyMap[C, D] = new MyMap[C, D]

Now, according to the SLS the second map1 does not override the first. Indeed the rule in question in https://www.scala-lang.org/files/archive/spec/2.11/05-classes-and-objects.html#class-members is:

 4. M and M′ define both polymorphic methods with equal number of argument types and equal numbers of type parameters, and ...

In our case the methods have different numbers of type parameters, so they neither match nor override each other. This means we have two members under consideration, not one. So I can't see why we are allowed to "The HOF parameter types of the applicable matches are LUBbed and therefore the Match is typechecked with the expected type ((Int, String)) => ?".

[szeiger]

@odersky The LUBbing is from the spec change here: https://github.com/scala/scala/pull/5307/files#diff-5d74b019862ed901d7800b4ace79fb5b

[odersky]

@szeiger You nailed it. Yes, that's where the compilers differ. Dotty requires formal parameter types to be the same, whereas scalac forms a lub.

[odersky]

@szeiger In fact, this was not the problem, after all. The problem was that pre-typing formal parameters of closure arguments did not apply to case-closures. Now it does. This makes
the test case compile completely with dotty. I have a separate PR (#2018) which aligns the lubbing behavior with scalac. But that change is not needed after all to make the test case here compile.

[szeiger]

Right, the argument types are the same, lubbing shouldn't be necessary in this case. I thought isFunArgs would handle the case closures but now I see it's only half of the story.

[odersky]

@szeiger

I thought isFunArgs would handle the case closures but now I see it's only half of the story.

Classic case of a missed refactoring. There were two ways to look at funargs in overloading resolution, one which checked whether parameters were unspecified and one which didn't.

[adriaanm]

Classic case of a missed refactoring.

@szeiger, could you make sure we didn't miss it as well over at scala/scala?

[szeiger]

@adriaanm Only one change was required for scalac:scala/scala@54bacee#diff-aa951aef1dfedeaa38591c8757a9d13c - This method is called from two places in Typers (checking whether missing parameter types exist and then checking where to apply the computed types)