From 53c7beb345e223aa0bff186dd0552c3eac0e6dae Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 3 Aug 2016 23:32:42 -0700 Subject: [PATCH] Tweak module initialization to comply with JVM spec Dmitry learned that we've been relying on a bug in the verifier that will be fixed in JDK 9 under the new classfile format. Assignment to a static final must occur lexically within the . We were performing this assignment from the constructor of the module class. This commit moves the assignment to . Before: ``` public final class O$ { public static final O$ MODULE$; public static {}; Code: 0: new #2 // class O$ 3: invokespecial #12 // Method "":()V 6: return private O$(); Code: 0: aload_0 1: invokespecial #13 // Method java/lang/Object."":()V 4: aload_0 5: putstatic #15 // Field MODULE$:LO$; 8: return } ``` After: ``` public final class O$ { public static final O$ MODULE$; public static {}; Code: 0: new #2 // class O$ 3: dup 4: invokespecial #12 // Method "":()V 7: putstatic #14 // Field MODULE$:LO$; 10: return private O$(); Code: 0: aload_0 1: invokespecial #15 // Method java/lang/Object."":()V 4: return } ``` The difference is observable is the constructor is called a second time with reflection/setAccessible. That used to overwrite the `MODULE$` field. I think few would argue that the new semantics are a clear improvement. --- .../scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala | 7 ------- .../scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala | 2 ++ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala index d5c4b5e20161..7f10798e2ebb 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeBodyBuilder.scala @@ -563,13 +563,6 @@ abstract class BCodeBodyBuilder extends BCodeSkelBuilder { fun.symbol.javaSimpleName.toString == INSTANCE_CONSTRUCTOR_NAME && isStaticModuleClass(claszSymbol)) { isModuleInitialized = true - mnode.visitVarInsn(asm.Opcodes.ALOAD, 0) - mnode.visitFieldInsn( - asm.Opcodes.PUTSTATIC, - thisBType.internalName, - strMODULE_INSTANCE_FIELD, - thisBType.descriptor - ) } } // 'super' call: Note: since constructors are supposed to diff --git a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala index 1bff8519eca3..22c361836590 100644 --- a/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala +++ b/src/compiler/scala/tools/nsc/backend/jvm/BCodeSkelBuilder.scala @@ -217,8 +217,10 @@ abstract class BCodeSkelBuilder extends BCodeHelpers { /* "legacy static initialization" */ if (isCZStaticModule) { clinit.visitTypeInsn(asm.Opcodes.NEW, thisBType.internalName) + clinit.visitInsn(asm.Opcodes.DUP) clinit.visitMethodInsn(asm.Opcodes.INVOKESPECIAL, thisBType.internalName, INSTANCE_CONSTRUCTOR_NAME, "()V", false) + clinit.visitFieldInsn(asm.Opcodes.PUTSTATIC, thisBType.internalName, strMODULE_INSTANCE_FIELD, thisBType.descriptor) } if (isCZParcelable) { legacyAddCreatorCode(clinit, cnode, thisBType.internalName) } clinit.visitInsn(asm.Opcodes.RETURN)