7176515: ExceptionInInitializerError for an enum with multiple switch statements

8299760: ExceptionInInitializerError for an enum with multiple switch statements, follow-up

Reviewed-by: vromero

Author: archiecobbs

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java

@@ -227,6 +227,34 @@ JCClassDecl classDef(ClassSymbol c) {
         return def;
     }
 
+    /**
+     * Get the enum constants for the given enum class symbol, if known.
+     * They will only be found if they are defined within the same top-level
+     * class as the class being compiled, so it's safe to assume that they
+     * can't change at runtime due to a recompilation.
+     */
+    List<Name> enumNamesFor(ClassSymbol c) {
+
+        // Find the class definition and verify it is an enum class
+        final JCClassDecl classDef = classDef(c);
+        if (classDef == null ||
+            (classDef.mods.flags & ENUM) == 0 ||
+            (types.supertype(currentClass.type).tsym.flags() & ENUM) != 0) {
+            return null;
+        }
+
+        // Gather the enum identifiers
+        ListBuffer<Name> idents = new ListBuffer<>();
+        for (List<JCTree> defs = classDef.defs; defs.nonEmpty(); defs=defs.tail) {
+            if (defs.head.hasTag(VARDEF) &&
+                (((JCVariableDecl) defs.head).mods.flags & ENUM) != 0) {
+                JCVariableDecl var = (JCVariableDecl)defs.head;
+                idents.append(var.name);
+            }
+        }
+        return idents.toList();
+    }
+
     /** A hash table mapping class symbols to lists of free variables.
      *  accessed by them. Only free variables of the method immediately containing
      *  a class are associated with that class.
@@ -427,14 +455,62 @@ List<VarSymbol> freevars(ClassSymbol c)  {
     Map<TypeSymbol,EnumMapping> enumSwitchMap = new LinkedHashMap<>();
 
     EnumMapping mapForEnum(DiagnosticPosition pos, TypeSymbol enumClass) {
-        EnumMapping map = enumSwitchMap.get(enumClass);
-        if (map == null)
-            enumSwitchMap.put(enumClass, map = new EnumMapping(pos, enumClass));
-        return map;
+
+        // If enum class is part of this compilation, just switch on ordinal value
+        if (enumClass.kind == TYP) {
+            final List<Name> idents = enumNamesFor((ClassSymbol)enumClass);
+            if (idents != null)
+                return new CompileTimeEnumMapping(idents);
+        }
+
+        // Map identifiers to ordinal values at runtime, and then switch on that
+        return enumSwitchMap.computeIfAbsent(enumClass, ec -> new RuntimeEnumMapping(pos, ec));
     }
 
-    /** This map gives a translation table to be used for enum
-     *  switches.
+    /** Generates a test value and corresponding cases for a switch on an enum type.
+     */
+    interface EnumMapping {
+
+        /** Given an expression for the enum value's ordinal, generate an expression for the switch statement.
+         */
+        JCExpression switchValue(JCExpression ordinalExpr);
+
+        /** Generate the switch statement case value corresponding to the given enum value.
+         */
+        JCLiteral caseValue(VarSymbol v);
+
+        default void translate() {
+        }
+    }
+
+    /** EnumMapping using compile-time constants. Only valid when compiling the enum class itself,
+     *  because otherwise the ordinals we use could become obsolete if/when the enum class is recompiled.
+     */
+    class CompileTimeEnumMapping implements EnumMapping {
+
+        final List<Name> enumNames;
+
+        CompileTimeEnumMapping(List<Name> enumNames) {
+            Assert.check(enumNames != null);
+            this.enumNames = enumNames;
+        }
+
+        @Override
+        public JCExpression switchValue(JCExpression ordinalExpr) {
+            return ordinalExpr;
+        }
+
+        @Override
+        public JCLiteral caseValue(VarSymbol v) {
+            final int ordinal = enumNames.indexOf(v.name);
+            Assert.check(ordinal != -1);
+            return make.Literal(ordinal);
+        }
+    }
+
+    /** EnumMapping using run-time ordinal lookup.
+     *
+     *  This builds a translation table to be used for enum switches.
      *
      *  <p>For each enum that appears as the type of a switch
      *  expression, we maintain an EnumMapping to assist in the
@@ -466,8 +542,8 @@ EnumMapping mapForEnum(DiagnosticPosition pos, TypeSymbol enumClass) {
      *  </pre>
      *  class EnumMapping provides mapping data and support methods for this translation.
      */
