Copilot commented on code in PR #2922:
URL: https://github.com/apache/groovy/pull/2922#discussion_r3987321373
##########
src/main/java/org/codehaus/groovy/classgen/asm/SwitchExpressionWriter.java:
##########
@@ -213,12 +215,31 @@ protected void writeDefaultOrThrow(final SwitchExpression
expression, final int
if (defaultStatement != null && !defaultStatement.isEmpty()) {
defaultStatement.visit(controller.getAcg());
if (maybeFallsThrough(defaultStatement)) {
- throwUnmatchedSelector(selectorIndex, selectorType);
+ writeUnmatchedSelector(expression, selectorIndex,
selectorType);
}
return;
}
if (completeEnum) {
throwIncompatibleClassChangeError();
+ } else {
+ writeUnmatchedSelector(expression, selectorIndex, selectorType);
Review Comment:
This branch takes precedence over `UNMATCHED_YIELDS_NULL`. Consequently, a
complete enum switch in implicit-return position still fails under
`@CompileStatic`: an unmatched new enum constant reaches
`IncompatibleClassChangeError`, and the static enum writer's null-selector path
directly calls `throwUnmatchedSelector`. Both paths must consult the new
metadata so implicit-return switches consistently yield `null`.
##########
src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java:
##########
@@ -2449,18 +2450,101 @@ public Statement visitBlock(final BlockContext ctx) {
@Override
public Statement visitCommandExprAlt(final CommandExprAltContext ctx) {
Expression expr = this.visitCommandExpression(ctx.commandExpression());
- // Statement-position `switch` with `->` is parsed as a
SwitchExpression
- // (switchStatement is colon-only). If an arrow block does not yield,
- // rewrite it to SwitchStatement so the block need not yield (JEP 361
- // statement rules). Expression-position incomplete arms already
errored.
- if (expr instanceof SwitchExpression se
- && ctx.getParent() instanceof ExpressionStmtAltContext
- && switchExpressionHasIncompleteArm(se)) {
- return configureAST(switchExpressionAsStatement(se), ctx);
+ // A `switch` with `->` is parsed as a SwitchExpression
(switchStatement is
+ // colon-only), but as in Java (JEP 361) the arrow only decides
fall-through;
+ // the position decides whether a value is produced. A switch whose
value is
+ // not used, or whose arrow block arms do not yield, is a
SwitchStatement.
+ // In implicit-return position (the last statement of a method,
closure or
+ // script body) the value is used, but an unmatched selector yields
null
+ // rather than throwing, as it did in 4.x/5.x (GROOVY-12399).
+ if (expr instanceof SwitchExpression se && ctx.getParent() instanceof
ExpressionStmtAltContext statement) {
+ if (switchExpressionHasIncompleteArm(se) ||
!isSwitchValueUsed(statement)) {
+ return configureAST(switchExpressionAsStatement(se), ctx);
Review Comment:
The statement/expression decision occurs only after `visitCommandExpression`
has visited every arm. During that visit, `visitReturnStmtAlt` still rejects
`return` whenever the parser stack contains a `SwitchExpression`, so a genuine
statement such as `switch (i) { case 1 -> { return 'one' } }` remains a
compilation error. Determine statement position before visiting the arms, or
make the control-transfer checks consult that position.
This issue also appears in the following locations of the same file:
- line 2478
- line 2506
--
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]
