backend: Emit calls using the correct receiver #8499
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lrytz
approved these changes
Mar 10, 2020
There was a problem hiding this comment.
looks good from my side
In the compiler we pretend that Java annotations are classes
see also scala/scala#6869
smarter
force-pushed
the
backend-call-receiver
branch
from
62968c1 to
a1f475d
Compare
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 11, 2020
These definitions are not supposed to be called from user code, so creating symbols for them is unnecessary. In particular, this fixes various issues with bridges: - Fix scala#6152: We require `override` to implement bridged abstract method - Fix scala#8435: Generic signature is missing for bridged method (This PR is only possible thanks to scala#8499, without this we would use non-bridge symbols as receivers and get IllegalAccessErrors in various cases). Note that this is slightly different from what Scala 2 does: `Type#member` in scalac excludes symbols with the Bridge flag set by default, we can't copy that behavior without more changes on our end since member filtering works differently in both compiler: in Scala 2 if a symbol with an excluded flag overrides a symbol without the flag, the overridden symbol is returned, whereas no symbol is returned in Dotty. In any case, I think that simply not creating the symbols is a more systematic solution, it also makes separate compilation of .java and .scala files work more like joint compilation since under joint compilation we never see the synthetic members generated by javac anyway, this reduces the risk of bugs which appear only under one kind of compilation and not the other.
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 11, 2020
These definitions are not supposed to be called from user code, so creating symbols for them is unnecessary. In particular, this fixes various issues with bridges: - Fix scala#6152: We require `override` to implement bridged abstract method - Fix scala#8435: Generic signature is missing for bridged method (This PR is only possible thanks to scala#8499, without this we would use non-bridge symbols as receivers and get IllegalAccessErrors in various cases). Note that this is slightly different from what Scala 2 does: `Type#member` in scalac excludes symbols with the Bridge flag set by default, we can't copy that behavior without more changes on our end since member filtering works differently in both compiler: in Scala 2 if a symbol with an excluded flag overrides a symbol without the flag, the overridden symbol is returned, whereas no symbol is returned in Dotty. In any case, I think that simply not creating the symbols is a more systematic solution, it also makes separate compilation of .java and .scala files work more like joint compilation since under joint compilation we never see the synthetic members generated by javac anyway, this reduces the risk of bugs which appear only under one kind of compilation and not the other.
anatoliykmetyuk
approved these changes
Mar 12, 2020
- Remove simpleName since it's equivalent to name - Remove unused fullName(sep: Char) - Rename fullName to showFullName since it's only used for logging statements
Will be used in the next commit.
This is a port of scala/scala@765eb29: When emitting a virtual call, the receiver in the bytecode cannot just be the method's owner (the class in which it is declared), because that class may not be accessible at the callsite. Instead we use the type of the receiver. This was basically done to fix - aladdin bug 455 (9954eaf) - SI-1430 (0bea2ab) - basically the same bug, slightly different - SI-4283 (8707c9e) - the same for field reads In Dotty, we rarely encountered this issue because under separate compilation, we see the bridge symbols generated by javac and use their owner as the receiver, but under joint compilation of .java and .scala sources this doesn't happen, in particular this commit fixes scala#6546. It also means that we should now be able to stop creating symbols in the ClassfileParser for Java bridges, this would make joint and separate compilation more similar and thus should reduce the number of bugs which appear in one but not the other as discussed in scala#6266. After this commit, some more changes are necessary to get the updated backend to work correctly with dotty, these are implemented in the later commits of this PR. Co-Authored-By: Lukas Rytz <[email protected]>
MoveStatics moves some of these definitions from the companion to the corresponding class, but does not update the trees referencing these definitions, so the qualifier type cannot be used as a receiver. I think we need to seriously rethink how we handle static methods in the compiler, but that would be a huge refactoring, so this commit just patches the backend to special case `@static` methods to use the method owner as receiver as was always done before this PR.
In the compiler we pretend that Java annotations are classes, but they're really interfaces from the JVM point of view, so we need to treat them as such in the backend when calling a method on them, otherwise we'll emit invalid bytecode. Needed to get tests/run/i2760 to pass after the backend changes in this PR.
Previously we returned NoSymbol which does not match what the backend now expects.
smarter
force-pushed
the
backend-call-receiver
branch
from
a1f475d to
4fa3eb2
Compare
smarter
added a commit
to dotty-staging/dotty
that referenced
this pull request
Mar 12, 2020
These definitions are not supposed to be called from user code, so creating symbols for them is unnecessary. In particular, this fixes various issues with bridges: - Fix scala#6152: We require `override` to implement bridged abstract method - Fix scala#8435: Generic signature is missing for bridged method (This PR is only possible thanks to scala#8499, without this we would use non-bridge symbols as receivers and get IllegalAccessErrors in various cases). Note that this is slightly different from what Scala 2 does: `Type#member` in scalac excludes symbols with the Bridge flag set by default, we can't copy that behavior without more changes on our end since member filtering works differently in both compiler: in Scala 2 if a symbol with an excluded flag overrides a symbol without the flag, the overridden symbol is returned, whereas no symbol is returned in Dotty. In any case, I think that simply not creating the symbols is a more systematic solution, it also makes separate compilation of .java and .scala files work more like joint compilation since under joint compilation we never see the synthetic members generated by javac anyway, this reduces the risk of bugs which appear only under one kind of compilation and not the other.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.