-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Construct ClassBTypes from parsed classfiles #4210
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
5f3e91d to
e4416ad
Compare
|
|
|
I'll be offline until January 7th. If a review is needed before that time, please reassign. Marry Christmas and Happy New Year! 🎄 🎁 🎆 |
3bbf6f0 to
e4416ad
Compare
|
I've went through the whole code. Apart from the few nitpicks, it all looks good. When addressing the feedback please make sure to rebase. The PR doesn't merge cleanly anymore. |
Each ClassBType is identified by its internalName, the fully qualified JVM class name. Before this change, the name was stored in the `chrs` array of the compiler name table (hash consed), with the idea to avoid materializing the string. However, we materialize the string anyway, because each ClassBType is stored in the classBTypeFromInternalNameMap, indexed by the string. If string equality turns out to be too slow we can use interning. For the inliner, we read classes from bytecode and create ClassBTypes for them. The names of these classes would not yet exist in the name table, so the backend would need to be able to create new names. Using Strings removes this dependency.
There's already the map classBTypeFromInternalNameMap in BTypes which stores all ClassBTypes.
Introduces methods for textifying classes, methods, InsnLists and individual AbstractInsnNodes.
This infrastructure is required for the inliner: when inlining code
from a classfile, the corresponding ClassBType is needed for various
things (eg access checks, InnerClass attribute).
The test creates two ClassBTypes for the same class: once using the
(unpickled) Symbol, once using the parsed ASM ClassNode, and verifies
that the two are the same.
There's a cleanup to the InnerClass attribute:
object T { class Member; def foo = { class Local } }
class T
For Java compatibility the InnerClass entry for Member says the class
is nested in T (not in the module class T$). We now make sure to add
that entry only to T, not to T$ (unless Member is actually referenced
in the classfile T$, in that case it will be added, as required).
e4416ad to
8c270de
Compare
- Rename CodeRepository to ByteCodeRepository - Scaladoc on OptimizerReporting - Scaladoc on ByteCodeRepository
8c270de to
f3f9eb4
Compare
|
Thanks for the review, @gkossakowski! Rebased on 2.11.x, reworded some commit messages, added one new commit that addresses the remaining feedback. |
|
LGTM Great work! |
Construct ClassBTypes from parsed classfiles
See individual commit messages.