paulk-asert commented on PR #2784:
URL: https://github.com/apache/groovy/pull/2784#issuecomment-5289028279
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
