GROOVY-10156: Unreachable bytecode in switch statement#2538
Open
daniellansun wants to merge 1 commit into
Open
GROOVY-10156: Unreachable bytecode in switch statement#2538daniellansun wants to merge 1 commit into
daniellansun wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses GROOVY-10156 by refining switch bytecode generation to avoid synthetic unreachable fall-through jumps and adding regression coverage around switch control-flow cases.
Changes:
- Adds switch fall-through analysis for cases, labels, try/finally, synchronized blocks, and nested switches.
- Updates switch and synchronized bytecode emission to skip unnecessary labels/jumps.
- Adds bytecode-focused regression tests for enum switches and labeled switch scenarios.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/main/java/org/codehaus/groovy/classgen/asm/StatementWriter.java |
Uses switch flow analysis to avoid emitting unnecessary case/default jump targets and synchronized fall-through jumps. |
src/main/java/org/codehaus/groovy/classgen/asm/CompileStack.java |
Adds switch label registration for named break targets. |
src/main/java/org/codehaus/groovy/ast/tools/GeneralUtils.java |
Implements switch fall-through analysis helpers. |
src/test/groovy/bugs/Groovy10156.groovy |
Adds regression tests checking for absence of the unreachable bytecode sequence. |
Comment on lines
+1331
to
+1332
| if (localLabels.contains(label)) { | ||
| return SwitchFlow.FALL_THROUGH; |
Comment on lines
+1364
to
+1369
| return SwitchFlow.of(maybeFallsThroughSwitch(switchStatement), false); | ||
| } | ||
|
|
||
| return SwitchFlow.of(maybeFallsThrough(statement), false); | ||
| } | ||
|
|
Comment on lines
+421
to
+423
| private void assertNoUnreachableSequenceInTestMethod(final String source) { | ||
| def bytecode = compile(classNamePattern: 'C', method: 'test', source) | ||
| assert !bytecode.hasStrictSequence(UNREACHABLE_SEQUENCE) |
1cafcc0 to
06dcecf
Compare
06dcecf to
d4955f0
Compare
|
🚨 TestLens detected 49 failed tests 🚨Here is what you can do:
Test Summary
🏷️ Commit: d4955f0 Test Failures (first 5 of 49)AsyncAwaitTest > testAwaitPlainFutureWithExceptionUnwrap() (:test in Build and test / lts (17, macos-latest))AsyncAwaitTest > testAwaitPlainFutureWithExceptionUnwrap() (:test in Build and test / lts (17, macos-latest))AsyncAwaitTest > testBuiltInAdapterPlainFutureExecutionException() (:test in Build and test / lts (17, macos-latest))AsyncAwaitTest > testBuiltInAdapterPlainFutureExecutionException() (:test in Build and test / lts (17, macos-latest))AsyncAwaitTest > testBuiltInAdapterPlainFutureWithError() (:test in Build and test / lts (17, macos-latest))Muted TestsSelect tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



No description provided.