Skip to content

Commit

Permalink
Drop special treatment of function types in overloading resolution (#…
Browse files Browse the repository at this point in the history
…19654)

Fixes #19641 

How we got here:

Originally, overloading resolution for types that were not applied was
handled like this:
```scala
      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.
  • Loading branch information
odersky authored Feb 12, 2024
2 parents a0ea484 + 0337efd commit c529a48
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 28 deletions.
49 changes: 21 additions & 28 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2123,34 +2123,27 @@ trait Applications extends Compatibility {
else resolveMapped(alts1, _.widen.appliedTo(targs1.tpes), pt1)

case pt =>
val compat0 = pt.dealias match
case defn.FunctionNOf(args, resType, _) =>
narrowByTypes(alts, args, resType)
case _ =>
Nil
if (compat0.isEmpty) then
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
else compat0
val compat = alts.filterConserve(normalizedCompatible(_, pt, keepConstraint = false))
if compat.isEmpty then
pt match
case SAMType(mtp, _) =>
// If we have a SAM type as expected type, treat it as if the expression was eta-expanded
// Note 1: No need to do that for function types, the previous normalizedCompatible test already
// handles those.
// Note 2: This case should not be moved to the enclosing match
// since fSAM types 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)
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
}

/** The type of alternative `alt` after instantiating its first parameter
Expand Down
17 changes: 17 additions & 0 deletions tests/run/i19641.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
type DiagnosticConsturctor = (Int) => DiagnosticSet

final class Diagnostic

final class DiagnosticSet(elements: List[Diagnostic] = List())

enum Result:
case Success extends Result
case Failure(diagnose: DiagnosticConsturctor) extends Result
def diagnose(n: Int): DiagnosticSet =
this match
case Success => DiagnosticSet()
case Failure(d) => d(n)

@main def Test(): Unit =
val r : Result = Result.Failure((n) => DiagnosticSet(List(Diagnostic())))
r.diagnose(1)
2 changes: 2 additions & 0 deletions tests/run/i4364a.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ object Test {
def f(x: String): Unit = ()

def foo(c: Consumer[String]) = c.accept("")
def bar(c: String => Unit) = c("")

def main(args: Array[String]) = {
foo(f)
bar(f)
}
}

0 comments on commit c529a48

Please sign in to comment.