-
Notifications
You must be signed in to change notification settings - Fork 21
Overloading Resolution + Names / Defaults + Varargs + Auto-Tupling + Spec #8342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Imported From: https://issues.scala-lang.org/browse/SI-8342?orig=1 |
@som-snytt said: |
@som-snytt said (edited on Nov 18, 2015 6:19:52 PM UTC): |
@som-snytt said: Example of losing that by introducing SAM conversion: #9796 The question is whether the shape test ought to trump samification. Or rather, I guess that question is settled; but should it still be called the shape test? The shape/sam test? The shame test. |
We recntly merged scala#9601 which unified our handling of `Object` coming from Java methods, an unintended consequence of that change is that some existing Java APIs can no longer be called without running into ambiguity errors, for example log4j defines two overloads for `Logger.error`: (x: String, y: Object): Unit (x: String, y: Object*): Unit previously we translated `Object` to `Any` but left `Object*` alone, now they're both treated the same way (translated to a special alias of `Object`) and so neither method ends up being more specific than the other, so `error("foo: {}, 1)` is now ambiguous. Clearly the problem lies with how we handle varargs in overloading resolution, but this has been a source of issues for years with no clear resolution: - scala/bug#8342 - scala/bug#4728 - scala/bug#8344 - scala#6230 This PR cuts the Gordian knot by simply declaring that non-varargs methods are always more specific than varargs. This has several advantages: - It's an easy rule to remember - It matches what Java does (see https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2) - It avoids unnecessary wrapping of arguments The downside is that it doesn't match what Scala 2 does, but our current behavior isn't a perfect match either (also it seems that Scala 2 handles Java varargs and Scala varargs differently in overloading resolution which is another source of complexity best avoided, see `tests/run/overload_repeated`).
We recently merged scala#9601 which unified our handling of `Object` coming from Java methods, an unintended consequence of that change is that some existing Java APIs can no longer be called without running into ambiguity errors, for example log4j defines two overloads for `Logger.error`: (x: String, y: Object): Unit (x: String, y: Object*): Unit previously we translated `Object` to `Any` but left `Object*` alone, now they're both treated the same way (translated to a special alias of `Object`) and so neither method ends up being more specific than the other, so `error("foo: {}, 1)` is now ambiguous. Clearly the problem lies with how we handle varargs in overloading resolution, but this has been a source of issues for years with no clear resolution: - scala/bug#8342 - scala/bug#4728 - scala/bug#8344 - scala#6230 This PR cuts the Gordian knot by simply declaring that non-varargs methods are always more specific than varargs. This has several advantages: - It's an easy rule to remember - It matches what Java does (see https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2) - It avoids unnecessary wrapping of arguments The downside is that it doesn't match what Scala 2 does, but our current behavior isn't a perfect match either (also it seems that Scala 2 handles Java varargs and Scala varargs differently in overloading resolution which is another source of complexity best avoided, see `tests/run/overload_repeated`).
We recently merged scala#9601 which unified our handling of `Object` coming from Java methods, an unintended consequence of that change is that some existing Java APIs can no longer be called without running into ambiguity errors, for example log4j defines two overloads for `Logger.error`: (x: String, y: Object): Unit (x: String, y: Object*): Unit previously we translated `Object` to `Any` but left `Object*` alone, now they're both treated the same way (translated to a special alias of `Object`) and so neither method ends up being more specific than the other, so `error("foo: {}, 1)` is now ambiguous. Clearly the problem lies with how we handle varargs in overloading resolution, but this has been a source of issues for years with no clear resolution: - scala/bug#8342 - scala/bug#4728 - scala/bug#8344 - scala#6230 This PR cuts the Gordian knot by simply declaring that non-varargs methods are always more specific than varargs. This has several advantages: - It's an easy rule to remember - It matches what Java does (see https://docs.oracle.com/javase/specs/jls/se8/html/jls-15.html#jls-15.12.2) - It avoids unnecessary wrapping of arguments The downside is that it doesn't match what Scala 2 does, but our current behavior isn't a perfect match either (also it seems that Scala 2 handles Java varargs and Scala varargs differently in overloading resolution which is another source of complexity best avoided, see `tests/run/overload_repeated`). Fixes scala#9688, supercedes scala#6230.
This ticket collects multiple issues around overloading resolution. See also comments in scala/scala#3562, scala/scala#3583.
Auto-tupling is not mentioned at all in the spec. One could argue that the following violates the spec: both alternatives are applicable, but the compiler picks the one that requires using a default argument, even though these are supposed to be eliminated if there are multiple alternatives.
Maybe a better way to spec it would be to ignore auto-tupling for overloading resolution and say that auto-tupling is tried last, when nothing else works. This is probably what the implementation does: the first alternative is not applicable (if you don't have auto-tupling), the second is.
Overloading resolution in the spec starts by selecting alternatives that are "applicable to expressions (e1 ... en) of types (shape(e1) ... shape(en)". It seems to me this is an implementation detail that is not needed for the spec, because afterwards we test if the alternatives are applicable to expressions (e1 ... en) of types (S1 .. Sn).
Also, the current implementation seems to ignore the vararg-marker (:_*) when testing shape matches. This is probably safe (more alternatives are kept at this stage), but not according to the (IMO unnecessary) part of the spec.
The spec of overloading resolution is incomplete when it comes to interpreting an argument
x = e
, which can be an assignment or a named argument.The current implementation has a bug when selecting the most specific alternative if a setter does not return Unit:
A different problem is that the current implementation, applicable alternatives are filtered based on parameter names. For each argument expression
x = e
, only those alternatives defining a parameterx
are kept. This fixes #4592, but it disallows interpreting the argument as assignment. For example:One idea to fix this: in doTypedApply, type-check the argument expression
x = e
in a silent context and to find out if the expression would be a valid assignment. If so, store its type (does not have to be Unit, due to setters) in the NamedType. Overloading resolution would then know if the expression is a valid assignment.Alternative solution: Decide if
x = e
is an assignment or a named argument purely based on the syntax. It seems that is what dotty does. Thread on scala internalsThe text was updated successfully, but these errors were encountered: