[
https://issues.apache.org/jira/browse/GROOVY-12255?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105049#comment-18105049
]
ASF GitHub Bot commented on GROOVY-12255:
-----------------------------------------
daniellansun commented on code in PR #2784:
URL: https://github.com/apache/groovy/pull/2784#discussion_r3790980332
##########
src/main/java/org/codehaus/groovy/ast/expr/SwitchExpression.java:
##########
@@ -0,0 +1,293 @@
+/*
+ * 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.ast.expr;
+
+import org.codehaus.groovy.ast.GroovyCodeVisitor;
+import org.codehaus.groovy.ast.stmt.AssertStatement;
+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.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.ast.stmt.YieldStatement;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Represents a {@code switch} used as an expression, as specified by
+ * JEP 361 (Switch Expressions). The selector is evaluated once and matched
+ * against the {@link CaseStatement} list using Groovy's {@code isCase}
+ * semantics (or a tableswitch / lookupswitch when the compiler can prove
+ * that is equivalent). Each completing arm yields a value via
+ * {@link YieldStatement}; the expression's result is that value.
+ *
+ * @see SwitchStatement
+ * @see YieldStatement
+ * @see CaseStatement
+ * @since 6.0.0
+ */
+public class SwitchExpression extends Expression {
+
+ private Expression expression;
+ private List<CaseStatement> caseStatements;
+ private Statement defaultStatement;
+
+ /**
+ * Constructs a switch expression with the given selector.
+ * The default statement is initialized to {@link EmptyStatement#INSTANCE}.
+ *
+ * @param expression the selector expression
+ */
+ public SwitchExpression(final Expression expression) {
+ this(expression, EmptyStatement.INSTANCE);
+ }
+
+ /**
+ * Constructs a switch expression with the given selector and default arm.
+ *
+ * @param expression the selector expression
+ * @param defaultStatement the arm executed when no case matches; may be
{@link EmptyStatement#INSTANCE}
+ */
+ public SwitchExpression(final Expression expression, final Statement
defaultStatement) {
+ this(expression, new ArrayList<>(), defaultStatement);
+ }
+
+ /**
+ * Constructs a switch expression with the given selector, case arms, and
default arm.
+ *
+ * @param expression the selector expression
+ * @param caseStatements the case arms
+ * @param defaultStatement the arm executed when no case matches
+ */
+ public SwitchExpression(final Expression expression, final
List<CaseStatement> caseStatements, final Statement defaultStatement) {
+ this.expression = expression;
+ this.caseStatements = caseStatements;
+ this.defaultStatement = defaultStatement;
+ }
+
+ /**
+ * Returns the selector expression matched against case values.
+ *
+ * @return the selector {@link Expression}
+ */
+ public Expression getExpression() {
+ return expression;
+ }
+
+ /**
+ * Sets the selector expression matched against case values.
+ *
+ * @param expression the selector {@link Expression}
+ */
+ public void setExpression(final Expression expression) {
+ this.expression = expression;
+ }
+
+ /**
+ * Returns the case arms of this switch expression.
+ *
+ * @return a list of {@link CaseStatement} objects; never null
+ */
+ public List<CaseStatement> getCaseStatements() {
+ return caseStatements;
+ }
+
+ /**
+ * Returns the arm executed when no case matches.
+ *
+ * @return the default {@link Statement}, or {@link
EmptyStatement#INSTANCE} if not set
+ */
+ public Statement getDefaultStatement() {
+ return defaultStatement;
+ }
+
+ /**
+ * Sets the arm executed when no case matches.
+ *
+ * @param defaultStatement the default {@link Statement}
+ */
+ public void setDefaultStatement(final Statement defaultStatement) {
+ this.defaultStatement = defaultStatement;
+ }
+
+ /**
+ * Adds a case arm to this switch expression.
+ *
+ * @param caseStatement the {@link CaseStatement} to add
+ */
+ public void addCase(final CaseStatement caseStatement) {
+ caseStatements.add(caseStatement);
+ }
+
+ @Override
+ public String getText() {
+ return "switch (" + expression.getText() + ") { ... }";
+ }
+
+ @Override
+ public String toString() {
+ return super.toString() + "[expression: " + expression + "; cases: " +
caseStatements + "; default: " + defaultStatement + "]";
+ }
+
+ @Override
+ public Expression transformExpression(final ExpressionTransformer
transformer) {
+ List<CaseStatement> transformedCases = new
ArrayList<>(caseStatements.size());
+ for (CaseStatement caseStatement : caseStatements) {
+ transformedCases.add(copyCase(caseStatement, transformer));
+ }
+ SwitchExpression ret = new SwitchExpression(
+ transformer.transform(expression),
+ transformedCases,
+ copyAndTransform(defaultStatement, transformer));
+ ret.setSourcePosition(this);
+ ret.copyNodeMetaData(this);
+ ret.setType(getType());
+ return ret;
+ }
+
+ @Override
+ public void visit(final GroovyCodeVisitor visitor) {
+ visitor.visitSwitchExpression(this);
+ }
+
+ private static CaseStatement copyCase(final CaseStatement caseStatement,
final ExpressionTransformer transformer) {
+ CaseStatement copy = new CaseStatement(
+ transformer.transform(caseStatement.getExpression()),
+ copyAndTransform(caseStatement.getCode(), transformer));
+ copy.setArrow(caseStatement.isArrow());
+ copy.setSourcePosition(caseStatement);
+ copy.copyNodeMetaData(caseStatement);
+ copy.copyStatementLabels(caseStatement);
+ return copy;
+ }
+
+ /**
+ * Returns a structural copy of {@code statement} whose nested expressions
+ * have been passed through {@code transformer}. The original tree is not
+ * mutated. Unknown statement types are returned as-is.
+ */
+ private static Statement copyAndTransform(final Statement statement, final
ExpressionTransformer transformer) {
Review Comment:
Agreed. `transformExpression` no longer copies arm statements. It rewrites
the selector and case-label expressions only and shares the arm bodies,
the same way `SwitchStatement` is rewritten by visit rather than by a
deep copy.
When the transformer is also a `GroovyCodeVisitor` (`ResolveVisitor`,
`ClassCodeExpressionTransformer`, …), those shared arms are then
*visited* so nested expressions still go through resolve and rewrite.
That is visit, not a new statement tree. A plain
`ExpressionTransformer` that is not a visitor leaves the arms untouched.
On `CaseExpression`: we kept `CaseStatement`. A `ClosureExpression`
already holds a statement body; a switch-expression arm is the same
shape (expression label, statement body). Introducing `CaseExpression`
would duplicate visitor surface without changing that. Happy to reopen
if you would rather split the node.
> 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)
