-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Make Java defined functions nullary when we override them #24461
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
Conversation
|
If Scala 3 is not going to add parens to the overriding definition, surely it should warn. Otherwise it's a gotcha forever. I'll contribute a warning if it is deemed useful. |
|
No need for a warning, this is only to not break source compatibility and already existing macros. Scala 2 already added those parentheses for Java defined methods. This is just porting those changes only in the Scala 2 inherited files. You see that Tuple.scala didn't change this. @sjrd I just saw that I had some unsaved changes locally that had to do with this PR, I'll include them and go over the list a second time. @som-snytt thank you for the help! |
…be nullary functions
…be nullary functions
6dc30d5 to
1f7c1aa
Compare
sjrd
left a comment
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.
Forgotten:
TupleXXL.toString()andhashCode(). (I know it was always Scala 3-only, and it's not even really public, but for consistency.)
I checked all the suspects: toString, hashCode, run, call, get, apply and getClass.
It was not really forgotten, I omitted it because it is Scala 3 only and TASTy already exist for it. Isn't this a TASTy breaking change? |
|
Fair enough. Maybe add a comment then, that it has no |
To preserve the same source compatibility, we mark
toStringandhashCodeas methods with empty parameter lists in all the inherited files from Scala 2. Note that I checked for other java defined methods that would qualify for this change but I couldn't find any (reviewer should double check too since we cannot fix this after 3.8.0).Closes #19616
Supersedes #24452