[ 
https://issues.apache.org/jira/browse/GROOVY-12161?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18095928#comment-18095928
 ] 

ASF GitHub Bot commented on GROOVY-12161:
-----------------------------------------

sonarqubecloud[bot] commented on PR #2706:
URL: https://github.com/apache/groovy/pull/2706#issuecomment-4959611414

   ## [![Quality Gate 
Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png
 'Quality Gate 
Passed')](https://sonarcloud.io/dashboard?id=apache_groovy&pullRequest=2706) 
**Quality Gate passed**  
   Issues  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [2 New 
issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2706&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png
 '') [0 Accepted 
issues](https://sonarcloud.io/project/issues?id=apache_groovy&pullRequest=2706&issueStatuses=ACCEPTED)
   
   Measures  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0 Security 
Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_groovy&pullRequest=2706&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [100.0% Coverage on New 
Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2706&metric=new_coverage&view=list)
  
   
![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png
 '') [0.0% Duplication on New 
Code](https://sonarcloud.io/component_measures?id=apache_groovy&pullRequest=2706&metric=new_duplicated_lines_density&view=list)
  
     
   <!

> 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)

Reply via email to