GROOVY-12255: Compile switch expressions as first-class AST (no closure desugar) by daniellansun · Pull Request #2784 · apache/groovy

daniellansun · 2026-08-13T17:33:10Z

https://issues.apache.org/jira/browse/GROOVY-12255

…re desugar)

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite	Current: `257866e`	Previous: `ad907ac`	Ratio
`org.apache.groovy.bench.dispatch.CallsiteBench.dispatch_8_megamorphic_java`	`2366.43538113778` ops/ms	`1495.2458815769692` ops/ms	`1.58`

This comment was automatically generated by workflow using github-action-benchmark.

codecov-commenter · 2026-08-13T18:11:24Z

Codecov Report

❌ Patch coverage is 82.52185% with 140 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.1888%. Comparing base (ad907ac) to head (7942320).
⚠️ Report is 33 commits behind head on master.

Files with missing lines	Patch %	Lines
...us/groovy/classgen/asm/SwitchExpressionWriter.java	77.0833%	14 Missing and 19 partials ⚠️
...roovy/transform/stc/StaticTypeCheckingVisitor.java	70.7692%	8 Missing and 11 partials ⚠️
...sgen/asm/sc/StaticTypesSwitchExpressionWriter.java	91.5493%	4 Missing and 14 partials ⚠️
...va/org/apache/groovy/parser/antlr4/AstBuilder.java	79.2683%	10 Missing and 7 partials ⚠️
...ava/org/codehaus/groovy/ast/GroovyCodeVisitor.java	0.0000%	8 Missing ⚠️
...oovy/macro/matcher/ContextualClassCodeVisitor.java	0.0000%	8 Missing ⚠️
...vy/groovy/console/ui/AstNodeToScriptAdapter.groovy	66.6667%	1 Missing and 5 partials ⚠️
...y/org/codehaus/groovy/macro/matcher/ASTFinder.java	0.0000%	6 Missing ⚠️
...org/codehaus/groovy/ast/expr/SwitchExpression.java	90.4762%	4 Missing ⚠️
...odehaus/groovy/classgen/FinalVariableAnalyzer.java	92.1569%	1 Missing and 3 partials ⚠️
... and 7 more

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2784        +/-   ##
==================================================
+ Coverage     70.1168%   70.1888%   +0.0720%     
- Complexity      35772      35999       +227     
==================================================
  Files            1561       1566         +5     
  Lines          132362     133084       +722     
  Branches        24331      24460       +129     
==================================================
+ Hits            92808      93410       +602     
- Misses          31156      31220        +64     
- Partials         8398       8454        +56

Files with missing lines	Coverage Δ
...aus/groovy/ast/ClassCodeExpressionTransformer.java	`100.0000% <100.0000%> (ø)`
...g/codehaus/groovy/ast/ClassCodeVisitorSupport.java	`100.0000% <100.0000%> (ø)`
...va/org/codehaus/groovy/ast/CodeVisitorSupport.java	`98.0520% <100.0000%> (+0.1646%)`	⬆️
...va/org/codehaus/groovy/ast/stmt/CaseStatement.java	`100.0000% <100.0000%> (+7.6923%)`	⬆️
...a/org/codehaus/groovy/ast/stmt/YieldStatement.java	`100.0000% <100.0000%> (ø)`
...rg/codehaus/groovy/classgen/AsmClassGenerator.java	`84.8373% <100.0000%> (+0.0563%)`	⬆️
...codehaus/groovy/classgen/VariableScopeVisitor.java	`94.1581% <100.0000%> (+0.0404%)`	⬆️
...us/groovy/classgen/asm/BinaryExpressionHelper.java	`89.1967% <100.0000%> (-0.0190%)`	⬇️
...haus/groovy/classgen/asm/DelegatingController.java	`92.1875% <100.0000%> (+0.1240%)`	⬆️
.../codehaus/groovy/classgen/asm/StatementWriter.java	`98.4018% <100.0000%> (-0.0180%)`	⬇️
... and 22 more

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

github-actions · 2026-08-13T18:22:03Z

JMH summary — classic (commit `72e3ae6`)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group	Speedup	Calibrated	n
bench	0.957 ×	0.976 ×	99
core	1.405 ×	1.040 ×	83
grails	1.022 ×	0.975 ×	80

⚠️ 1 benchmark at least 1.5× slower than the 90-day baseline:

org.apache.groovy.perf.grails.MetaclassChangeBench.burstThenSteadyState — 1.79× slower (calibrated)

⚠️ Runner speed differs ≥15% from the historical baseline hardware for: core-ag, core-hz. Raw speedups are not meaningful for those parts — use the calibrated column.

_{Runner calibration (this run vs baseline hardware): bench 0.98× (26 rulers) · core-ag 1.30× (3 rulers) · core-hz 1.43× (3 rulers) · grails-ad 0.98× (3 rulers) · grails-ez 1.11× (3 rulers)}

_{Baseline: dev/bench/jmh/<part>/classic/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data}

github-actions · 2026-08-13T18:30:14Z

JMH summary — indy (commit `72e3ae6`)

Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
1.00 = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The calibrated
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.

Group	Speedup	Calibrated	n
bench	1.231 ×	0.981 ×	99
core	2.686 ×	2.817 ×	83
grails	4.464 ×	3.730 ×	80

⚠️ 4 benchmarks at least 1.5× slower than the 90-day baseline:

org.apache.groovy.bench.AryBench.groovyCS ( {"n":"1000000"} ) — 2.44× slower (calibrated)

org.apache.groovy.bench.StaticMethodCallIndyBench.staticChain_groovyCS — 1.63× slower (calibrated)

org.apache.groovy.bench.StaticMethodCallIndyBench.instanceSum_groovy — 1.54× slower (calibrated)

org.apache.groovy.bench.AryBench.groovyCS ( {"n":"10"} ) — 1.50× slower (calibrated)

⚠️ Runner speed differs ≥15% from the historical baseline hardware for: bench, grails-ez. Raw speedups are not meaningful for those parts — use the calibrated column.

_{Runner calibration (this run vs baseline hardware): bench 1.24× (26 rulers) · core-ag 0.96× (3 rulers) · core-hz 0.94× (3 rulers) · grails-ad 0.98× (3 rulers) · grails-ez 1.41× (3 rulers)}

_{Baseline: dev/bench/jmh/<part>/indy/data.js on gh-pages, trailing 90 days. Daily dashboard · Per-suite raw data}

paulk-asert · 2026-08-14T03:23:11Z

I am still reviewing but an initial AI assessment below:

Gaps I'd close before merge:

Labeled break/continue escaping an arm: outer: while (...) { def r = switch (x) { case 1 -> { for (;;) { break outer } } } }. The parser's peek-based check sees the loop frame and allows it, and the new LabelVerifier code fences closures for yield but doesn't fence labels at the expression boundary. Java rejects this ("attempt to break out of a switch expression"); here it likely compiles to a jump that abandons the expression mid-evaluation. Needs a check plus a fail test.

Colon group that can complete normally despite containing a yield: case 'a': if (cond) yield 1 as the last group passes the parser's contains-yield check, then at runtime falls into the "does not cover" ISE even though the selector did match — Java makes this a compile error, and the runtime message is misleading. Verify and decide.

Null selector under @CS — no test for any of the three fast paths (see above).

@CS fast paths + yield inside try/finally — the finally-block stashing logic is only exercised dynamically.

Switch expressions in field initializers / constructors / static initializers under @CS — the writer leans on controller state; untested.

GINQ: the LabelVerifier javadoc explicitly calls out switch-expressions-inside-GINQ-queries as allowed, but no GINQ test was added.

