You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2026/08/16 10:07:13 UTC
(groovy) 02/03: GROOVY-12255: Harden static switch-expression tableswitch dispatch
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch GROOVY-12255
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit e36deddc5133ca53b1632cf9b3c890dd5ed6a002
Author: Daniel Sun <su...@apache.org>
AuthorDate: Sun Aug 16 18:44:10 2026 +0900
GROOVY-12255: Harden static switch-expression tableswitch dispatch
Seed empty colon suffixes with the default label so case 1: default:
yield is not a compile error, keep OperandStack in step with the
switch and equals sequences, and lock the tableswitch / lookupswitch
shape with bytecode tests.
---
.../asm/sc/StaticTypesSwitchExpressionWriter.java | 65 ++++----
.../sc/SwitchExpressionStaticCompileTest.groovy | 176 +++++++++++++++++++++
2 files changed, 213 insertions(+), 28 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesSwitchExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesSwitchExpressionWriter.java
index 982c47cbd1..536a07188b 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesSwitchExpressionWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesSwitchExpressionWriter.java
@@ -51,7 +51,6 @@ import static org.codehaus.groovy.transform.stc.StaticTypeCheckingSupport.findDG
import static org.objectweb.asm.Opcodes.GOTO;
import static org.objectweb.asm.Opcodes.IFEQ;
import static org.objectweb.asm.Opcodes.IFNULL;
-import static org.objectweb.asm.Opcodes.ILOAD;
import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
/**
@@ -174,14 +173,14 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
boolean primitive = isIntegralType(selectorType);
if (!primitive && !isIntegralWrapper(selectorType)) return false;
+ Label defaultTarget = new Label();
ArmGroup<Integer> group = groupArms(expression.getCaseStatements(),
- cs -> intConstant(cs.getExpression()));
+ cs -> intConstant(cs.getExpression()), defaultTarget);
if (group.error) return true;
if (group.keys == null) return false;
OperandStack operandStack = controller.getOperandStack();
CompileStack compileStack = controller.getCompileStack();
- Label defaultTarget = new Label();
int intSelector = selectorIndex;
if (!primitive) {
@@ -206,21 +205,33 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
for (int i = 0; i < keys.size(); i += 1) {
sorted.put(keys.get(i), targets.get(i));
}
- int[] keyArray = sorted.keySet().stream().mapToInt(Integer::intValue).toArray();
- Label[] targetArray = sorted.values().toArray(Label[]::new);
- MethodVisitor mv = controller.getMethodVisitor();
- mv.visitVarInsn(ILOAD, intSelector);
+ int n = sorted.size();
+ int[] keyArray = new int[n];
+ Label[] targetArray = new Label[n];
+ int i = 0;
+ for (var entry : sorted.entrySet()) {
+ keyArray[i] = entry.getKey();
+ targetArray[i] = entry.getValue();
+ i += 1;
+ }
+
+ OperandStack operandStack = controller.getOperandStack();
+ operandStack.load(ClassHelper.int_TYPE, intSelector);
+ operandStack.remove(1);
+
int min = keyArray[0];
- int max = keyArray[keyArray.length - 1];
+ int max = keyArray[n - 1];
long span = (long) max - (long) min + 1L;
- // same size heuristic javac uses: tableswitch if it is no larger than lookupswitch
+ // classfile payload, padding omitted (JVMS §6.5): tableswitch ≈ 12+4*span,
+ // lookupswitch ≈ 8+8*n. Prefer tableswitch when it is no larger.
long tableSize = 12L + 4L * span;
- long lookupSize = 8L + 8L * keyArray.length;
- if (tableSize <= lookupSize) {
+ long lookupSize = 8L + 8L * n;
+ MethodVisitor mv = controller.getMethodVisitor();
+ if (span > 0 && span <= Integer.MAX_VALUE && tableSize <= lookupSize) {
Label[] table = new Label[(int) span];
Arrays.fill(table, defaultTarget);
- for (int i = 0; i < keyArray.length; i += 1) {
- table[keyArray[i] - min] = targetArray[i];
+ for (int k = 0; k < n; k += 1) {
+ table[(int) ((long) keyArray[k] - min)] = targetArray[k];
}
mv.visitTableSwitchInsn(min, max, defaultTarget, table);
} else {
@@ -234,14 +245,14 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
return false;
}
+ Label defaultTarget = new Label();
ArmGroup<String> group = groupArms(expression.getCaseStatements(),
- cs -> stringConstant(cs.getExpression()));
+ cs -> stringConstant(cs.getExpression()), defaultTarget);
if (group.error) return true;
if (group.keys == null) return false;
- Label defaultTarget = new Label();
jumpIfNull(selectorIndex, selectorType, defaultTarget);
- emitStringIndexDispatch(selectorIndex, group.keys, group.targets, defaultTarget);
+ emitStringHashDispatch(selectorIndex, group.keys, group.targets, defaultTarget);
finishArms(expression, group.targets, defaultTarget, selectorIndex, selectorType, false);
return true;
}
@@ -256,8 +267,9 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
ClassNode enumType = unwrapEnumType(selectorType);
if (enumType == null || !enumType.isEnum()) return false;
+ Label defaultTarget = new Label();
ArmGroup<String> group = groupArms(expression.getCaseStatements(),
- cs -> enumConstantName(cs.getExpression(), enumType));
+ cs -> enumConstantName(cs.getExpression(), enumType), defaultTarget);
if (group.error) return true;
if (group.keys == null) return false;
@@ -268,7 +280,6 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
boolean hasDefault = expression.getDefaultStatement() != null && !expression.getDefaultStatement().isEmpty();
boolean complete = !hasDefault && group.keys.size() == enumConstantCount(enumType);
- Label defaultTarget = new Label();
// a null selector matches no constant label; with no default it must
// throw ISE, never the complete-enum ICCE
Label nullTarget = complete ? new Label() : defaultTarget;
@@ -279,7 +290,7 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
operandStack.replace(ClassHelper.STRING_TYPE);
int nameLocal = compileStack.defineTemporaryVariable("$switchEnumName", ClassHelper.STRING_TYPE, true);
- emitStringIndexDispatch(nameLocal, group.keys, group.targets, defaultTarget);
+ emitStringHashDispatch(nameLocal, group.keys, group.targets, defaultTarget);
finishArms(expression, group.targets, defaultTarget, selectorIndex, selectorType, complete);
if (nullTarget != defaultTarget) {
mv.visitLabel(nullTarget);
@@ -295,7 +306,7 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
* jumping straight to the shared arm label. Fall-through keys already
* share that label, so a second index tableswitch is unnecessary.
*/
- private void emitStringIndexDispatch(final int stringLocal, final List<String> keys,
+ private void emitStringHashDispatch(final int stringLocal, final List<String> keys,
final List<Label> targets, final Label defaultTarget) {
Map<Integer, List<Integer>> hashToIndexes = new TreeMap<>();
for (int i = 0; i < keys.size(); i += 1) {
@@ -322,8 +333,9 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
for (int index : hashToIndexes.get(hashes[i])) {
operandStack.load(ClassHelper.STRING_TYPE, stringLocal);
mv.visitLdcInsn(keys.get(index));
+ operandStack.push(ClassHelper.STRING_TYPE);
mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "equals", "(Ljava/lang/Object;)Z", false);
- operandStack.replace(ClassHelper.boolean_TYPE);
+ operandStack.replace(ClassHelper.boolean_TYPE, 2);
Label next = operandStack.jump(IFEQ);
mv.visitJumpInsn(GOTO, targets.get(index));
mv.visitLabel(next);
@@ -334,10 +346,11 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
/**
* Forward pass extracts every constant key (or skips the optimizer).
- * Backward pass gives empty colon prefixes the following body's label.
+ * Backward pass gives empty colon prefixes the following body's label,
+ * or {@code defaultTarget} when the empty suffix falls into default.
*/
private <K> ArmGroup<K> groupArms(final List<CaseStatement> caseStatements,
- final Function<CaseStatement, K> keyFn) {
+ final Function<CaseStatement, K> keyFn, final Label defaultTarget) {
int n = caseStatements.size();
if (n == 0) return ArmGroup.skip();
@@ -354,15 +367,11 @@ public class StaticTypesSwitchExpressionWriter extends SwitchExpressionWriter {
}
Label[] targets = new Label[n];
- Label current = null;
+ Label current = defaultTarget;
for (int i = n - 1; i >= 0; i -= 1) {
if (!caseStatements.get(i).getCode().isEmpty()) {
current = new Label();
}
- if (current == null) {
- addError("the switch expression case does not complete with yield or throw", caseStatements.get(i));
- return ArmGroup.error();
- }
targets[i] = current;
}
return ArmGroup.of(keys, Arrays.asList(targets));
diff --git a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/SwitchExpressionStaticCompileTest.groovy b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/SwitchExpressionStaticCompileTest.groovy
index 2623211a2d..9bd6b7e61c 100644
--- a/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/SwitchExpressionStaticCompileTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/classgen/asm/sc/SwitchExpressionStaticCompileTest.groovy
@@ -311,6 +311,182 @@ final class SwitchExpressionStaticCompileTest extends AbstractBytecodeTestCase {
'''
}
+ @Test
+ void staticCommaArrowDenseIntUsesTableSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ switch (n) {
+ case 6, 8, 10 -> 3
+ default -> 0
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['TABLESWITCH'])
+ assert !bytecode.hasSequence(['LOOKUPSWITCH'])
+ assert findUnreachableInstructions(classBytes, 'm') == []
+ }
+
+ @Test
+ void staticColonFallThroughDenseIntUsesTableSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ return switch (n) {
+ case 1:
+ case 2:
+ yield 12
+ case 3:
+ yield 3
+ default:
+ yield 0
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['TABLESWITCH'])
+ assert !bytecode.hasSequence(['LOOKUPSWITCH'])
+ }
+
+ @Test
+ void staticNegativeDenseIntUsesTableSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ switch (n) {
+ case -1 -> -10
+ case 0 -> 0
+ case 1 -> 10
+ default -> 99
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['TABLESWITCH'])
+ assert !bytecode.hasSequence(['LOOKUPSWITCH'])
+ }
+
+ @Test
+ void staticWideIntSpanUsesLookupSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ switch (n) {
+ case -2000000000 -> -1
+ case 2000000000 -> 1
+ default -> 0
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['LOOKUPSWITCH'])
+ assert !bytecode.hasSequence(['TABLESWITCH'])
+ }
+
+ @Test
+ void staticManyStringCasesStillAvoidTableSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ String m(String s) {
+ switch (s) {
+ case 'a' -> 'A'
+ case 'b' -> 'B'
+ case 'c' -> 'C'
+ case 'd' -> 'D'
+ case 'e' -> 'E'
+ case 'f' -> 'F'
+ case 'g' -> 'G'
+ case 'h' -> 'H'
+ default -> 'z'
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['LOOKUPSWITCH'])
+ assert !bytecode.hasSequence(['TABLESWITCH'])
+ }
+
+ @Test
+ void staticStringHashCollisionUsesSingleLookupSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ String m(String s) {
+ switch (s) {
+ case 'Aa' -> 'first'
+ case 'BB' -> 'second'
+ default -> 'none'
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['LOOKUPSWITCH'])
+ assert !bytecode.hasSequence(['TABLESWITCH'])
+ assert bytecode.instructions.count { it.startsWith('INVOKEVIRTUAL java/lang/String.equals') } == 2
+ }
+
+ @Test
+ void staticColonEmptyCaseFallsIntoDefault() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ return switch (n) {
+ case 1:
+ case 2:
+ default:
+ yield 0
+ }
+ }
+ assert m(1) == 0
+ assert m(2) == 0
+ assert m(3) == 0
+ '''
+ }
+
+ @Test
+ void staticColonEmptyCaseThenDefaultUsesTableSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ return switch (n) {
+ case 1:
+ default:
+ yield 0
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['TABLESWITCH'])
+ assert !bytecode.hasSequence(['LOOKUPSWITCH'])
+ }
+
+ @Test
+ void staticColonArmIfFallsThroughToCompletingDefault() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ int m(int n, boolean cond) {
+ return switch (n) {
+ case 1:
+ if (cond) yield 10
+ default:
+ yield 0
+ }
+ }
+ assert m(1, true) == 10
+ assert m(1, false) == 0
+ assert m(2, false) == 0
+ '''
+ }
+
+ @Test
+ void staticStringColonEmptyCaseFallsIntoDefault() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ String m(String s) {
+ return switch (s) {
+ case 'a':
+ default:
+ yield 'd'
+ }
+ }
+ assert m('a') == 'd'
+ assert m('b') == 'd'
+ '''
+ }
+
@Test
void staticDuplicateIntCaseIsError() {
def err = shouldFail '''