[
https://issues.apache.org/jira/browse/GROOVY-12161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095927#comment-18095927
]
ASF GitHub Bot commented on GROOVY-12161:
-----------------------------------------
codecov-commenter commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959600246
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2706?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 69.0906%. Comparing base
([`86c6648`](https://app.codecov.io/gh/apache/groovy/commit/86c6648d1e61436fe513afaa85d8d0c403de3cda?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`47a13fc`](https://app.codecov.io/gh/apache/groovy/commit/47a13fc0cde67baf89ab70d1725d694d85a66c53?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2706?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2706 +/- ##
==================================================
- Coverage 69.1077% 69.0906% -0.0172%
- Complexity 34235 34239 +4
==================================================
Files 1537 1537
Lines 129356 129365 +9
Branches 23503 23509 +6
==================================================
- Hits 89395 89379 -16
- Misses 31941 31962 +21
- Partials 8020 8024 +4
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2706?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...org/codehaus/groovy/classgen/asm/CompileStack.java](https://app.codecov.io/gh/apache/groovy/pull/2706?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FCompileStack.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9Db21waWxlU3RhY2suamF2YQ==)
| `86.3081% <ø> (-0.2773%)` | :arrow_down: |
|
[.../codehaus/groovy/classgen/asm/StatementWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2706?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FStatementWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9TdGF0ZW1lbnRXcml0ZXIuamF2YQ==)
| `96.9697% <100.0000%> (+0.0785%)` | :arrow_up: |
... and [16 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2706/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> Omit identity catch-all for try/catch without finally and slim exception
> bytecode
> ---------------------------------------------------------------------------------
>
> Key: GROOVY-12161
> URL: https://issues.apache.org/jira/browse/GROOVY-12161
> Project: Groovy
> Issue Type: Improvement
> Reporter: Daniel Sun
> Priority: Major
>
> h2. Summary
> {{StatementWriter.writeTryCatchFinally}} currently emits a catch-all identity
> rethrow ({{astore}} / {{aload}} / {{athrow}}) and an empty shared-finally
> path for *every* try/catch, including plain try/catch with no finally. That
> diverges from javac, bloats the exception table, and adds avoidable
> control-flow noise that is less JIT-friendly.
> h2. Problem
> For a plain try/catch (empty finally), the generator still:
> * registers a catch-all handler that only stores and rethrows the throwable
> * routes normal fall-through through an empty "finally" block and a second
> {{GOTO}} past the catch-all
> * always creates a catch-side {{BlockRecorder}} even when there are no catch
> clauses
> On abrupt exits ({{return}} / {{break}} / {{continue}}),
> {{CompileStack.applyBlockRecorder}} also emits a trailing {{NOP}} after
> inlined finally/synchronized guards, which is unnecessary once the restarted
> range can bind to the next real instruction.
> Non-empty finally must keep a real catch-all so uncaught exceptions still run
> finally then rethrow. Nested try/catch inside try/finally must keep
> {{BlockRecorder}} range splitting so an enclosing finally inlined on
> {{return}} is *not* covered by an inner typed handler (regression risk for
> GROOVY-8229). Stack-map casts for assignments inside try (GROOVY-9805)
> continue to depend on an active {{BlockRecorder}}.
> h2. Proposed approach
> * Keep a {{BlockRecorder}} on try/catch regions always (empty finally ⇒ no-op
> excluded statement) so range splits, nested finally semantics, and
> GROOVY-9805 casts stay correct.
> * When finally is empty: omit the catch-all identity rethrow and the empty
> shared-finally block; fall-through jumps straight to a join label after the
> handlers.
> * When finally is non-empty: keep typed handlers first, then catch-all +
> shared fall-through finally (existing semantics).
> * Only push a catch-side {{BlockRecorder}} when there is at least one
> {{catch}}.
> * In {{applyBlockRecorder}}, keep a leading {{NOP}} so abrupt-exit-only try
> ranges stay non-empty (illegal empty exception ranges), but drop the trailing
> {{NOP}}.
> h2. Expected bytecode shape (illustrative)
> Plain try/catch after change — typed handlers only, no catch-all:
> {code:java}
> int m(int x) {
> try {
> return x
> } catch (RuntimeException e) {
> return -1
> }
> }
> {code}
> Exception table should list only {{RuntimeException}} (possibly multiple
> ranges after return-path range splits), not a {{null}}/any handler, and must
> not contain an adjacent {{ASTORE}}/{{ALOAD}}/{{ATHROW}} identity rethrow.
> Try/finally must still have a catch-all any-handler so finally runs for
> uncaught exceptions.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