SwitchExpression.transformExpression is a design smell: for a plain ExpressionTransformer it only rewrites top-level yield/throw/expression-statement expressions inside arms (if-conditions and loop conditions are missed); when the transformer is also a GroovyCodeVisitor it additionally re-visits the arm code, risking double transformation. It also mutates the original arm statements while claiming to produce a copy. Worth tightening before third-party transforms depend on it.

Compile switch expressions as first-class AST (fix review comments)

paulk-asert · 2026-08-14T05:32:50Z

@daniellansun I added a commit that potentially addresses the issues raised above. We can delete if you have another preference.

Compile switch expressions as first-class AST (improve test coverage to keep Sonar happy)

blackdrag

Additionally to what Paul said I added some review comments. Some of the comments annotate only one place, but actually mean many similar. The fully qualified class name usage is such a case.

Tighten last-arm completion, copy switch-expression trees without mutating the original, and drop historical desugar notes from javadoc.

Type the label stream, build each arm once, and drop the ad-hoc AST walk for yield/throw in favour of mayCompleteNormally.

daniellansun · 2026-08-14T16:54:45Z

Additionally to what Paul said I added some review comments. Some of the comments annotate only one place, but actually mean many similar. The fully qualified class name usage is such a case.

Thank you for the review. The inline notes are addressed below; the same FQCN cleanup was applied at the other sites introduced by this PR.

daniellansun · 2026-08-14T17:02:42Z

I am still reviewing but an initial AI assessment below:

Gaps I'd close before merge:

Labeled break/continue escaping an arm: outer: while (...) { def r = switch (x) { case 1 -> { for (;;) { break outer } } } }. The parser's peek-based check sees the loop frame and allows it, and the new LabelVerifier code fences closures for yield but doesn't fence labels at the expression boundary. Java rejects this ("attempt to break out of a switch expression"); here it likely compiles to a jump that abandons the expression mid-evaluation. Needs a check plus a fail test.

Colon group that can complete normally despite containing a yield: case 'a': if (cond) yield 1 as the last group passes the parser's contains-yield check, then at runtime falls into the "does not cover" ISE even though the selector did match — Java makes this a compile error, and the runtime message is misleading. Verify and decide.

Null selector under @CS — no test for any of the three fast paths (see above).

@CS fast paths + yield inside try/finally — the finally-block stashing logic is only exercised dynamically.

Switch expressions in field initializers / constructors / static initializers under @CS — the writer leans on controller state; untested.

GINQ: the LabelVerifier javadoc explicitly calls out switch-expressions-inside-GINQ-queries as allowed, but no GINQ test was added.

SwitchExpression.transformExpression is a design smell: for a plain ExpressionTransformer it only rewrites top-level yield/throw/expression-statement expressions inside arms (if-conditions and loop conditions are missed); when the transformer is also a GroovyCodeVisitor it additionally re-visits the arm code, risking double transformation. It also mutates the original arm statements while claiming to produce a copy. Worth tightening before third-party transforms depend on it.

Thank you — including for 9a0c682, which already closed labeled break/continue, the @CS null-selector paths, and several bytecode cases. The remaining items are handled as follows.

Gap	Response
Labeled `break` / `continue` escaping an arm	Kept your `LabelVerifier` isolation of loop/switch/label state. Added a `@CompileStatic` for-loop-in-arm case with an implicit-this call so the copied `ForStatement` keeps its `VariableScope` after `transformExpression`.
Colon group that contains a `yield` but can still complete	Last group is now rejected unless every path yields or throws (`GeneralUtils.mayCompleteNormally`, which uses the existing statement-flow analysis). Intermediate colon groups may still fall through. Covered by `lastColonArmIfWithoutElseIsError`, `fail/SwitchExpression_14x.groovy`, and `colonArmIfFallsThroughToCompletingDefault`.
Null selector under `@CS`	Your three fast-path tests remain.
`@CS` fast paths + `yield` in `try`/`finally`	Added int / `String` / enum variants (`compileStaticYieldInsideTryFinally*`).
Field / constructor / static initializer under `@CS`	Added `compileStaticSwitchExpressionInFieldInitializer`, `…InConstructor`, `…InStaticInitializer`.
GINQ	Arrow-form coverage already lived in `GinqTest` (`testGinq - switch - 1`…`6`). Added `testGinq - switch - yield block` so an explicit `yield` inside `GQ { }` is covered as well.
`transformExpression`	It now returns a structural copy: selector and case labels are transformed, arm statements are copied (not mutated), and the transformer is not also applied as a `GroovyCodeVisitor`. `AssertStatement` and `ForStatement.variableScope` are preserved. `transformExpressionCopiesArmsAndDoesNotMutateOriginal` checks that a constant inside an `if` / `yield` / `assert` is rewritten only on the copy.

