[
https://issues.apache.org/jira/browse/GROOVY-12255?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105050#comment-18105050
]
ASF GitHub Bot commented on GROOVY-12255:
-----------------------------------------
daniellansun commented on code in PR #2784:
URL: https://github.com/apache/groovy/pull/2784#discussion_r3790982275
##########
src/main/java/org/codehaus/groovy/classgen/asm/SwitchExpressionWriter.java:
##########
@@ -0,0 +1,610 @@
+/*
+ * 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 org.codehaus.groovy.classgen.asm;
+
+import org.codehaus.groovy.GroovyBugError;
+import org.codehaus.groovy.ast.ClassHelper;
+import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.DynamicVariable;
+import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.expr.ClassExpression;
+import org.codehaus.groovy.ast.expr.ConstantExpression;
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.PropertyExpression;
+import org.codehaus.groovy.ast.expr.SwitchExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.codehaus.groovy.ast.stmt.CaseStatement;
+import org.codehaus.groovy.ast.stmt.Statement;
+import org.codehaus.groovy.ast.stmt.YieldStatement;
+import org.codehaus.groovy.classgen.AsmClassGenerator;
+import org.codehaus.groovy.classgen.asm.sc.StaticTypesTypeChooser;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+
+import static org.codehaus.groovy.ast.tools.GeneralUtils.maybeFallsThrough;
+import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.ATHROW;
+import static org.objectweb.asm.Opcodes.CHECKCAST;
+import static org.objectweb.asm.Opcodes.DUP;
+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.ISTORE;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.INVOKEVIRTUAL;
+import static org.objectweb.asm.Opcodes.NEW;
+
+/**
+ * Emits JVM bytecode for a {@link SwitchExpression}. Completing arms leave the
+ * result on the operand stack and jump to a shared join point — the same shape
+ * javac uses for JEP 361 switch expressions.
+ * <p>
+ * When the selector type and case labels permit it, the writer emits
+ * {@code tableswitch} / {@code lookupswitch} (with the Java 7 two-switch form
+ * for String selectors, reused over {@code Enum.name()} for enum selectors so
+ * separately recompiled enums cannot retarget arms). A null selector matches
+ * no constant label and takes the default path. Otherwise it falls back to
+ * Groovy's sequential {@code isCase} tests so Class, regex, Collection and
+ * Closure cases keep working.
+ *
+ * @since 6.0.0
+ */
+public class SwitchExpressionWriter {
+
+ private static final String ISE_INTERNAL_NAME =
"java/lang/IllegalStateException";
+ private static final String ICCE_INTERNAL_NAME =
"java/lang/IncompatibleClassChangeError";
+ private static final String STRING_BUILDER_INTERNAL_NAME =
"java/lang/StringBuilder";
+
+ /** The controller coordinating all bytecode writers for the current
class. */
+ protected final WriterController controller;
+
+ /**
+ * Creates a switch-expression writer with the given controller.
+ *
+ * @param controller the writer controller
+ */
+ public SwitchExpressionWriter(final WriterController controller) {
+ this.controller = controller;
+ }
+
+ /**
+ * Generates bytecode for a switch expression. The result is left on the
+ * operand stack with the expression's resolved type.
+ *
+ * @param expression the switch expression to compile
+ */
+ public void writeSwitchExpression(final SwitchExpression expression) {
+ AsmClassGenerator acg = controller.getAcg();
+ acg.onLineNumber(expression, "visitSwitchExpression");
+
+ WriterController effective = effectiveController();
+ CompileStack compileStack = effective.getCompileStack();
+ OperandStack operandStack = effective.getOperandStack();
+ MethodVisitor mv = effective.getMethodVisitor();
+
+ ClassNode resultType = resolveResultType(expression, effective);
+ Label endLabel = compileStack.pushSwitchExpression(resultType);
+
+ expression.getExpression().visit(acg);
+ ClassNode selectorType = operandStack.getTopOperand();
+ ClassNode storedSelectorType =
ClassHelper.isPrimitiveType(selectorType)
+ ? ClassHelper.getWrapper(selectorType)
+ : selectorType;
+ operandStack.box();
+ int selectorIndex = compileStack.defineTemporaryVariable("switch",
storedSelectorType, true);
+
+ boolean emitted = writeIntSwitch(expression, selectorIndex,
storedSelectorType, resultType, endLabel)
+ || writeStringSwitch(expression, selectorIndex,
storedSelectorType, resultType, endLabel)
+ || writeEnumSwitch(expression, selectorIndex,
storedSelectorType, resultType, endLabel);
+ if (!emitted) {
+ writeIsCaseSwitch(expression, selectorIndex, endLabel);
+ }
+
+ mv.visitLabel(endLabel);
+ operandStack.push(resultType);
+
+ compileStack.removeVar(selectorIndex);
+ compileStack.popSwitchExpression();
+ }
+
+ /**
+ * Generates bytecode for a {@code yield} statement: evaluate the operand,
+ * cast it to the enclosing switch-expression result type, apply
intervening
+ * finally blocks, and jump to the expression join point.
+ *
+ * @param statement the yield statement to compile
+ */
+ public void writeYield(final YieldStatement statement) {
+ WriterController effective = effectiveController();
+ CompileStack compileStack = effective.getCompileStack();
+ CompileStack.SwitchExpressionContext context =
compileStack.getSwitchExpressionContext();
+ if (context == null) {
+ throw new GroovyBugError("yield outside of a switch expression");
+ }
+
+ effective.getAcg().onLineNumber(statement, "visitYieldStatement");
+ OperandStack operandStack = effective.getOperandStack();
+ MethodVisitor mv = effective.getMethodVisitor();
+ statement.getExpression().visit(effective.getAcg());
+ operandStack.doGroovyCast(context.resultType);
+ if (compileStack.hasBlockRecorder()) {
+ // stash the result so intervening finally / synchronized can run
+ // without seeing it on the operand stack (same shape as return)
+ int rv = compileStack.defineTemporaryVariable("$yield",
context.resultType, true);
+ compileStack.applyFinallyBlocks(context.endLabel, true);
+ BytecodeHelper.load(mv, context.resultType, rv);
+ compileStack.removeVar(rv);
+ } else {
+ operandStack.remove(1);
+ }
+ mv.visitJumpInsn(GOTO, context.endLabel);
+ }
+
+
//--------------------------------------------------------------------------
+
+ private void writeIsCaseSwitch(final SwitchExpression expression, final
int selectorIndex, final Label endLabel) {
+ WriterController effective = effectiveController();
+
+ List<CaseStatement> caseStatements = expression.getCaseStatements();
+ int caseCount = caseStatements.size();
+ Label[] bodyLabels = new Label[caseCount + 1];
+ for (int i = 0; i < caseCount; i += 1) {
+ bodyLabels[i] = new Label();
+ }
+
+ for (int i = 0; i < caseCount; i += 1) {
+ writeIsCaseArm(caseStatements.get(i), selectorIndex,
bodyLabels[i], bodyLabels[i + 1]);
+ }
+
+ writeDefaultOrThrow(expression, selectorIndex, false);
+ }
+
+ private void writeIsCaseArm(final CaseStatement caseStatement, final int
selectorIndex,
+ final Label thisLabel, final Label nextLabel) {
+ WriterController effective = effectiveController();
+ MethodVisitor mv = effective.getMethodVisitor();
+ OperandStack operandStack = effective.getOperandStack();
+ AsmClassGenerator acg = effective.getAcg();
+
+ acg.onLineNumber(caseStatement, "visitCaseStatement");
+
+ mv.visitVarInsn(ALOAD, selectorIndex);
+ caseStatement.getExpression().visit(acg);
+ operandStack.box();
+ effective.getBinaryExpressionHelper().getIsCaseMethod().call(mv);
+ operandStack.replace(ClassHelper.boolean_TYPE);
+
+ Label miss = operandStack.jump(IFEQ);
+
+ mv.visitLabel(thisLabel);
+ caseStatement.getCode().visit(acg);
+
+ if (nextLabel != null && maybeFallsThrough(caseStatement.getCode())) {
+ mv.visitJumpInsn(GOTO, nextLabel);
+ }
+
+ mv.visitLabel(miss);
+ }
+
+ private void writeDefaultOrThrow(final SwitchExpression expression, final
int selectorIndex, final boolean completeEnum) {
+ WriterController effective = effectiveController();
+ Statement defaultStatement = expression.getDefaultStatement();
+ if (defaultStatement != null && !defaultStatement.isEmpty()) {
+ defaultStatement.visit(effective.getAcg());
+ return;
+ }
+ if (completeEnum) {
+ throwIncompatibleClassChangeError(effective.getMethodVisitor());
+ } else {
+ throwIllegalState(effective.getMethodVisitor(), selectorIndex);
+ }
+ }
+
+ private boolean writeIntSwitch(final SwitchExpression expression, final
int selectorIndex,
+ final ClassNode storedSelectorType, final ClassNode resultType,
final Label endLabel) {
+ if (!isStaticCompilation()) return false;
+ if (!isIntegralType(storedSelectorType) &&
!isIntegralWrapper(storedSelectorType)) return false;
+
+ List<CaseStatement> caseStatements = expression.getCaseStatements();
+ Map<Integer, Label> keyToBody = new TreeMap<>();
+ Label currentGroupBody = null;
+ for (CaseStatement caseStatement : caseStatements) {
+ Integer key = intConstant(caseStatement.getExpression());
+ if (key == null) return false;
+ if (!caseStatement.getCode().isEmpty() || caseStatement.isArrow())
{
+ currentGroupBody = new Label();
+ }
+ if (currentGroupBody == null) {
+ currentGroupBody = new Label();
+ }
+ if (keyToBody.put(key, currentGroupBody) != null) {
Review Comment:
Agreed. Empty colon arms are now grouped with the *following* completing
arm (walked backwards). A trailing empty group, or a duplicate constant
label, is reported as a compilation error rather than inventing a
standalone jump target or silently falling back to `isCase`.
> Compile switch expressions as first-class AST (no closure desugar)
> ------------------------------------------------------------------
>
> Key: GROOVY-12255
> URL: https://issues.apache.org/jira/browse/GROOVY-12255
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
> Labels: breaking
>
> h3. Problem
> GROOVY-9272 added switch expressions. The 4.0 implementation rewrites them in
> {{AstBuilder}} to an immediately-called closure around a switch
> {{{}statement{}}}:
> {code:groovy}
> // source
> def r = switch (x) {
> case 0, 1 -> 'a'
> default -> 'z'
> }
> // compiled as
> def r = { ->
> switch (x) {
> case 0:
> case 1: return 'a'
> default: return 'z'
> }
> }.call()
> {code}
> That is a simulation, not a JEP 361 switch expression:
> * every evaluation allocates a closure and an extra call frame
> * an unmatched selector completes with {{null}} instead of throwing
> * {{return}} / {{break}} / {{continue}} are interpreted against the
> synthetic closure, not the enclosing method
> * locals assigned in an arm are closure-shared, not method locals
> * {{@CompileStatic}} cannot emit {{tableswitch}} / {{lookupswitch}} the way
> javac does
> h3. Goal
> Compile a switch expression as a first-class {{SwitchExpression}} whose arms
> {{yield}} (or throw). Emit the result on the operand stack. Keep Groovy
> {{isCase}} matching (Class, regex, Collection, Closure). Align control flow
> and exhaustiveness with [JEP 361|https://openjdk.org/jeps/361] for both
> dynamic Groovy and {{@TypeChecked}} / {{{}@CompileStatic{}}}.
> h3. Proposed shape
> * Parser builds {{SwitchExpression}} / {{{}YieldStatement{}}}; arrow
> expressions become implicit {{{}yield{}}}. No closure wrapper.
> * Codegen: join all completing arms at one label with the value on the
> stack. When the selector and labels allow it, emit {{tableswitch}} /
> {{{}lookupswitch{}}}, the Java string-switch (hash + {{equals}} + second
> switch), or {{{}Enum.ordinal(){}}}; otherwise sequential {{{}isCase{}}}.
> * Exhaustiveness: unmatched dynamic selector throws
> {{{}IllegalStateException{}}}; a complete enum may omit {{default}}
> (synthetic {{IncompatibleClassChangeError}} if a new constant appears at
> runtime). {{@TypeChecked}} / {{@CompileStatic}} reject a provably
> non-exhaustive expression at compile time.
> * Control flow: {{return}} must not leave the enclosing method through a
> switch expression; {{yield}} must not jump through a nested closure/lambda.
> An arrow arm must {{yield}} or throw on every path.
> {code:groovy}
> int n = switch (day) {
> case MONDAY, FRIDAY -> 6
> case TUESDAY -> 7
> default -> {
> int len = day.toString().length()
> yield len
> }
> }
> {code}
> h3. Compatibility
> ||topic||4.0-5.x (closure rewrite)||after this change||
> |unmatched selector (dynamic)|{{null}}|{{IllegalStateException}}|
> |non-exhaustive under STC / CS|often accepted|compile error (unless a
> complete enum)|
> |arrow block with no {{yield}}|last expression is the closure result|compile
> error unless every path yields or throws|
> |Groovy {{isCase}} cases|works|still works (fast path only when labels are
> int / String / enum constants)|
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
