[
https://issues.apache.org/jira/browse/GROOVY-12255?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105051#comment-18105051
]
ASF GitHub Bot commented on GROOVY-12255:
-----------------------------------------
daniellansun commented on code in PR #2784:
URL: https://github.com/apache/groovy/pull/2784#discussion_r3790984456
##########
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();
Review Comment:
Agreed.
(a)(b) The dynamic writer boxes once, because it only does `isCase`. The
static writer keeps the type `OperandStack` reports after visiting the
selector: a primitive `int` is stored as `int` and fed to `tableswitch`
with no box/unbox; a wrapper is stored as a wrapper, null-checked, then
unboxed only on that path.
(c) The selector is evaluated once through the normal visit (which
unwraps a holder via `OperandStack`) and stored in a non-holder temp.
Later loads use `OperandStack.load` on that temp, never a raw `ALOAD` of
the original slot. Tests:
`referenceSelectorWrittenByClosure`,
`referenceSelectorWrittenByClosureUnderCompileStatic`,
`referenceWrapperSelectorWrittenByClosureUnderCompileStatic`,
`staticReferenceSelectorWrittenByClosure`,
`staticIntegerReferenceSelectorWrittenByClosure`.
> 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)