Happy to adjust further if any of these should take a different shape.

paulk-asert · 2026-08-15T01:40:51Z

AI read:

Verdict: technically ready, but blocked on process; what remains is re-review/sign-off, not code work.

blackdrag

A summary for the review would be the following:

please use more of the provided infrastructure of OperandStack and CompileStack
tests where the switch is using a Reference
split between static compiler and non-static compiler and tests
maybe we can reuse some code for the normal case statement as well?

blackdrag · 2026-08-15T22:21:33Z

+     * 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) {


actually I think this does not belong in here. I think this should be its own class and become something like the ClassCodeTransformer/ClassCodeVisitorSupport. I actually think the way you use this here violates the idea of ExpressionTransformer. transformExpression is not supposed to create new statements. Which actually shows here that using CaseStatement is a logical problem for the otherwise relatively strict difference between statements and expressions we have. in fact... the code makes copies of statements inside the cases, but it does not keep all the metadata, like the metadata map, source positions or for example variable scopes. We do not know when transformExpression is called. I think for the normal switch-case statement we do not copy statements, especially not nested statements. Why should we here? So my suggestion is if you need this transformExpression call, make a new copy-visitor, that also copies metadata and use it through a different method. If you only wanted to provide a transformExpression implementation, then I do not think it is right to transform nested statements. The part on if a CaseStatement should actually be a CaseExpression for a SwitchExpression I leave mainly to you, but I think I see strong idicators for it, as it would be strange to have an expression consisting of statements.

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.

blackdrag · 2026-08-15T22:49:19Z

+                return false; // duplicate case value
+            }
+        }
+        if (keyToBody.isEmpty()) return false;


same here... why no compilation error?

blackdrag · 2026-08-15T23:00:39Z

+        ClassNode storedSelectorType = ClassHelper.isPrimitiveType(selectorType)
+                ? ClassHelper.getWrapper(selectorType)
+                : selectorType;
+        operandStack.box();


(a) if you simply do operandStack.box(); ClassNode storedSelectorType = operandStack.getTopOperand(); you would not need the primitive type check here. You are not using selectorType past this point anyway.
(b) why are you actually boxing? For a tableswitch or a lookupswitch you have to unbox again.
(c) what if the variable is a reference? For example what if the same variable is used in a switch expression and also used in a Closure, where it is written even? like
´´´
def x = 1
def cl = {x=2}
cl()
def y = switch(x)....
´´´
Then for x getTopOperand would return most likely int, But it actually is a Reference, potentially endangering all manipulation you do on that variable that does not use OperandStack. That wrong type extends to the temporary variable you define in the line after this.

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.

blackdrag · 2026-08-15T23:07:22Z

+        caseStatement.getExpression().visit(acg);
+        operandStack.box();
+        effective.getBinaryExpressionHelper().getIsCaseMethod().call(mv);
+        operandStack.replace(ClassHelper.boolean_TYPE);


why not emit a method call here and let others handle this properly? This looks like it enforces the dynamic isCase call even in static compilation.

