[WIP] Improve overload resolution

[EugeneFlesselle]

resolveOverloaded1 starts with narrowMostSpecific(candidates) which compares the alternatives based on the 1st argument list. If more than one result if found, we can continue with the 2nd argument list. But we do so with the original candidates, rather than with the result of narrowMostSpecific. This can lead to choosing an alternative which is not the most specific, by now disregarding the 1st argument list.

Fixes #20053, the 1st pass correctly eliminated the 3rd def ^^^, but could not resolve between the 1st two (having the same 1st argument list). The 2nd pass then disregarded this and restarted the comparison based on the 2nd argument list alone, which yielded the 3rd option.

The change is just to use the initial result of narrowMostSpecific in the recursive resolution based on subsequent argument lists.

EDIT: as of f40c609 the changes in overload resolution apply starting from 3.7 and are reported in warnings in 3.6.

[sjrd]

Phew, I got it minimized to

class TestBody1
class TestBody2

class StartWithWord
class EndWithWord

class Matchers:
  extension (leftSideString: String)
    def should(body: TestBody1): Unit = ()
    def should(body: TestBody2): Unit = ()

  extension [T](leftSideValue: T)
    def should(word: StartWithWord)(using T <:< String): Unit = ()
    def should(word: EndWithWord)(using T <:< String): Unit = ()

  def endWith(rightSideString: String): EndWithWord = new EndWithWord

class Test extends Matchers:
  def test(): Unit =
    "hello world" should endWith ("world")

Before the change in this PR, the above code compiles. After, it fails to compile with

-- [E008] Not Found Error: tests/pos/i20053c.scala:20:18 -----------------------
20 |    "hello world" should endWith ("world")
   |    ^^^^^^^^^^^^^^^^^^^^
   |value should is not a member of String.
   |An extension method was tried, but could not be fully constructed:
   |
   |    this.should()
   |
   |    failed with:
   |
   |        value should: <overloaded Test.this.should> does not take parameters
1 error found

It's not exactly the same error that we find in Scalactic, but I'm pretty sure it has the same root cause.

As we can see, there are 4 initial candidates for should. Based only on the first parameter list, the ones with leftSideString: String are more specific than the ones with leftSideValue: T. Since there are 2 of them, however, resolution proceeds to the second parameter list.

• Before this PR, all four candidates are carried to the second round, where overload resolution finds that the only applicable one (never mind specificity) is the one taking an EndWithWord. So resolution succeeds.
• After this PR, only the two candidates with leftSideString: String are carried. But then overload resolution finds no applicable alternative that accepts an EndWithWord, so compilation fails.

This example shows that the changes proposed here, as is, are not valid. They definitely break existing code.

It might be possible to refine the logic further, in a non-local way: keep resolving based on further parameter lists with all candidates, but after all param lists have been exhausted, look back at the combined specificity or something. Not sure we want to go there.

[EugeneFlesselle]

Great minimization ! On a very minor note, the (using T <:< String) can also be removed.

This is really just restating the same thing, but also an interesting example:

object A:
  def f(x: Any)(y: Double) = ??? // alt1
  def f(x: Int)(y: String) = ??? // alt2
  def f(x: Int)(y: Boolean) = ??? // alt3
  f(3)(4d) // ok, resolves to alt1

object B: // same as A but without alt3
  def f(x: Any)(y: Double) = ??? // alt1
  def f(x: Int)(y: String) = ??? // alt2
  f(3)(4d) // error, even though alt1 is the only applicable alternative

[soronpo]

This is really just restating the same thing, but also an interesting example:

Actually, this could be seen as expected behavior, and signatures for overloading should be matched block-by-block.
Mainly because later block signatures can be type dependent on earlier blocks.
def f[T <: Foo](x: T)(y: x.Dep) = ???

[EugeneFlesselle]

This would be another PR worth testing the impact of on the community build. @WojciechMazur could you run it based on the changes here ?

I already expect numerous errors caused by the scalatest should overload set, which would hopefully be fixed by a change in scalatest. It'd be great to have an idea of whether this is the only major source of impacted code.

[WojciechMazur]

After testing 1590 projects we have 5 new regressions when compared with 3.6.0-RC1-bin-20240819-b64afad-NIGHTLY for which 29 projects were already failing.
Tested using commit f40c609

Side note: there is a large number of projects that are now using the -source:3.5-migration or 3.4-migration version (~100). In most of these, it should no longer be needed because we can no perform automatic rewrites to replace usages of syntax that fails to compiler since 3.6. I'll retry the build after resolving this issue.

Project	Version	Build URL	Notes
fenixedu/fenixedu-scala-sdk	0.5.0	Open CB logs
fs2-blobstore/fs2-blobstore	0.9.14	Open CB logs
ist-dsi/scala-keystone-client	0.12.2	Open CB logs
ist-dsi/scala-nova-client	0.10.1	Open CB logs
laserdisc-io/mysql-binlog-stream	3.0.0	Open CB logs

I can see there are also migration warnings for the overloads. I'm collecting statistics about these warnings right now.

#Edit: There are no warnings because projects were compiled using -source:3.6-migration. The statistics need to way until a next run with forced -source:3.6 or -source:3.7-migration setting