This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch GROOVY-10156 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 2a6694a17e140dd7e78a12ec85b2c470e2c6d097 Author: Daniel Sun <[email protected]> AuthorDate: Sat May 16 17:28:08 2026 +0900 GROOVY-10156: Unreachable bytecode in switch statement --- .../codehaus/groovy/ast/tools/GeneralUtils.java | 275 ++++++++++- .../codehaus/groovy/classgen/asm/CompileStack.java | 20 + .../groovy/classgen/asm/StatementWriter.java | 29 +- src/test/groovy/bugs/Groovy10156.groovy | 548 +++++++++++++++++++++ 4 files changed, 848 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java index 05086ea028..ce4c83f077 100644 --- a/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java +++ b/src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java @@ -19,6 +19,7 @@ package org.codehaus.groovy.ast.tools; import groovy.lang.MetaProperty; +import groovy.transform.Internal; import org.codehaus.groovy.ast.ASTNode; import org.codehaus.groovy.ast.AnnotatedNode; import org.codehaus.groovy.ast.AnnotationNode; @@ -38,6 +39,7 @@ import org.codehaus.groovy.ast.expr.BooleanExpression; import org.codehaus.groovy.ast.expr.CastExpression; import org.codehaus.groovy.ast.expr.ClassExpression; import org.codehaus.groovy.ast.expr.ClosureExpression; +import org.codehaus.groovy.ast.expr.ClosureListExpression; import org.codehaus.groovy.ast.expr.ConstantExpression; import org.codehaus.groovy.ast.expr.ConstructorCallExpression; import org.codehaus.groovy.ast.expr.DeclarationExpression; @@ -56,18 +58,23 @@ import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; import org.codehaus.groovy.ast.expr.TernaryExpression; import org.codehaus.groovy.ast.expr.VariableExpression; import org.codehaus.groovy.ast.stmt.BlockStatement; +import org.codehaus.groovy.ast.stmt.BreakStatement; import org.codehaus.groovy.ast.stmt.CaseStatement; import org.codehaus.groovy.ast.stmt.CatchStatement; +import org.codehaus.groovy.ast.stmt.ContinueStatement; +import org.codehaus.groovy.ast.stmt.DoWhileStatement; import org.codehaus.groovy.ast.stmt.EmptyStatement; import org.codehaus.groovy.ast.stmt.ExpressionStatement; import org.codehaus.groovy.ast.stmt.ForStatement; import org.codehaus.groovy.ast.stmt.IfStatement; +import org.codehaus.groovy.ast.stmt.LoopingStatement; import org.codehaus.groovy.ast.stmt.ReturnStatement; import org.codehaus.groovy.ast.stmt.Statement; import org.codehaus.groovy.ast.stmt.SwitchStatement; import org.codehaus.groovy.ast.stmt.SynchronizedStatement; import org.codehaus.groovy.ast.stmt.ThrowStatement; import org.codehaus.groovy.ast.stmt.TryCatchStatement; +import org.codehaus.groovy.ast.stmt.WhileStatement; import org.codehaus.groovy.classgen.BytecodeExpression; import org.codehaus.groovy.control.io.ReaderSource; import org.codehaus.groovy.runtime.GeneratedClosure; @@ -1264,26 +1271,268 @@ public class GeneralUtils { return false; } else if (statement instanceof ThrowStatement) { return false; - } else if (statement instanceof BlockStatement) { - List<Statement> list = ((BlockStatement) statement).getStatements(); + } else if (statement instanceof BlockStatement blockStmt) { + List<Statement> list = blockStmt.getStatements(); final int last = list.size() - 1; if (!maybeFallsThrough(list.get(last))) return false; for (int i = 0; i < last; i += 1) if (!maybeFallsThrough(list.get(i))) return false; - } else if (statement instanceof IfStatement) { - return maybeFallsThrough(((IfStatement) statement).getElseBlock()) - || maybeFallsThrough(((IfStatement) statement).getIfBlock()); - } else if (statement instanceof SwitchStatement) { - // TODO - } else if (statement instanceof TryCatchStatement tryCatch) { - if (!maybeFallsThrough(tryCatch.getFinallyStatement())) return false; - for (CatchStatement cs : tryCatch.getCatchStatements()) + } else if (statement instanceof IfStatement ifStmt) { + return maybeFallsThrough(ifStmt.getElseBlock()) + || maybeFallsThrough(ifStmt.getIfBlock()); + } else if (statement instanceof LoopingStatement loopingStmt) { + return maybeFallsThroughLoop(loopingStmt); + } else if (statement instanceof SwitchStatement switchStmt) { + return maybeFallsThroughSwitch(switchStmt); + } else if (statement instanceof TryCatchStatement tryCatchStmt) { + if (!maybeFallsThrough(tryCatchStmt.getFinallyStatement())) return false; + for (CatchStatement cs : tryCatchStmt.getCatchStatements()) if (maybeFallsThrough(cs.getCode())) return true; - return maybeFallsThrough(tryCatch.getTryStatement()); - } else if (statement instanceof SynchronizedStatement) { - return maybeFallsThrough(((SynchronizedStatement) statement).getCode()); + return maybeFallsThrough(tryCatchStmt.getTryStatement()); + } else if (statement instanceof SynchronizedStatement syncStmt) { + return maybeFallsThrough(syncStmt.getCode()); } return true; } + + /** + * Indicates whether switch-case code may fall through into the next case body. + */ + @Internal + public static boolean maybeFallsThroughToNextSwitchCase(final Statement statement, final SwitchStatement switchStatement) { + Objects.requireNonNull(switchStatement); + return analyzeSwitchFlow(statement).mayContinue; + } + + private static boolean maybeFallsThroughSwitch(final SwitchStatement statement) { + return analyzeSwitchFlow(statement).mayContinue; + } + + private static boolean maybeFallsThroughLoop(final LoopingStatement statement) { + return analyzeLoopFlow(statement).mayContinue; + } + + private static SwitchFlow analyzeSwitchFlow(final Statement statement) { + if (statement.isEmpty()) return SwitchFlow.FALL_THROUGH; + + if (statement instanceof BreakStatement breakStatement) { + return SwitchFlow.breakFlow(breakStatement.getLabel()); + } else if (statement instanceof ContinueStatement || statement instanceof ReturnStatement || statement instanceof ThrowStatement) { + return SwitchFlow.ABRUPT; + } else if (statement instanceof BlockStatement block) { + SwitchFlow flow = SwitchFlow.FALL_THROUGH; + for (Statement subStatement : block.getStatements()) { + flow = flow.then(analyzeSwitchFlow(subStatement)); + if (!flow.mayContinue) break; + } + return flow.consumeLabels(labelSet(block.getStatementLabels())); + } else if (statement instanceof IfStatement ifStatement) { + SwitchFlow thenFlow = analyzeSwitchFlow(ifStatement.getIfBlock()); + SwitchFlow elseFlow = analyzeSwitchFlow(ifStatement.getElseBlock()); + return thenFlow.or(elseFlow); + } else if (statement instanceof TryCatchStatement tryCatch) { + SwitchFlow finallyFlow = analyzeSwitchFlow(tryCatch.getFinallyStatement()); + SwitchFlow bodyFlow = analyzeSwitchFlow(tryCatch.getTryStatement()); + for (CatchStatement catchStatement : tryCatch.getCatchStatements()) { + bodyFlow = bodyFlow.or(analyzeSwitchFlow(catchStatement.getCode())); + } + return bodyFlow.thenFinally(finallyFlow); + } else if (statement instanceof SynchronizedStatement synchronizedStatement) { + return analyzeSwitchFlow(synchronizedStatement.getCode()); + } else if (statement instanceof LoopingStatement loopingStatement) { + return analyzeLoopFlow(loopingStatement); + } else if (statement instanceof SwitchStatement switchStatement) { + return analyzeNestedSwitchFlow(switchStatement); + } + + return SwitchFlow.of(maybeFallsThrough(statement), Set.of()); + } + + private static SwitchFlow analyzeLoopFlow(final LoopingStatement statement) { + SwitchFlow flow = analyzeSwitchFlow(statement.getLoopBlock()); + LoopCondition condition = loopCondition(statement); + Set<String> loopLabels = labelSet(((Statement) statement).getStatementLabels()); + boolean mayBreakOutOfLoop = flow.mayBreakUnlabeled || flow.breaksToAny(loopLabels); + boolean mayExitViaCondition = mayExitLoopViaCondition(statement, condition, flow.mayContinue); + Set<String> breakLabels = mayRunLoopBlock(statement, condition) + ? withoutLabels(flow.breakLabels, loopLabels) + : Set.of(); + return SwitchFlow.of(mayBreakOutOfLoop || mayExitViaCondition, breakLabels); + } + + private static boolean mayRunLoopBlock(final LoopingStatement statement, final LoopCondition condition) { + return !(statement instanceof WhileStatement || isClassicForLoop(statement)) + || condition != LoopCondition.ALWAYS_FALSE; + } + + private static boolean mayExitLoopViaCondition(final LoopingStatement statement, final LoopCondition condition, final boolean bodyMayContinue) { + if (statement instanceof DoWhileStatement) { + return bodyMayContinue && condition != LoopCondition.ALWAYS_TRUE; + } + if (statement instanceof ForStatement forStatement && !isClassicForLoop(forStatement)) { + return true; + } + return condition != LoopCondition.ALWAYS_TRUE; + } + + private static boolean isClassicForLoop(final LoopingStatement statement) { + return statement instanceof ForStatement + && ((ForStatement) statement).getCollectionExpression() instanceof ClosureListExpression; + } + + private static LoopCondition loopCondition(final LoopingStatement statement) { + if (statement instanceof WhileStatement whileStatement) { + return constantBooleanValue(whileStatement.getBooleanExpression()); + } + if (statement instanceof DoWhileStatement doWhileStatement) { + return constantBooleanValue(doWhileStatement.getBooleanExpression()); + } + if (statement instanceof ForStatement forStatement + && forStatement.getCollectionExpression() instanceof ClosureListExpression closureListExpression) { + List<Expression> expressions = closureListExpression.getExpressions(); + if (!expressions.isEmpty()) { + return constantBooleanValue(expressions.get((expressions.size() - 1) / 2)); + } + } + return LoopCondition.UNKNOWN; + } + + private static LoopCondition constantBooleanValue(Expression expression) { + boolean reverse = false; + while (expression instanceof BooleanExpression) { + if (expression instanceof NotExpression) reverse = !reverse; + expression = ((BooleanExpression) expression).getExpression(); + } + if (expression instanceof ConstantExpression constantExpression && constantExpression.getValue() instanceof Boolean) { + return ((Boolean) constantExpression.getValue()) != reverse ? LoopCondition.ALWAYS_TRUE : LoopCondition.ALWAYS_FALSE; + } + return LoopCondition.UNKNOWN; + } + + private static Set<String> labelSet(final List<String> labels) { + if (labels == null || labels.isEmpty()) return Set.of(); + return new LinkedHashSet<>(labels); + } + + private static SwitchFlow analyzeNestedSwitchFlow(final SwitchStatement statement) { + Set<String> switchLabels = labelSet(statement.getStatementLabels()); + SwitchFlow nextFlow = flowAfterSwitch(statement.getDefaultStatement(), switchLabels); + SwitchFlow switchFlow = nextFlow; + + List<CaseStatement> caseStatements = statement.getCaseStatements(); + for (int i = caseStatements.size() - 1; i >= 0; i -= 1) { + SwitchFlow flow = analyzeSwitchFlow(caseStatements.get(i).getCode()); + boolean mayBreakSwitch = flow.mayBreakUnlabeled || flow.breaksToAny(switchLabels); + Set<String> breakLabels = withoutLabels(flow.breakLabels, switchLabels); + nextFlow = SwitchFlow.of( + mayBreakSwitch || (flow.mayContinue && nextFlow.mayContinue), + union(breakLabels, flow.mayContinue ? nextFlow.breakLabels : Set.of()) + ); + switchFlow = nextFlow.or(switchFlow); + } + + return switchFlow; + } + + private static SwitchFlow flowAfterSwitch(final Statement statement, final Set<String> switchLabels) { + SwitchFlow flow = analyzeSwitchFlow(statement); + return SwitchFlow.of( + flow.mayBreakUnlabeled || flow.breaksToAny(switchLabels) || flow.mayContinue, + withoutLabels(flow.breakLabels, switchLabels) + ); + } + + private static Set<String> union(final Set<String> left, final Set<String> right) { + if (left.isEmpty()) return right; + if (right.isEmpty()) return left; + + Set<String> labels = new LinkedHashSet<>(left); + labels.addAll(right); + return labels; + } + + private static Set<String> withoutLabels(final Set<String> labels, final Set<String> removedLabels) { + if (labels.isEmpty() || removedLabels.isEmpty()) return labels; + + Set<String> remainingLabels = new LinkedHashSet<>(labels); + remainingLabels.removeAll(removedLabels); + return remainingLabels; + } + + /** + * Represents the flow of control through a switch case or loop body, tracking whether execution may continue past the end of the block + * @param mayContinue whether execution may continue past the end of the block (i.e. not all paths return, throw, or break out of the block) + * @param mayBreakUnlabeled whether the block may be exited via an unlabeled break statement + * @param breakLabels the set of labels of break statements that may exit the block (if any) + */ + private record SwitchFlow(boolean mayContinue, boolean mayBreakUnlabeled, Set<String> breakLabels) { + /** + * Code always terminates abruptly via {@code return}, {@code throw}, or {@code continue}. + */ + static final SwitchFlow ABRUPT = new SwitchFlow(false, false, Set.of()); + /** + * Code may continue after this statement and does not leave unresolved breaks behind. + */ + static final SwitchFlow FALL_THROUGH = new SwitchFlow(true, false, Set.of()); + + static SwitchFlow breakFlow(final String label) { + if (label == null) return new SwitchFlow(false, true, Set.of()); + return new SwitchFlow(false, false, Set.of(label)); + } + + static SwitchFlow of(final boolean mayContinue, final Set<String> breakLabels) { + if (!mayContinue && breakLabels.isEmpty()) return ABRUPT; + if (mayContinue && breakLabels.isEmpty()) return FALL_THROUGH; + return new SwitchFlow(mayContinue, false, breakLabels); + } + + SwitchFlow then(final SwitchFlow next) { + if (!mayContinue) return this; + if (!mayBreakUnlabeled && breakLabels.isEmpty()) return next; + return new SwitchFlow(next.mayContinue, mayBreakUnlabeled || next.mayBreakUnlabeled, union(breakLabels, next.breakLabels)); + } + + SwitchFlow or(final SwitchFlow other) { + return new SwitchFlow( + mayContinue || other.mayContinue, + mayBreakUnlabeled || other.mayBreakUnlabeled, + union(breakLabels, other.breakLabels) + ); + } + + SwitchFlow thenFinally(final SwitchFlow finallyFlow) { + return new SwitchFlow( + finallyFlow.mayContinue && mayContinue, + finallyFlow.mayBreakUnlabeled || (finallyFlow.mayContinue && mayBreakUnlabeled), + union(finallyFlow.breakLabels, finallyFlow.mayContinue ? breakLabels : Set.of()) + ); + } + + SwitchFlow consumeLabels(final Set<String> labels) { + if (labels.isEmpty() || breakLabels.isEmpty()) return this; + + Set<String> remainingBreakLabels = withoutLabels(breakLabels, labels); + boolean consumedBreak = remainingBreakLabels.size() != breakLabels.size(); + return new SwitchFlow(mayContinue || consumedBreak, mayBreakUnlabeled, remainingBreakLabels); + } + + boolean breaksToAny(final Set<String> labels) { + if (labels.isEmpty() || breakLabels.isEmpty()) return false; + + for (String label : labels) { + if (breakLabels.contains(label)) return true; + } + return false; + } + } + + /** + * Represents the condition of a loop that may affect whether the loop body is executed or exited via a condition. + */ + private enum LoopCondition { + ALWAYS_FALSE, + ALWAYS_TRUE, + UNKNOWN + } } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java index 314e7ef91e..3629e9f6da 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java @@ -607,8 +607,28 @@ public class CompileStack { * @return the break label */ public Label pushSwitch() { + return pushSwitch(null); + } + + /** + * Creates a new break label, registers it as the named break target for each + * label name, and pushes an element onto the state stack so that a later call + * to {@link #pop()} restores the previous state. + * + * @param labelNames the statement labels on the switch (may be {@code null}) + * @return the break label + * + * @since 6.0.0 + */ + public Label pushSwitch(final List<String> labelNames) { pushState(); breakLabel = new Label(); + if (labelNames != null) { + for (String labelName : labelNames) { + if (labelName == null) continue; + namedLoopBreakLabel.put(labelName, breakLabel); + } + } return breakLabel; } diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java index 9faca8a624..fb63b915df 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java @@ -578,7 +578,7 @@ public class StatementWriter { // switch does not have a continue label; use enclosing continue label CompileStack compileStack = controller.getCompileStack(); - Label breakLabel = compileStack.pushSwitch(); + Label breakLabel = compileStack.pushSwitch(statement.getStatementLabels()); int switchVariableIndex = compileStack.defineTemporaryVariable("switch", exprType, true); @@ -591,24 +591,26 @@ public class StatementWriter { int i = 0; for (Iterator<CaseStatement> iter = caseStatements.iterator(); iter.hasNext(); i += 1) { - writeCaseStatement(iter.next(), switchVariableIndex, labels[i], labels[i + 1]); + writeCaseStatement(iter.next(), statement, switchVariableIndex, labels[i], labels[i + 1]); } statement.getDefaultStatement().visit(controller.getAcg()); - controller.getMethodVisitor().visitLabel(breakLabel); + if (maybeFallsThrough(statement) || statement.getStatementLabels() != null) { + controller.getMethodVisitor().visitLabel(breakLabel); + } compileStack.removeVar(switchVariableIndex); compileStack.pop(); } - private void writeCaseStatement(final CaseStatement statement, final int switchVariableIndex, final Label thisLabel, final Label nextLabel) { - controller.getAcg().onLineNumber(statement, "visitCaseStatement"); + private void writeCaseStatement(final CaseStatement caseStatement, final SwitchStatement switchStatement, final int switchVariableIndex, final Label thisLabel, final Label nextLabel) { + controller.getAcg().onLineNumber(caseStatement, "visitCaseStatement"); MethodVisitor mv = controller.getMethodVisitor(); OperandStack operandStack = controller.getOperandStack(); mv.visitVarInsn(ALOAD, switchVariableIndex); - statement.getExpression().visit(controller.getAcg()); + caseStatement.getExpression().visit(controller.getAcg()); operandStack.box(); controller.getBinaryExpressionHelper().getIsCaseMethod().call(mv); operandStack.replace(ClassHelper.boolean_TYPE); @@ -617,10 +619,10 @@ public class StatementWriter { mv.visitLabel(thisLabel); - statement.getCode().visit(controller.getAcg()); + caseStatement.getCode().visit(controller.getAcg()); // now if we don't finish with a break we need to jump past the next comparison - if (nextLabel != null) { + if (nextLabel != null && maybeFallsThroughToNextSwitchCase(caseStatement.getCode(), switchStatement)) { mv.visitJumpInsn(GOTO, nextLabel); } @@ -699,13 +701,18 @@ public class StatementWriter { compileStack.writeExceptionTable(fb, catchAll, null); compileStack.pop(); //pop fb - finallyPart.run(); - mv.visitJumpInsn(GOTO, synchronizedEnd); + boolean fallthrough = maybeFallsThrough(statement.getCode()); + if (fallthrough) { + finallyPart.run(); + mv.visitJumpInsn(GOTO, synchronizedEnd); + } mv.visitLabel(catchAll); finallyPart.run(); mv.visitInsn(ATHROW); - mv.visitLabel(synchronizedEnd); + if (fallthrough) { + mv.visitLabel(synchronizedEnd); + } compileStack.removeVar(index); } diff --git a/src/test/groovy/bugs/Groovy10156.groovy b/src/test/groovy/bugs/Groovy10156.groovy new file mode 100644 index 0000000000..7aa023294d --- /dev/null +++ b/src/test/groovy/bugs/Groovy10156.groovy @@ -0,0 +1,548 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package bugs + +import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase +import org.junit.jupiter.api.Test + +final class Groovy10156 extends AbstractBytecodeTestCase { + + @Test + void testEnumSwitchWithBreakDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e) { + int result = 0 + switch (e) { + case E.ONE: + result = 1 + break + case E.TWO: + result = 2 + break + case E.THREE: + result = 3 + break + default: + result = 4 + } + result + } + } + enum E { + ONE, TWO, THREE + } + ''') + } + + @Test + void testEnumSwitchWithReturnDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + return 1 + case E.TWO: + return 2 + default: + return 3 + } + } + } + enum E { + ONE, TWO + } + ''') + } + + @Test + void testEnumSwitchWithThrowDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + throw new IllegalStateException('one') + case E.TWO: + throw new IllegalStateException('two') + default: + throw new IllegalStateException('default') + } + } + } + enum E { + ONE, TWO + } + ''') + } + + @Test + void testEnumSwitchWithNestedSwitchWhereAllPathsReturnDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e, boolean flag) { + switch (e) { + case E.ONE: + switch (flag) { + case true: + return 1 + default: + return 2 + } + case E.TWO: + return 3 + default: + return 4 + } + } + } + enum E { + ONE, TWO + } + ''') + } + + @Test + void testEnumSwitchWithContinueDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E[] values) { + int result = 0 + outer: + for (E e : values) { + switch (e) { + case E.ONE: + result += 1 + continue outer + case E.TWO: + result += 2 + break + default: + result += 4 + } + result += 8 + } + result + } + } + enum E { + ONE, TWO, THREE + } + ''') + } + + @Test + void testEnumSwitchWithTryCatchFinallyWhereAllPathsReturnDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + try { + return 1 + } catch (RuntimeException ignored) { + return 2 + } finally { + } + default: + return 3 + } + } + } + enum E { + ONE + } + ''') + } + + @Test + void testEnumSwitchWithSynchronizedReturnDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod ''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO + } + + @CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + synchronized (this) { + return 1 + } + default: + return 2 + } + } + } + ''' + } + + @Test + void testEnumSwitchWithInfiniteWhileDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod ''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO + } + + @CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + while (true) { + } + case E.TWO: + return 2 + default: + return 3 + } + } + } + ''' + } + + @Test + void testEnumSwitchWithInfiniteDoWhileDoesNotEmitSyntheticFallthroughJumps() { + assertNoUnreachableSequenceInTestMethod ''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO + } + + @CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + do { + } while (true) + case E.TWO: + return 2 + default: + return 3 + } + } + } + ''' + } + + @Test + void testEnumSwitchWithEmptyCaseStillFallsThrough() { + assertNoUnreachableSequenceInTestMethod ''' + @groovy.transform.CompileStatic + class C { + int test(E e) { + switch (e) { + case E.ONE: + case E.TWO: + return 2 + default: + return 3 + } + } + } + enum E { + ONE, TWO + } + ''' + } + + @Test + void testEnumSwitchWithMixedBreakAndFallThroughPreservesSemantics() { + assertNoUnreachableSequenceInTestMethod ''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO, THREE + } + + @CompileStatic + class C { + int test(E e, boolean stopEarly) { + int result = 0 + switch (e) { + case E.ONE: + if (stopEarly) break + result = 1 + case E.TWO: + return result + 2 + default: + return 4 + } + return result + 8 + } + } + ''' + } + + @Test + void testLabeledSwitchRemainsValid() { + assertNoUnreachableSequenceInTestMethod ''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO + } + + @CompileStatic + class C { + int test(E e) { + outer: + switch (e) { + case E.ONE: + return 1 + default: + return 2 + } + } + } + ''' + } + + @Test + void testNestedLabeledSwitchRemainsValid() { + assertNoUnreachableSequenceInTestMethod ''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO + } + + @CompileStatic + class C { + int test(E e, boolean flag) { + switch (e) { + case E.ONE: + inner: + switch (flag) { + case true: + return 2 + default: + return 1 + } + default: + return 3 + } + } + } + ''' + } + + @Test + void testSwitchFlowAnalysisHandlesLabeledBlockBreaksInCaseBodies() { + assertNoUnreachableSequenceAndRunAssertions(''' + @groovy.transform.CompileStatic + class C { + int test(E e, boolean stopEarly) { + switch (e) { + case E.ONE: + inner: { + if (stopEarly) { + break inner + } + return 1 + } + case E.TWO: + return 2 + default: + return 3 + } + } + } + enum E { + ONE, TWO, THREE + } + ''') { loader -> + def c = loader.loadClass('C').getDeclaredConstructor().newInstance() + def e = loader.loadClass('E') + assert c.test(Enum.valueOf(e, 'ONE'), false) == 1 + assert c.test(Enum.valueOf(e, 'ONE'), true) == 2 + assert c.test(Enum.valueOf(e, 'TWO'), false) == 2 + assert c.test(Enum.valueOf(e, 'THREE'), false) == 3 + } + } + + @Test + void testSwitchFlowAnalysisHandlesTryCatchFinallyInCaseBodies() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e, boolean flag) { + switch (e) { + case E.ONE: + try { + if (flag) return 1 + } catch (RuntimeException ignored) { + return 2 + } finally { + } + case E.TWO: + return 3 + default: + return 4 + } + } + } + enum E { + ONE, TWO + } + ''') + } + + @Test + void testSwitchFlowAnalysisHandlesSynchronizedBlocksInCaseBodies() { + assertNoUnreachableSequenceInTestMethod(''' + @groovy.transform.CompileStatic + class C { + int test(E e, boolean flag) { + switch (e) { + case E.ONE: + synchronized (this) { + if (flag) { + return 1 + } + } + case E.TWO: + return 2 + default: + return 3 + } + } + } + enum E { + ONE, TWO + } + ''') + } + + @Test + void testSwitchFlowAnalysisHandlesInfiniteWhileWithBreakInCaseBodies() { + assertRunAssertions(''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO, THREE + } + + @CompileStatic + class C { + int test(E e, boolean stopEarly) { + switch (e) { + case E.ONE: + while (true) { + if (stopEarly) break + return 1 + } + case E.TWO: + return 2 + default: + return 3 + } + } + } + ''') { loader -> + def c = loader.loadClass('C').getDeclaredConstructor().newInstance() + def e = loader.loadClass('E') + assert c.test(Enum.valueOf(e, 'ONE'), false) == 1 + assert c.test(Enum.valueOf(e, 'ONE'), true) == 2 + assert c.test(Enum.valueOf(e, 'TWO'), false) == 2 + assert c.test(Enum.valueOf(e, 'THREE'), false) == 3 + } + } + + @Test + void testLabeledBreakToSwitchInsideLoopStillAllowsNormalCompletion() { + assertNoUnreachableSequenceAndRunAssertions(''' + import groovy.transform.CompileStatic + + @CompileStatic + enum E { + ONE, TWO + } + + @CompileStatic + class C { + int test(E e) { + int result = 0 + synchronized (this) { + outer: + switch (e) { + case E.ONE: + for (int i = 0; i < 3; i++) { + if (i == 1) break outer + } + return 99 + default: + return -1 + } + result = 7 + } + return result + } + } + ''') { loader -> + def c = loader.loadClass('C').getDeclaredConstructor().newInstance() + def e = loader.loadClass('E') + assert c.test(Enum.valueOf(e, 'ONE')) == 7 + assert c.test(Enum.valueOf(e, 'TWO')) == -1 + } + } + + private void assertNoUnreachableSequenceInTestMethod(final String source) { + def bytecode = compile(classNamePattern: 'C', method: 'test', source) + assert !bytecode.hasStrictSequence(UNREACHABLE_SEQUENCE) + } + + private void assertNoUnreachableSequenceAndRunAssertions(final String source, final Closure<?> assertions) { + assertNoUnreachableSequenceInTestMethod(source) + assertRunAssertions(source, assertions) + } + + private void assertRunAssertions(final String source, final Closure<?> assertions) { + def loader = new GroovyClassLoader(this.class.classLoader) + try { + loader.parseClass(source, 'Groovy10156Runtime.groovy') + assertions.call(loader) + } finally { + loader.close() + } + } + + private static final List<String> UNREACHABLE_SEQUENCE = ['NOP', 'NOP', 'ATHROW'] +}
