Avoid name check when loading repl products #10253
Conversation
|
The stack trace in the test is jdk-specific. I will just remove that line; it showed that the loadClass fails the second time, but of course it does. |
som-snytt
force-pushed
the
issue/12705-repl-reflective-class-load
branch
from
6b755e1
to
70862c4
Compare
| private class TranslatingClassLoader(parent: ClassLoader) extends AbstractFileClassLoader(replOutput.dir, parent) { | ||
| override protected def findAbstractFile(name: String): AbstractFile = super.findAbstractFile(name) match { | ||
| case null if _initializeComplete => translateSimpleResource(name).map(super.findAbstractFile).orNull | ||
| case file => file | ||
| } | ||
| // if the name was mapped by findAbstractFile, supply null name to avoid name check in defineClass | ||
| override protected def findClass(name: String): Class[_] = { |
There was a problem hiding this comment.
do super.findClass first and use the new code below only as a fallback?
There was a problem hiding this comment.
I considered doing something fancy, but the class loader API is so complex already, plus ScalaClassLoader and AbstractFileClassLoader, and also throws, that I wasn't "up" for it.
I could imagine a boolean member called verifyDefiningName for AbstractFileClassLoader to consult.
There was a problem hiding this comment.
A complex mix of complex ingredients, so I'm just generally worried about breaking stuff... I just saw now the AbstractFileClassLoader.findClass code that's overridden. In that light, LGTM.
Supplying the name to
defineClassmeans the name is checked, but if REPL found the class for a "translated" name, the name will never match. So REPL's class loader suppliesnullto mean DK. Some extra code could be added to preserve the translation and the name check, but for the REPL that might be unnecessary.Fixes scala/bug#12705