Agreed. The static writer emits caseValue.isCase(selector) as a
MethodCallExpression with a resolved target (instance isCase, or a
DGM overload more specific than Object). Only if that resolution is
ambiguous does it fall back to
BinaryExpressionHelper.writeIsCase (ScriptBytecodeAdapter). The
dynamic writer uses that shared helper from the start.

blackdrag · 2026-08-15T23:50:23Z

+        int nameLocal = compileStack.defineTemporaryVariable("$switchEnumName", ClassHelper.STRING_TYPE, true);
+        int caseIndexLocal = compileStack.defineTemporaryVariable("$switchCase", ClassHelper.int_TYPE, false);
+
+        List<String> ordered = new ArrayList<>(nameToBody.keySet());


Besides the cases that should probably be a compilation error you do not need a map. You could work with a List from the get-go.

Agreed. Keys and jump targets are collected as parallel lists. A map is
kept only where the algorithm needs one (duplicate detection, hash
buckets for the Java 7 string dispatch, sorted int keys for
tableswitch).

blackdrag · 2026-08-15T23:51:49Z

+            }
+            if (!caseStatement.getCode().isEmpty()) {
+                caseStatement.getCode().visit(acg);
+            }


same question for EmptyStatement.

blackdrag · 2026-08-16T00:14:34Z

+        CompileStack.SwitchExpressionContext context = compileStack.getSwitchExpressionContext();
+        if (context == null) {
+            throw new GroovyBugError("yield outside of a switch expression");
+        }


I think that check should be in CompileStack

Agreed. CompileStack.requireSwitchExpressionContext() now owns the
guard; writeYield just uses the returned context.

blackdrag · 2026-08-16T01:10:13Z

+                }
+            }
+        ''')
+        assert bytecode.hasSequence(['TABLESWITCH']) || bytecode.hasSequence(['LOOKUPSWITCH'])


why "or"? It is either one or the other. Same for staticStringSwitchUsesLookupSwitch. Also this is a static compilation test so far, thus it should be under sc

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.

Move tableswitch/lookupswitch and resolved isCase onto StaticTypesSwitchExpressionWriter. Store and load the selector through OperandStack so Reference-backed locals stay correct, share isCase emission with switch statements, and stop copying arm statements in transformExpression.

daniellansun · 2026-08-16T04:57:01Z

A summary for the review would be the following:

please use more of the provided infrastructure of OperandStack and CompileStack

tests where the switch is using a Reference

split between static compiler and non-static compiler and tests

maybe we can reuse some code for the normal case statement as well?

Agreed on all four.

Selector load/store, yield stashing, string/enum casts, and the unmatched
throw now go through OperandStack / CompileStack (load, box,
doGroovyCast, storeVar via defineTemporaryVariable,
requireSwitchExpressionContext). Raw ALOAD / CHECKCAST on those
paths is gone.
Reference-selector tests cover a closure-shared local that is written
before the switch, both dynamically and under @CompileStatic (int and
Integer), plus the same pattern on a switch statement.
Sequential isCase lives on SwitchExpressionWriter. Tableswitch /
lookupswitch and resolved isCase live on
StaticTypesSwitchExpressionWriter, installed from
StaticTypesWriterController only while
isInStaticallyCheckedMethod. Dynamic bytecode tests stay in
SwitchExpressionBytecodeTest; static ones moved to
asm/sc/SwitchExpressionStaticCompileTest.
BinaryExpressionHelper.writeIsCase is now shared with
StatementWriter.writeCaseStatement.

daniellansun · 2026-08-16T09:46:17Z

@blackdrag

yield can be used with colon:

int result = switch (s) {
    case "Foo": 
        yield 1;
    case "Bar":
        yield 2;
    default:
        System.out.println("Neither Foo nor Bar, hmmm...");
        yield 0;
};

Quality Gate passed

Issues
15 New issues
0 Accepted issues

Measures
0 Security Hotspots
85.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

blackdrag

I think we are almost there. The split looks promising, the type extension issue is in my opinion the only thing that really blocks this.

blackdrag · 2026-08-17T18:05:06Z

+    }
+
+    @Test
+    void compileStaticYieldInsideTryFinallyStringSwitch() {


why is this specific to static compilation? Also why does it matter if r is int or String?

blackdrag · 2026-08-17T18:06:05Z

+    }
+
+    @Test
+    void compileStaticYieldInsideTryFinallyIntSwitch() {


why is this specific to static compilation?

blackdrag · 2026-08-17T18:08:08Z

+                    if (true) yield 1
+            }
+        ''')
+        assert err.message.contains('yield') || err.message.contains('throw')


