[
https://issues.apache.org/jira/browse/GROOVY-12255?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105056#comment-18105056
]
ASF GitHub Bot commented on GROOVY-12255:
-----------------------------------------
daniellansun commented on code in PR #2784:
URL: https://github.com/apache/groovy/pull/2784#discussion_r3790989174
##########
src/test/groovy/org/codehaus/groovy/classgen/asm/SwitchExpressionBytecodeTest.groovy:
##########
@@ -0,0 +1,154 @@
+/*
+ * 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() {
+ // Probe the current closure bytecode so the negative assertions
+ // below stay meaningful if closure codegen changes (indy, naming).
+ def closureBytecode = compile(method: 'run', '''\
+ def c = { -> 'closure' }
+ c()
+ ''')
+ def closureText = closureBytecode.toString()
+ assert closureText.contains('$_run_closure')
+
+ def switchBytecode = compile(method: 'run', '''\
+ def r = switch (1) {
+ case 1 -> 'a'
+ default -> 'z'
+ }
+ ''')
+ def switchText = switchBytecode.toString()
+ assert !switchText.contains('$_run_closure')
+ if (closureText.contains('InnerClassNode')) {
+ assert !switchText.contains('InnerClassNode')
+ }
+ }
+
+ @Test
+ void staticIntSwitchUsesTableSwitch() {
+ def bytecode = compile(method: 'm', '''\
+ @groovy.transform.CompileStatic
+ int m(int n) {
+ switch (n) {
+ case 1 -> 10
+ case 2 -> 20
+ case 3 -> 30
+ default -> 0
+ }
+ }
+ ''')
+ assert bytecode.hasSequence(['TABLESWITCH']) ||
bytecode.hasSequence(['LOOKUPSWITCH'])
Review Comment:
Agreed. Dense `int` asserts `TABLESWITCH` only; sparse `int` asserts
`LOOKUPSWITCH` only; `String` asserts the Java 7 pair (`LOOKUPSWITCH` on
`hashCode`, then `TABLESWITCH` on the case index). Those tests live in
`src/test/groovy/org/codehaus/groovy/classgen/asm/sc/SwitchExpressionStaticCompileTest.groovy`.
> 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)
