Wrong initialization order for static fields in enums

Roshan Dawrani added a comment - 29/Nov/08 12:56 PM Hi, Attaching the patch for 1.5.8 as a partial fix. In 1.6, the same fix is resulting in some errors and I am looking into them. If I am able to resolve them, I'll update the patches. The issue is related to Verifier pushing the static enum fields defined by user to the beginning of the static init block of the enum class (before the enum values are initialized). When EnumVisitor visits an enum class node, it creates a <clinit>(static init) block in which it defines the enum values. But when Verifier visits this class node later, if it sees some static fields, it adds their AST statements to the beginning of the <clinit> block. enum GroovyColors { // AST statements for following values are first added to <clinit> by EnumVisitor red, blue, green /* Later, ExpressionStatement for this static Field gets moved by Verifier before the red/blue/green in <clinit> */ static def ALL_COLORS = [red, blue, green] } // since <clinit> has declaration of ALL_COLORS before red/blue/green are being set, instead of having the correct values, it has un-initialized null values. println GroovyColors.ALL_COLORS Here is the test case with which I have verified this partial fix: class Groovy3161Bug extends GroovyTestCase { def void testStaticEnumFieldWithEnumValues() { def allColors = GroovyColors3161.ALL_COLORS assert allColors.size == 3 assert allColors[0] == GroovyColors3161.red assert allColors[1] == GroovyColors3161.blue assert allColors[2] == GroovyColors3161.green } } enum GroovyColors3161 { red, blue, green static def ALL_COLORS = [red, blue, green] }

Roshan Dawrani added a comment - 30/Nov/08 1:52 AM Attaching the fix for versions 1.6-RC-1 / 1.7-beta-1 and a revised patch for 1.5.8. Briefly, the fix applied is only this: If there are any explicitly declared static fields inside an enum, their initialization inside the <clinit> block should not come before the initialization of the enum values. Otherwise if such a static field uses enum values, it does not see the initialized enum value. The changes have been made to org.codehaus.groovy.classgen.Verifier's addInitialization() and addFieldInitialization() methods to handle initializaton of static fields inside an enum a little differently. Hope it helps. rgds, Roshan

Roshan Dawrani added a comment - 16/Dec/08 12:15 PM Hi, If the attached patches can be reviewed, we can move forward on this issue one way or other. Thanks, Roshan

blackdrag blackdrag added a comment - 16/Dec/08 3:34 PM the patch is ok for me

Roshan Dawrani added a comment - 19/Dec/08 11:15 AM Fixed by ensuring that enum values are initialized before any static fields explicitly declared in the enum.