Skip to content

Commit d73932d

Browse files
committed
Generate static forwarders for object members in companion interface
We used to disable generation of static forwarders when a object had a trait as a companion, as one could not add methods with bodies to an interface in JVM 6. The JVM lifted this restriction to support default methods in interfaces, so we can lift the restriction on static forwarders, too. I've only commited the test using the default backend (GenBCode), but I've also made the change and manaully verified the test works under the soon-to-be-removed GenASM. ``` % qscalac -Ybackend:GenASM test/files/run/trait-static-forwarder/forwarders.scala && javac -d . -classpath . test/files/run/trait-static-forwarder/Test.java && qscala Test ./T.class: warning: Cannot find annotation method 'bytes()' in type 'ScalaSignature': class file for scala.reflect.ScalaSignature not found 1 warning 42 ``` Fixes scala/scala-dev#59 I had to disable use of the lambda deserialization refletion cache when in interfaces, as this change revealed a latent design issue with our lambda deserialization approach. Disabling the cache isn't a correctness problem, but hampers performance.
1 parent c99e53e commit d73932d

File tree

6 files changed

+24
-5
lines changed

6 files changed

+24
-5
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ abstract class BCodeSkelBuilder extends BCodeHelpers {
180180

181181
} else {
182182

183-
val skipStaticForwarders = (claszSymbol.isInterface || settings.noForwarders)
183+
val skipStaticForwarders = settings.noForwarders
184184
if (!skipStaticForwarders) {
185185
val lmoc = claszSymbol.companionModule
186186
// add static forwarders if there are no name conflicts; see bugs #363 and #1735

src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1304,7 +1304,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters { self =>
13041304
for (constructor <- c.lookupStaticCtor) {
13051305
addStaticInit(Some(constructor))
13061306
}
1307-
val skipStaticForwarders = (c.symbol.isInterface || settings.noForwarders)
1307+
val skipStaticForwarders = settings.noForwarders
13081308
if (!skipStaticForwarders) {
13091309
val lmoc = c.symbol.companionModule
13101310
// add static forwarders if there are no name conflicts; see bugs #363 and #1735

src/compiler/scala/tools/nsc/backend/jvm/analysis/BackendUtils.scala

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
6464
* return scala.runtime.LambdaDeserializer.deserializeLambda(MethodHandles.lookup(), cache, l);
6565
* }
6666
*/
67+
// TODO the cache can't be emitted if we're in an interface, is it acceptable to pay for a method handle
68+
// lookup for each call? We could use a more global cache (e.g. in LambdaDeserializer), but then we have to
69+
// worry about using weak references to avoid classloader leaks.
6770
def addLambdaDeserialize(classNode: ClassNode): Unit = {
6871
val cw = classNode
6972
import scala.tools.asm.Opcodes._
@@ -76,16 +79,21 @@ class BackendUtils[BT <: BTypes](val btypes: BT) {
7679
// This is fine, even if `visitInnerClass` was called before for MethodHandles.Lookup: duplicates are not emitted.
7780
cw.visitInnerClass("java/lang/invoke/MethodHandles$Lookup", "java/lang/invoke/MethodHandles", "Lookup", ACC_PUBLIC + ACC_FINAL + ACC_STATIC)
7881

79-
{
82+
val isInterface = (classNode.access | ACC_INTERFACE) != 0
83+
if (isInterface){
8084
val fv = cw.visitField(ACC_PRIVATE + ACC_STATIC + ACC_SYNTHETIC, "$deserializeLambdaCache$", "Ljava/util/Map;", null, null)
8185
fv.visitEnd()
8286
}
8387

8488
{
8589
val mv = cw.visitMethod(ACC_PRIVATE + ACC_STATIC + ACC_SYNTHETIC, "$deserializeLambda$", "(Ljava/lang/invoke/SerializedLambda;)Ljava/lang/Object;", null, null)
8690
mv.visitCode()
87-
// javaBinaryName returns the internal name of a class. Also used in BTypesFromsymbols.classBTypeFromSymbol.
88-
mv.visitFieldInsn(GETSTATIC, classNode.name, "$deserializeLambdaCache$", "Ljava/util/Map;")
91+
if (isInterface)
92+
mv.visitInsn(ACONST_NULL)
93+
else {
94+
// javaBinaryName returns the internal name of a class. Also used in BTypesFromsymbols.classBTypeFromSymbol.
95+
mv.visitFieldInsn(GETSTATIC, classNode.name, "$deserializeLambdaCache$", "Ljava/util/Map;")
96+
}
8997
mv.visitVarInsn(ASTORE, 1)
9098
mv.visitVarInsn(ALOAD, 1)
9199
val l0 = new Label()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
42
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
public final class Test {
2+
public static void main(String... args) {
3+
System.out.println(T.foo());
4+
}
5+
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
trait T
2+
3+
object T {
4+
def foo = 42
5+
}

0 commit comments

Comments
 (0)