Drop special treatment of function types in overloading resolution

[odersky]

Fixes #19641

How we got here:

Originally, overloading resolution for types that were not applied was handled like this:

      case defn.FunctionOf(args, resultType, _) =>
        narrowByTypes(alts, args, resultType)

      case pt =>
        val compat = alts.filterConserve(normalizedCompatible(_, pt, keepConstraint = false))
        if (compat.isEmpty)
          /*
           * the case should not be moved to the enclosing match
           * since SAM type must be considered only if there are no candidates
           * For example, the second f should be chosen for the following code:
           *   def f(x: String): Unit = ???
           *   def f: java.io.OutputStream = ???
           *   new java.io.ObjectOutputStream(f)
           */
          pt match {
            case SAMType(mtp, _) =>
              narrowByTypes(alts, mtp.paramInfos, mtp.resultType)
            case _ =>
              // pick any alternatives that are not methods since these might be convertible
              // to the expected type, or be used as extension method arguments.
              val convertible = alts.filterNot(alt =>
                  normalize(alt, IgnoredProto(pt)).widenSingleton.isInstanceOf[MethodType])
              if convertible.length == 1 then convertible else compat
          }
        else compat

Note the warning comment that the case for SAM types should not be moved out, yet we do exactly the same thing for plain function types. I believe this was simply wrong, but it was not discovered in a test.

Then in #16507 we changed the defn.FunctionOf extractor so that aliases of function types were matched by it. This triggered test failures since we now hit the wrong case with aliases of function types.

In #18286, we moved the extractor test around, but that was not enough, as #19641 shows. Instead the test for FunctionOf should be aligned with the test for SAM case. But it turns out that's not even necessary since the
preceding val compat = ... handles function prototypes correctly by simulating an eta expansion. So in the end
we could simply delete the problematic case.

[odersky]

@jchyb ping for review. It would be good if we got this into 3.4.1, but that means it has to be merged today. /cc @Kordyjan

[jchyb]

Apologies for the very late review (and for not replying quickly in the related threads).

When working on #18286 I initially tried to just remove the FunctionOf case, but I discovered that doing only that led to an undesirable change in runtime behavior (which I should have documented somewhere, I apologize for that), so I added some (removed in this PR) workarounds to remedy them. The change can be observed in the snippet below (with additional discussion here):

def f(s: Int): Unit = println("a")
def f: Int => Unit = _ => println("b")

@main def Test =
  val test: Int => Unit = f
  test(0)

I tested this PR with the above snippet, and sure enough, previously it printed "a", after this PR it prints "b". Apparently it was also "b" in scala 2, and "b" makes slightly more sense, so personally I am fine with this change, but I don't think I should have a say in approving runtime changing behavior. @Kordyjan what do you think? Perhaps we should delay 3.4.1? Or is this fine? If this change in behavior is not a problem, I can of course approve this PR.

[odersky]

"b" is correct and as specified, and since it is also Scala 2's behavior, we should definitely classify "a" as a bug. Maybe backport to LTS at some point, so that we don't get differing behavior on LTS on the one hand and Scala 2, Scala 3.4 on the other hand. But the backport is not so urgent, since I don't think many people expected "a" anyway. On the other hand, #19641 is a pretty bad bug that needs to be fixed ASAP.

I think we should merge today then it makes it into 3.4.1 tomorrow.