daniellansun commented on PR #2784:
URL: https://github.com/apache/groovy/pull/2784#issuecomment-5305806716
> 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`.
--
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]
