Skip to content

Commit

Permalink
Tweak module initialization to comply with JVM spec
Browse files Browse the repository at this point in the history
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 <clinit>. We were performing this assignment
from the constructor of the module class.

This commit moves the assignment to <clinit>.

Before:

```
public final class O$ {
  public static final O$ MODULE$;

  public static {};
    Code:
       0: new           #2                  // class O$
       3: invokespecial #12                 // Method "<init>":()V
       6: return

  private O$();
    Code:
       0: aload_0
       1: invokespecial #13                 // Method java/lang/Object."<init>":()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 "<init>":()V
       7: putstatic     #14                 // Field MODULE$:LO$;
      10: return

  private O$();
    Code:
       0: aload_0
       1: invokespecial #15                 // Method java/lang/Object."<init>":()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.
  • Loading branch information
retronym committed Aug 4, 2016
1 parent 25b29ea commit 53c7beb
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 53c7beb

Please sign in to comment.