-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix vararg overloading #9732
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
Fix vararg overloading #9732
Conversation
Alternative to #9718. It solves the original issue in a less drastic way and keeps compatibility with Scala 2 for all tests we have. |
I now realize I miswrote A_1.foo1, it was supposed to test the same thing as bar1: class A_1 {
public static int foo1(Object x) { return 1; }
public static int foo1(String... x) { return 2; }
} With thsi PR, this is now ambiguous: -- [E051] Reference Error: tests/run/overload_repeated/B_2.scala:21:15 ---------
21 | assert(A_1.foo1("") == 1) // Same as in Java and Scala 2
| ^^^^^^^^
|Ambiguous overload. The overloaded alternatives of method foo1 in object A_1 with types
| (x: String*): Int
| (x: Object): Int
|both match arguments (("" : String)) Scala 2 has the same issue, this can only be fixed by either going with #9718 or special-casing Java varargs. |
If my understanding of the situation is correct, using "ASA" to stand for "as specific as"
So the only difference between the two is that in this PR we can be in a situation where we have a vararg and a non-vararg alternative, neither of which is ASA the other, resulting in ambiguous overloads, whereas in #9718 the non-vararg would be picked. Is this accurate or are there situation where this PR would pick a different overload than #9718? |
I believe the biggest win of this over #9718 is that it does not create new special cases.
Plus all the interactions with other rules (e.g t8197.scala, which flipped in behavior with #9718). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't exactly what I intended when I proposed the rules, but after seeing the examples and thinking all the approaches through I'd say that this change is arguably the most intuitive. At least, it is more in line with the spirit of the spec than the other changes: an overload is selected as the most specific if all calls to it can also be calls to another overload, but not vice versa.
I'd like to point out (and be corrected if I'm wrong) that if we have two overloads, one of which has varargs, and neither is more specific than another then we should always be able to explicitly pick one of them, either by upcasting an argument or adding a repeated argument annotation. Specifically, in the foo(a: Any) / foo(s: String*)
case, we can explicitly select either of the overloads.
Refine/clarify the rule when a method is applicable to a vararg parameter list. Test cases adapted from scala#9718 which used a different approach.
07b7c26
to
ff7e342
Compare
I've updated the commit in this PR to fix my test to have the correct signature for |
Following up @abgruszecki this is different from 2.13.3:
It's not disallowing
|
Yeah we should open an issue for that. |
…ions syntax - Dotty 0.27.0-RC1 broke logging calls due to [Ambiguous overload between varargs and non-varargs #9688](scala/scala3#9688) which was fixed in [Fix vararg overloading #9732](scala/scala3#9732) - Remove a few remaining extension methods that still used an 'older' extension mothod syntax
…ions syntax - Dotty 0.27.0-RC1 broke logging calls due to [Ambiguous overload between varargs and non-varargs #9688](scala/scala3#9688) which was fixed in [Fix vararg overloading #9732](scala/scala3#9732) - Remove a few remaining extension methods that still used an 'older' extension mothod syntax
* Bump Dotty version to Scala 3.0.0-M1/Remove workaround/Correct extensions syntax - Dotty 0.27.0-RC1 broke logging calls due to [Ambiguous overload between varargs and non-varargs #9688](scala/scala3#9688) which was fixed in [Fix vararg overloading #9732](scala/scala3#9732) - Remove a few remaining extension methods that still used an 'older' extension mothod syntax * Update exercise instructions - Exercise on extentions still used a syntax that has since then been changed - Use indentation based syntax in instructions
Refine/clarify the rule when a method is applicable to a vararg parameter list.