[
https://issues.apache.org/jira/browse/GROOVY-12255?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104776#comment-18104776
]
ASF GitHub Bot commented on GROOVY-12255:
-----------------------------------------
blackdrag commented on code in PR #2784:
URL: https://github.com/apache/groovy/pull/2784#discussion_r3783456429
##########
src/test/groovy/org/codehaus/groovy/classgen/asm/SwitchExpressionBytecodeTest.groovy:
##########
@@ -0,0 +1,143 @@
+/*
+ * 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.junit.jupiter.api.Test
+
+/**
+ * GROOVY-12255: bytecode shape of first-class switch expressions — no closure
+ * wrapper, and tableswitch / lookupswitch when the selector and case labels
+ * permit it.
+ */
+final class SwitchExpressionBytecodeTest extends AbstractBytecodeTestCase {
+
+ @Test
+ void noClosureAllocationForSwitchExpression() {
+ def bytecode = compile(method: 'run', '''\
+ def r = switch (1) {
+ case 1 -> 'a'
+ default -> 'z'
+ }
+ ''')
+ assert !bytecode.hasStrictSequence(['INVOKESPECIAL',
'org/codehaus/groovy/runtime/callsite'])
+ assert !bytecode.toString().contains('InnerClassNode')
+ assert !bytecode.toString().contains('$_run_closure')
Review Comment:
I think this test only works if Closures stay the same in their bytecode
usage. Callsite may not be in use anymore because of Indy. Not sure where
InnerClassNode is coming from and the $_run_closure method may also change. So
this test is only reliable with you also test a Closure usage for those things
to exist. And it should be in the same test to ensure the tests are not "fixed"
independently.
##########
src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java:
##########
@@ -1160,26 +1141,34 @@ public void visitThrowStatement(ThrowStatement
statement) {
}
}
- if (!(exprOrBlockStatement instanceof ReturnStatement ||
exprOrBlockStatement instanceof ThrowStatement)) {
+ if (!(exprOrBlockStatement instanceof YieldStatement ||
exprOrBlockStatement instanceof ThrowStatement)) {
if (isArrow) {
- MethodCallExpression callClosure = callX(
- configureAST(
- closureX(null,
exprOrBlockStatement),
- exprOrBlockStatement
- ), CALL_STR);
- callClosure.setImplicitThis(false);
- Expression resultExpr = exprOrBlockStatement
instanceof ExpressionStatement
- ? ((ExpressionStatement)
exprOrBlockStatement).getExpression()
- : callClosure;
-
- codeBlock = configureAST(
- createBlockStatement(configureAST(
- returnS(resultExpr),
- exprOrBlockStatement
- )),
- exprOrBlockStatement
- );
+ if (exprOrBlockStatement instanceof
ExpressionStatement expressionStatement) {
+ codeBlock = configureAST(
+ createBlockStatement(configureAST(
+
yieldS(expressionStatement.getExpression()),
+ exprOrBlockStatement
+ )),
+ exprOrBlockStatement
+ );
+ } else if
(!containsYieldOrThrow(exprOrBlockStatement)) {
+ throw createParsingFailedException("`yield` or
`throw` is expected", exprOrBlockStatement);
+ } else {
+ codeBlock = configureAST(
+
createBlockStatement(exprOrBlockStatement),
+ exprOrBlockStatement
+ );
+ }
}
+ } else if (exprOrBlockStatement instanceof YieldStatement
|| exprOrBlockStatement instanceof ThrowStatement) {
+ codeBlock = configureAST(
+ createBlockStatement(exprOrBlockStatement),
+ exprOrBlockStatement
+ );
+ }
+
+ if (isArrow &&
org.codehaus.groovy.ast.tools.GeneralUtils.maybeFallsThrough(codeBlock)) {
Review Comment:
why org.codehaus.groovy.ast.tools.GeneralUtils? Above I see plenty of static
imports for parts of that class why not import the method here as well? If not,
then why not import the class itself? Is there already a class of that name
that conflicts here that I overlooked?
##########
src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java:
##########
@@ -1020,27 +1021,13 @@ public Expression visitSwitchExprAlt(final
SwitchExprAltContext ctx) {
}
/**
- * <pre>
- * switch(x) {
- * case 0, 1 -> 'a'
- * case 2 -> 'b'
- * default -> 'z'
- * }
- * </pre>
- * will be transformed to:
- * <pre>
- * { ->
- * switch(x) {
- * case 0:
- * case 1: return 'a'
- * case 2: return 'b'
- * default: return 'z'
- * }
- * }.call()
- * </pre>
+ * Builds a first-class {@link SwitchExpression} (GROOVY-12255 / JEP 361).
+ * Arrow arms that are a single expression become {@link YieldStatement}s;
+ * colon arms use explicit {@code yield}. The expression is compiled inline
+ * — it is not rewritten to a closure wrapping a {@link SwitchStatement}.
Review Comment:
AI has the tendency to add comments like this, but I don't think they are
good in this case. I suggest to remove "The expression is compiled inline — it
is not rewritten to a closure wrapping a {@link SwitchStatement}." Referencing
the JEP is good. Referencing GROOVY-12255 is also not something I would do
here. It is the base for the change, yes. And the base for the comment part I
just suggested to delete. But the information for the changes is in the git
history. I mean it does also not reference GROOVY-9272 for example. And why
should it? As a comment in the code, that this and that is special because of a
certain issue is ok for me, just I find it misplaced in the javadoc in this
case.
##########
src/spec/test/SemanticsTest.groovy:
##########
@@ -184,11 +184,28 @@ final class SemanticsTest {
case 'Adam' -> 'Eve'
case 'Antony' -> 'Cleopatra'
case 'Bonnie' -> 'Clyde'
+ default -> 'Unknown'
Review Comment:
why did you add the "default"?
##########
src/test/groovy/org/codehaus/groovy/classgen/Groovy12255.groovy:
##########
@@ -0,0 +1,670 @@
+/*
+ * 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
+
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.ExpressionTransformer
+import org.codehaus.groovy.ast.expr.SwitchExpression
+import org.codehaus.groovy.ast.stmt.CaseStatement
+import org.codehaus.groovy.ast.stmt.YieldStatement
+import org.codehaus.groovy.ast.tools.GeneralUtils
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+
+/**
+ * GROOVY-12255: first-class switch expressions (JEP 361) for dynamic and
static Groovy.
+ * Compiles as {@code SwitchExpression} / {@code YieldStatement}, not as a
+ * closure wrapping a switch statement.
+ */
+final class Groovy12255 {
+
+ @Test
+ void arrowExpressionArms() {
+ assertScript '''
+ def letter = switch (2) {
+ case 1 -> 'a'
+ case 2 -> 'b'
+ default -> 'z'
+ }
+ assert letter == 'b'
+ '''
+ }
+
+ @Test
+ void commaSeparatedArrowLabels() {
+ assertScript '''
+ def n = switch (8) {
+ case 6, 8, 10 -> 3
+ default -> 0
+ }
+ assert n == 3
+ '''
+ }
+
+ @Test
+ void yieldInArrowBlock() {
+ assertScript '''
+ def n = switch (2) {
+ case 1 -> 10
+ case 2 -> {
+ int doubled = 2 * 10
+ yield doubled
+ }
+ default -> 0
+ }
+ assert n == 20
+ '''
+ }
+
+ @Test
+ void colonStyleWithYieldAndFallThrough() {
+ assertScript '''
+ def s = 'Bar'
+ int result = switch (s) {
+ case 'Foo':
+ yield 1
+ case 'Bar':
+ // fall through
+ case 'Baz':
+ yield 2
+ default:
+ yield 0
+ }
+ assert result == 2
+ '''
+ }
+
+ @Test
+ void throwFromArm() {
+ def err = shouldFail(RuntimeException, '''
+ def x = 9
+ def r = switch (x) {
+ case 1 -> 1
+ default -> throw new RuntimeException('nope')
+ }
+ ''')
+ assert err.message == 'nope'
+ }
+
+ @Test
+ void unmatchedSelectorThrows() {
+ def err = shouldFail(IllegalStateException, '''
+ def r = switch (99) {
+ case 1 -> 1
+ }
+ ''')
+ assert err.message.contains('does not cover')
+ }
+
+ @Test
+ void groovyIsCaseMatching() {
+ assertScript '''
+ def r = switch ('abc') {
+ case String -> 'str'
+ case Integer -> 'int'
+ default -> 'other'
+ }
+ assert r == 'str'
+
+ r = switch (5) {
+ case 1..10 -> 'range'
+ default -> 'out'
+ }
+ assert r == 'range'
+
+ r = switch ('hello') {
+ case ~/h.*/ -> 're'
+ default -> 'no'
+ }
+ assert r == 're'
+
+ r = switch (4) {
+ case { it % 2 == 0 } -> 'even'
+ default -> 'odd'
+ }
+ assert r == 'even'
+ '''
+ }
+
+ @Test
+ void nestedSwitchExpressions() {
+ assertScript '''
+ def r = switch (1) {
+ case 1 -> switch (2) {
+ case 2 -> 'inner'
+ default -> 'x'
+ }
+ default -> 'outer'
+ }
+ assert r == 'inner'
+ '''
+ }
+
+ @Test
+ void usedAsStatement() {
+ assertScript '''
+ int n = 0
+ switch (1) {
+ case 1 -> n += 1
+ default -> n += 10
+ }
+ assert n == 1
+ '''
+ }
+
+ @Test
+ void assignToOuterLocal() {
+ assertScript '''
+ int acc = 0
+ def r = switch (1) {
+ case 1 -> {
+ acc = 7
+ yield acc
+ }
+ default -> 0
+ }
+ assert r == 7
+ assert acc == 7
+ '''
+ }
+
+ @Test
+ void compileStaticArrowAndYield() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ def meth(int a) {
+ switch (a) {
+ case 1 -> 'one'
+ case 2 -> {
+ yield 'two'
+ }
+ default -> 'many'
+ }
+ }
+ assert meth(1) == 'one'
+ assert meth(2) == 'two'
+ assert meth(9) == 'many'
+ '''
+ }
+
+ @Test
+ void compileStaticStringSwitch() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ String partner(String person) {
+ switch (person) {
+ case 'Romeo' -> 'Juliet'
+ case 'Adam' -> 'Eve'
+ default -> 'Unknown'
+ }
+ }
+ assert partner('Romeo') == 'Juliet'
+ assert partner('Adam') == 'Eve'
+ assert partner('X') == 'Unknown'
+ '''
+ }
+
+ @Test
+ void compileStaticEnumSwitch() {
+ assertScript '''
+ import java.time.Month
+ import static java.time.Month.*
+
+ @groovy.transform.CompileStatic
+ String quarter(Month month) {
+ switch (month) {
+ case JANUARY, FEBRUARY, MARCH -> 'Q1'
+ case APRIL, MAY, JUNE -> 'Q2'
+ case JULY, AUGUST, SEPTEMBER -> 'Q3'
+ case OCTOBER, NOVEMBER, DECEMBER -> 'Q4'
+ }
+ }
+ assert quarter(JUNE) == 'Q2'
+ assert quarter(DECEMBER) == 'Q4'
+ '''
+ }
+
+ @Test
+ void yieldMethodNameOutsideSwitch() {
+ assertScript '''
+ def yield(String msg) { msg }
+ assert yield('ok') == 'ok'
+ '''
+ }
+
+ @Test
+ void primitiveResult() {
+ assertScript '''
+ int n = switch (2) {
+ case 1 -> 10
+ case 2 -> 20
+ default -> 0
+ }
+ assert n == 20
+ '''
+ }
+
+ @Test
+ void yieldInsideTryFinally() {
+ assertScript '''
+ def log = []
+ def r = switch (1) {
+ case 1 -> {
+ try {
+ yield 42
+ } finally {
+ log << 'fin'
+ }
+ }
+ default -> 0
+ }
+ assert r == 42
+ assert log == ['fin']
+ '''
+ }
+
+ @Test
+ void nullSelectorUsesDefaultDynamically() {
+ assertScript '''
+ def r = switch (null) {
+ case 1 -> 'one'
+ default -> 'none'
+ }
+ assert r == 'none'
+ '''
+ }
+
+ @Test
+ void defaultOnly() {
+ assertScript '''
+ assert 7 == switch (99) {
+ default -> 7
+ }
+ '''
+ }
+
+ @Test
+ void tryFinallyAroundSwitchExpression() {
+ assertScript '''
+ def log = []
+ def r = null
+ try {
+ r = switch (1) {
+ case 1 -> 42
+ default -> 0
+ }
+ } finally {
+ log << 'outer'
+ }
+ assert r == 42
+ assert log == ['outer']
+ '''
+ }
+
+ @Test
+ void synchronizedAroundSwitchExpression() {
+ assertScript '''
+ def lock = new Object()
+ def r
+ synchronized (lock) {
+ r = switch (1) {
+ case 1 -> 7
+ default -> 0
+ }
+ }
+ assert r == 7
+ '''
+ }
+
+ @Test
+ void compileStaticDefiniteAssignmentAfterYield() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ int meth(int n) {
+ int x
+ int r = switch (n) {
+ case 1 -> {
+ x = 1
+ yield 10
+ }
+ default -> {
+ x = 2
+ yield 20
+ }
+ }
+ return r + x
+ }
+ assert meth(1) == 11
+ assert meth(0) == 22
+ '''
+ }
+
+ @Test
+ void nestedExpressionInsideSwitchStatementDifferentEnums() {
+ assertScript '''
+ enum Color { RED, BLUE }
+ enum Size { S, L }
+
+ @groovy.transform.CompileStatic
+ int meth(Color color, Size size) {
+ switch (color) {
+ case RED:
+ return switch (size) {
+ case S -> 1
+ case L -> 2
+ }
+ case BLUE:
+ return switch (size) {
+ case S -> 3
+ case L -> 4
+ }
+ }
+ }
+ assert meth(Color.RED, Size.S) == 1
+ assert meth(Color.BLUE, Size.L) == 4
+ '''
+ }
+
+ @Test
+ void returnInsideLoopInSwitchExpressionIsError() {
+ def err = shouldFail('''
+ def r = switch (1) {
+ case 1 -> {
+ for (;;) {
+ return 1
+ }
+ }
+ default -> 0
+ }
+ ''')
+ assert err.message.contains('does not support `return`')
+ }
+
+ @Test
+ void switchExpressionInsideClosure() {
+ assertScript '''
+ def r = { int n ->
+ switch (n) {
+ case 1 -> 'one'
+ default -> 'other'
+ }
+ }(1)
+ assert r == 'one'
+ '''
+ }
+
+ @Test
+ void compileStaticNullStringSelectorUsesDefault() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ String m(String s) {
+ switch (s) {
+ case 'Foo' -> 'a'
+ case 'Bar' -> 'b'
+ default -> 'dflt'
+ }
+ }
+ assert m('Foo') == 'a'
+ assert m(null) == 'dflt'
+ '''
+ }
+
+ @Test
+ void compileStaticNullEnumSelectorUsesDefault() {
+ assertScript '''
+ import java.time.DayOfWeek
+
+ @groovy.transform.CompileStatic
+ String m(DayOfWeek d) {
+ switch (d) {
+ case DayOfWeek.MONDAY -> 'mon'
+ default -> 'dflt'
+ }
+ }
+ assert m(DayOfWeek.MONDAY) == 'mon'
+ assert m(null) == 'dflt'
+ '''
+ }
+
+ @Test
+ void compileStaticNullSelectorOnExhaustiveEnumThrows() {
+ def err = shouldFail(IllegalStateException, '''
+ enum Flag { ON, OFF }
+
+ @groovy.transform.CompileStatic
+ String m(Flag f) {
+ switch (f) {
+ case Flag.ON -> 'on'
+ case Flag.OFF -> 'off'
+ }
+ }
+ m(null)
+ ''')
+ assert err.message.contains('does not cover')
+ }
+
+ @Test
+ void compileStaticNullIntegerSelectorUsesDefault() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ String m(Integer n) {
+ switch (n) {
+ case 1 -> 'one'
+ case 2 -> 'two'
+ default -> 'dflt'
+ }
+ }
+ assert m(1) == 'one'
+ assert m(null) == 'dflt'
+ '''
+ }
+
+ @Test
+ void labeledBreakOutOfSwitchExpressionIsError() {
+ def err = shouldFail('''
+ outer:
+ while (true) {
+ def r = switch (1) {
+ case 1 -> {
+ for (;;) { break outer }
+ yield -1
+ }
+ default -> 0
+ }
+ }
+ ''')
+ assert err.message.contains("cannot break to label 'outer'")
+ }
+
+ @Test
+ void labeledContinueOutOfSwitchExpressionIsError() {
+ def err = shouldFail('''
+ outer:
+ while (true) {
+ def r = switch (1) {
+ case 1 -> {
+ for (;;) { continue outer }
+ yield -1
+ }
+ default -> 0
+ }
+ }
+ ''')
+ assert err.message.contains("cannot continue to label 'outer'")
+ }
+
+ @Test
+ void labeledBreakToArmLocalLoopIsAllowed() {
+ assertScript '''
+ def r = switch (1) {
+ case 1 -> {
+ int n = 0
+ inner:
+ for (;;) {
+ n += 1
+ if (n > 2) break inner
+ }
+ yield n
+ }
+ default -> 0
+ }
+ assert r == 3
+ '''
+ }
+
+ @Test
+ void compileStaticEnumSwitchWithUnqualifiedConstantNames() {
+ assertScript '''
+ import java.time.Month
+
+ @groovy.transform.CompileStatic
+ String m(Month month) {
+ switch (month) {
+ case JANUARY -> 'jan'
+ case JUNE -> 'jun'
+ default -> 'other'
+ }
+ }
+ assert m(Month.JANUARY) == 'jan'
+ assert m(Month.JUNE) == 'jun'
+ assert m(Month.MARCH) == 'other'
+ '''
+ }
+
+ @Test
+ void compileStaticEnumSwitchLocalVariableShadowingConstantName() {
+ assertScript '''
+ import java.time.Month
+
+ @groovy.transform.CompileStatic
+ String m(Month month, Month JANUARY) {
+ switch (month) {
+ case JANUARY -> 'matched local'
+ default -> 'other'
+ }
+ }
+ assert m(Month.JUNE, Month.JUNE) == 'matched local'
+ assert m(Month.JANUARY, Month.JUNE) == 'other'
+ '''
+ }
+
+ @Test
+ void compileStaticStringSwitchWithHashCollision() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ String m(String s) {
+ switch (s) {
+ case 'Aa' -> 'first' // 'Aa' and 'BB' share a hashCode,
+ case 'BB' -> 'second' // exercising the equals chain
+ default -> 'none'
+ }
+ }
+ assert m('Aa') == 'first'
+ assert m('BB') == 'second'
+ assert m('Cc') == 'none'
+ '''
+ }
+
+ @Test
+ void compileStaticSparseIntKeysStillDispatch() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ switch (n) {
+ case 1 -> 10
+ case 100 -> 20
+ case 1000000 -> 30
+ default -> 0
+ }
+ }
+ assert m(1) == 10
+ assert m(100) == 20
+ assert m(1000000) == 30
+ assert m(7) == 0
+ '''
+ }
+
+ @Test
+ void compileStaticNonConstantLabelFallsBackToIsCase() {
+ assertScript '''
+ @groovy.transform.CompileStatic
+ String m(int n) {
+ switch (n) {
+ case 1 -> 'one'
+ case 300..400 -> 'range'
+ default -> 'other'
+ }
+ }
+ assert m(1) == 'one'
+ assert m(350) == 'range'
+ assert m(7) == 'other'
+ '''
+ }
+
+ @Test
+ void switchExpressionNodeApi() {
+ def se = GeneralUtils.switchX(GeneralUtils.constX(1),
+ [new CaseStatement(GeneralUtils.constX(1),
GeneralUtils.stmt(GeneralUtils.constX('a')))],
+ GeneralUtils.stmt(GeneralUtils.constX('z')))
+ se.addCase(new CaseStatement(GeneralUtils.constX(2),
GeneralUtils.yieldS(GeneralUtils.constX('b'))))
+ assert se.text.startsWith('switch (')
+ assert se.toString().contains('cases')
+ se.expression = GeneralUtils.constX(3)
+ assert se.expression.text == '3'
+ se.defaultStatement = GeneralUtils.yieldS(GeneralUtils.constX('y'))
+ assert se.defaultStatement instanceof YieldStatement
+ assert se.defaultStatement.text == "yield y"
+ def copy = se.transformExpression(new ExpressionTransformer() {
+ @Override
+ Expression transform(Expression expression) { expression }
+ })
+ assert copy instanceof SwitchExpression
+ assert copy.caseStatements.size() == 2
+ assert copy.caseStatements[1].arrow == se.caseStatements[1].arrow
+ }
+
+ @Test
+ void yieldThroughNestedClosureIsError() {
+ def err = shouldFail('''
+ def r = switch (1) {
+ case 1 -> {
+ def c = { yield 1 }
+ yield c()
+ }
+ default -> 0
+ }
+ ''')
+ assert err.message.contains('yield cannot jump through a closure or
lambda')
Review Comment:
I actually would like to see here two things additionally: (1) async usage
where yield in the closure is then valid. (2) a switch statement in a switch
expression that uses yield.
> 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)