again the question as of why yield or throw?

blackdrag · 2026-08-17T18:20:11Z

+    @Test
+    void compileStaticSparseIntKeysStillDispatch() {
+        assertScript '''
+            @groovy.transform.CompileStatic


I am randomly using this line to anchor the comment, but it actually is about several places with CompileStatic. You already have tests that ensure the bytecode contains structures you expect from static compilation. Which means I assume the test here is to finalize that the behavior is consistent. But for this you need a base to compare to, which should be the same test without static compilation. So I suggest you do something like

def script = """...""" assertScript script assertScript "@groovy.transform.CompileStatic\n" + script

And that way you ensure baseline (dynamic Groovy) and static compiler align in behavior. You should look at each @CompileStatic using test in this class and ask yourself if it is really specific to the static compiler. If not you should change it like suggested, if it is special, then is should go into the static compilation test suite instead - or at least the test should have something explaining why this is only with static compilation.

blackdrag · 2026-08-17T18:28:26Z

+                }
+            }
+        ''')
+        assert err.message.contains("cannot continue to label 'outer'")


This is a compilation error, right? You should assert the exception type as well.

blackdrag · 2026-08-17T18:28:54Z

+                }
+            }
+        ''')
+        assert err.message.contains("cannot break to label 'outer'")


same case as for continue

blackdrag · 2026-08-17T18:48:57Z

+        if (transformer instanceof ClassCodeExpressionTransformer visitor) {
+            visitor.visitSwitchExpression(this);
+            return this;
+        }


I think this code should be in ClassCodeExpressionTransformer. The other part of the method looks good to me now

blackdrag · 2026-08-17T19:03:48Z

+    private MethodNode resolveIsCaseTarget(final Expression caseValue, final ClassNode selectorType) {
+        ClassNode caseType = controller.getTypeChooser().resolveType(caseValue, controller.getClassNode());
+        ClassNode switchArg = ClassHelper.isPrimitiveType(selectorType)
+                ? ClassHelper.getWrapper(selectorType) : selectorType;


minor: getWrapper is already doing this check for you.

blackdrag · 2026-08-17T20:04:24Z

+     * @since 6.0.0
+     */
+    @Override
+    public void visitSwitchExpression(final SwitchExpression expression) {


I already mentioned it before but I think I was way to unspecific and then you misunderstood me. There are basically 2 cases for the switch expression: the generic isCase variant and the intrinsic variant. I think you need to check isCase here and if it is the isCase variant, you should actually go through method selection here to have a direct method call target be chosen for isCase. If there is no target this is a compilation error. This should then handle instance and DGM, other extensions, as well as making the typechecking extensions work for the isCase call. The isCase write is then actually a direct method call write only, which is handled by writeDirectMethodCall in StaticInvocationWriter. The intrinsic cases are to be handled by StaticTypesSwitchExpressionWriter directly, while for isCase should then go through the invocation writer mechanism.

blackdrag · 2026-08-17T20:05:51Z

+    }
+
+    private static MethodNode chooseInstanceIsCase(final ClassNode caseType, final ClassNode switchArg) {
+        if (caseType == null) return null;


in combination with my comment on StaticTypeCheckingVisitor you would actually only have to check for the direct method call target here and then write it, if it exists.

…g transform Resolve a non-intrinsic case label as label.isCase(selector) in the type checker and store the chosen MethodNode on the CaseStatement so static codegen can emit a direct call. Keep ClassCodeExpressionTransformer.transform generic: SwitchExpression.transformExpression still rewrites only the selector and labels, and ResolveVisitor, StaticImportVisitor, static compilation and GINQ visit the node themselves, the same way they walk closures.

testlens-app · 2026-08-18T19:31:53Z

✅ All tests passed ✅

🏷️ Commit: b2b9b73
▶️ Tests: 457 executed
⚪️ Checks: 31/31 completed

Learn more about TestLens at testlens.app.

GROOVY-12255: Compile switch expressions as first-class AST (no closu…

11215d6

…re desugar)

daniellansun requested review from blackdrag and paulk-asert August 13, 2026 17:36

github-actions Bot reviewed Aug 13, 2026

View reviewed changes

GROOVY-12255

9a0c682

Compile switch expressions as first-class AST (fix review comments)

GROOVY-12255

425d42f

Compile switch expressions as first-class AST (improve test coverage to keep Sonar happy)

paulk-asert force-pushed the GROOVY-12255 branch from f0c6688 to 425d42f Compare August 14, 2026 08:13

blackdrag requested changes Aug 14, 2026

View reviewed changes

daniellansun added 2 commits August 15, 2026 01:25

GROOVY-12255: Address review comments on first-class switch expressions

f31fd77

Tighten last-arm completion, copy switch-expression trees without mutating the original, and drop historical desugar notes from javadoc.

GROOVY-12255: Simplify switch-expression arm building in AstBuilder

257866e

Type the label stream, build each arm once, and drop the ad-hoc AST walk for yield/throw in favour of mayCompleteNormally.

daniellansun requested a review from blackdrag August 14, 2026 20:13

paulk-asert approved these changes Aug 15, 2026

View reviewed changes

blackdrag requested changes Aug 16, 2026

View reviewed changes

daniellansun requested a review from blackdrag August 16, 2026 05:05

daniellansun added 3 commits August 16, 2026 19:06

GROOVY-12255: Slim static switch-expression dispatch and transform

d17d3c6

Collapse grouping to one ArmGroup, share colon/comma labels on the following body, jump string/enum equals straight to the arm, and let ClassCodeExpressionTransformer walk switch expressions in place.

GROOVY-12255: Cover JEP 361 switch-expression scenarios in both modes

7942320

Add a dual-mode conformance suite for the JEP 361 examples and extend STC coverage for non-exhaustive statements and target-type pushdown.

blackdrag requested changes Aug 17, 2026

View reviewed changes

Uh oh!

Conversation

daniellansun commented Aug 13, 2026

Uh oh!

github-actions Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

codecov-commenter commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JMH summary — classic (commit 72e3ae6)

Uh oh!

github-actions Bot commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

JMH summary — indy (commit 72e3ae6)

Uh oh!

paulk-asert commented Aug 14, 2026

Uh oh!

paulk-asert commented Aug 14, 2026

Uh oh!

blackdrag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

daniellansun commented Aug 14, 2026

Uh oh!

daniellansun commented Aug 14, 2026

Uh oh!

paulk-asert commented Aug 15, 2026

Uh oh!

blackdrag left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daniellansun commented Aug 16, 2026

Uh oh!

daniellansun commented Aug 16, 2026

Uh oh!

sonarqubecloud Bot commented Aug 16, 2026

Quality Gate passed

Uh oh!

github-actions Bot left a comment •

edited

Loading

codecov-commenter commented Aug 13, 2026 •

edited

Loading

github-actions Bot commented Aug 13, 2026 •

edited

Loading

JMH summary — classic (commit `72e3ae6`)

github-actions Bot commented Aug 13, 2026 •

edited

Loading

JMH summary — indy (commit `72e3ae6`)

blackdrag left a comment •

edited

Loading