-    class EnumMapping {
-        EnumMapping(DiagnosticPosition pos, TypeSymbol forEnum) {
+    class RuntimeEnumMapping implements EnumMapping {
+        RuntimeEnumMapping(DiagnosticPosition pos, TypeSymbol forEnum) {
             this.forEnum = forEnum;
             this.values = new LinkedHashMap<>();
             this.pos = pos;
@@ -500,15 +576,22 @@ class EnumMapping {
         // the mapped values
         final Map<VarSymbol,Integer> values;
 
-        JCLiteral forConstant(VarSymbol v) {
+        @Override
+        public JCExpression switchValue(JCExpression ordinalExpr) {
+            return make.Indexed(mapVar, ordinalExpr);
+        }
+
+        @Override
+        public JCLiteral caseValue(VarSymbol v) {
             Integer result = values.get(v);
             if (result == null)
                 values.put(v, result = next++);
             return make.Literal(result);
         }
 
         // generate the field initializer for the map
-        void translate() {
+        @Override
+        public void translate() {
             boolean prevAllowProtectedAccess = attrEnv.info.allowProtectedAccess;
             try {
                 make.at(pos.getStartPosition());
@@ -3760,7 +3843,7 @@ public JCTree visitEnumSwitch(JCTree tree, JCExpression selector, List<JCCase> c
                                                selector.type,
                                                currentMethodSym);
             JCStatement var = make.at(tree.pos()).VarDef(dollar_s, selector).setType(dollar_s.type);
-            newSelector = make.Indexed(map.mapVar,
+            newSelector = map.switchValue(
                     make.App(make.Select(make.Ident(dollar_s),
                             ordinalMethod)));
             newSelector =
@@ -3771,7 +3854,7 @@ public JCTree visitEnumSwitch(JCTree tree, JCExpression selector, List<JCCase> c
                                      .setType(newSelector.type))
                         .setType(newSelector.type);
         } else {
-            newSelector = make.Indexed(map.mapVar,
+            newSelector = map.switchValue(
                     make.App(make.Select(selector,
                             ordinalMethod)));
         }
@@ -3783,7 +3866,7 @@ public JCTree visitEnumSwitch(JCTree tree, JCExpression selector, List<JCCase> c
                     pat = makeLit(syms.intType, -1);
                 } else {
                     VarSymbol label = (VarSymbol)TreeInfo.symbol(((JCConstantCaseLabel) c.labels.head).expr);
-                    pat = map.forConstant(label);
+                    pat = map.caseValue(label);
                 }
                 newCases.append(make.Case(JCCase.STATEMENT, List.of(make.ConstantCaseLabel(pat)), c.stats, null));
             } else {

test/hotspot/jtreg/runtime/cds/appcds/jvmti/ClassFileLoadHookTest.java

@@ -42,7 +42,6 @@ public class ClassFileLoadHookTest {
     public static String sharedClasses[] = {
         "ClassFileLoadHook",
         "ClassFileLoadHook$TestCaseId",
-        "ClassFileLoadHook$1",
         "LoadMe",
         "java/sql/SQLException"
     };

test/langtools/tools/javac/T8011181/EmptyUTF8ForInnerClassNameTest.java

@@ -67,11 +67,10 @@ void checkClassFile(final Path path) throws Exception {
     }
 
     static class EnumPlusSwitch {
-        enum E {E1}
 
-        public int m (E e) {
+        public int m (Thread.State e) {
             switch (e) {
-                case E1:
+                case NEW:
                     return 0;
             }
             return -1;

test/langtools/tools/javac/enum/EnumLookupTableExceptionInInitializer.java

@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 7176515 8299760
+ * @summary ExceptionInInitializerError for an enum with multiple switch statements
+ */
+
+import java.math.RoundingMode;
+
+public class EnumLookupTableExceptionInInitializer {
+
+    public enum MyEnum {
+        FIRST(RoundingMode.CEILING),
+        SECOND(RoundingMode.HALF_DOWN),
+        THIRD(RoundingMode.UNNECESSARY),
+        FOURTH(RoundingMode.HALF_EVEN),
+        FIFTH(RoundingMode.HALF_DOWN),
+        SIXTH(RoundingMode.CEILING),
+        SEVENTH(RoundingMode.UNNECESSARY);
+
+        private final RoundingMode mode;
+
+        private MyEnum(RoundingMode mode) {
+            switch (mode) {
+            case CEILING:
+            case HALF_DOWN:
+            case UNNECESSARY:
+            case HALF_EVEN:
+                break;
+            default:
+                throw new IllegalArgumentException();
+            }
+            this.mode = mode;
+        }
+
+        public boolean isOdd() {
+            switch (this) {
+            case FIRST:
+            case THIRD:
+            case FIFTH:
+            case SEVENTH:
+                return true;
+            default:
+                return false;
+            }
+        }
+    }
+
+    public enum Nested {
+        AAA(MyEnum.FIRST),
+        BBB(MyEnum.THIRD),
+        CCC(MyEnum.FIFTH),
+        DDD(MyEnum.SEVENTH),
+        EEE(MyEnum.SECOND);
+
+        private Nested(MyEnum x) {
+            switch (x) {
+            default:
+                break;
+            }
+        }
+    }
+
+    public static void main(String[] args) {
+        boolean shouldBeOdd = true;
+        for (MyEnum x : MyEnum.values()) {
+            if (x.isOdd() != shouldBeOdd)
+                throw new RuntimeException("failed");
+            shouldBeOdd = !shouldBeOdd;
+        }
+        Nested.class.hashCode();
+    }
+}