Port some ClassfileParser changes from scalac #2229
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
Adapted from scalac commit 2a19cd56258884e25f26565d7b865cc2ec931b23 by Jason Zaugg, but without the testing infrastructure added: A classfile in the wild related to Vaadin lacked the InnerClasses attribute. As such, our class file parser treated a nested enum class as top-level, which led to a crash when trying to find its linked module. More details of the investigation are available in the JIRA comments. The test introduces a new facility to rewrite classfiles. This commit turns this situation into a logged warning, rather than crashing. Code by paulp, test by yours truly.
Adapted from scalac commit 050b4c951c838699c2fe30cbf01b63942c63a299 by Jason Zaugg: Java synthesizes public constructors in private classes to allow access from inner classes. The signature of that synthetic constructor (known as a "access constructor") has a dummy parameter appended to avoid overloading clashes. javac chooses the type "Enclosing$1" for the dummy parameter (called the "access constructor tag") which is either an existing anonymous class or a synthesized class for this purpose. In OpenJDK, this transformation is performed in: langtools/src/share/classes/com/sun/tools/javac/comp/Lower.java (Incidentally, scalac would just emits a byte-code public constructor in this situation, rather than a private constructor / access constructor pair.) Scala parses the signature of the access contructor, and drops the $outer parameter, but retains the dummy parameter. This causes havoc when it tries to parse the bytecode for that anonymous class; the class file parser doesn't have the enclosing type parameters of Vector in scope and crash ensues. In any case, we shouldn't allow user code to see that constructor; it should only be called from within its own compilation unit. This commit drops the dummy parameter from access constructor signatures in class file parsing.
Adapted from scalac commit 3c5990ce5839f4bdfca8fed7f2c415a72f6a8bd8 by Som Snytt: Use DataInputStream.readUTF to read CONSTANT_Utf8_info. This fixes reading embedded null char and supplementary chars.
smarter
force-pushed
the
sync-classfile-parser
branch
from
5b1a714 to
65f438e
Compare
odersky
approved these changes
Apr 12, 2017
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.
Most of the remaining relevant changes are related to being more tolerant of missing classfiles by using stubs and are thus a bit harder to port (and their tests need to be adapted to our testing framework). There is also support for named Java method parameters that I haven't tried to implement: scala/scala@c78d771 scala/scala@954ea7e