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

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

github-actions[bot] commented on PR #2773:
URL: https://github.com/apache/groovy/pull/2773#issuecomment-5224985694

   ### JMH summary — indy (commit `59116b7`)
   
   Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
   `1.00` = in line with history. Per-benchmark ratio, geomean within group.
   Time-per-op units inverted so direction is consistent. The *calibrated*
   column divides out this runner's speed vs the baseline hardware, as
   measured by Groovy-independent pure-Java ruler benchmarks.
   
   | Group  | Speedup | Calibrated | n |
   |--------|---------|------------|---|
   | bench | 0.968 × | 0.984 × | 99 |
   | core | 2.944 × | 2.797 × | 83 |
   | grails | 4.174 × | 3.959 × | 80 |
   
   <sub>Runner calibration (this run vs baseline hardware): bench 0.98× (26 
rulers) · core-ag 1.13× (3 rulers) · core-hz 0.96× (3 rulers) · grails-ad 0.99× 
(3 rulers) · grails-ez 1.11× (3 rulers)</sub>
   
   <sub>Baseline: <code>dev/bench/jmh/&lt;part&gt;/indy/data.js</code> on 
gh-pages, trailing 90 days. <a 
href="https://apache.github.io/groovy/dev/bench/jmh/summary.html";>Daily 
dashboard</a> · <a 
href="https://apache.github.io/groovy/dev/bench/jmh/";>Per-suite raw 
data</a></sub>
   
   <!

> instanceof pattern variable scope is not aligned with Java flow scoping (JEP 
> 394)
> ---------------------------------------------------------------------------------
>
>                 Key: GROOVY-12242
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12242
>             Project: Groovy
>          Issue Type: Bug
>            Reporter: Daniel Sun
>            Priority: Major
>
> h2. Summary
> After {{instanceof}} type patterns landed in GROOVY-11229, pattern variables 
> were still scoped with a coarse lexical approximation. That diverges from 
> Java’s *flow scoping* (JEP 394): a pattern variable must be visible only 
> where the pattern has *definitely* matched.
> The gaps appear as:
>  # variables missing where Java allows them
>  # variables leaking past the statement that introduced them
>  # name resolution and bytecode disagreeing, so an “out of scope” use can 
> still load a local slot
> h2. Background
>  * GROOVY-11229 added {{e instanceof T t}} (parser, AST, store-on-match).
>  * Java (JEP 394 / JLS): scope follows boolean flow and abrupt completion, 
> not simple block poison.
>  * Groovy initially limited leakage with push/pop around statements, but did 
> not implement true/false-path binding or CompileStack polarity.
> h2. Problems (before the fix)
> ||#||Scenario||Java||Groovy (before)||
> |1|negated {{instanceof}} — use pattern var in else|in scope|missing|
> |2|negated {{instanceof}} + early {{return}} — use pattern var after if|in 
> scope|missing|
> |3|positive {{instanceof}} + abrupt else — use pattern var after if|in 
> scope|missing|
> |4|{{boolean b = (o instanceof String s)}} then use {{s}}|not in 
> scope|CompileStack leak (local still loadable)|
> |5|expression statement with pattern, then use pattern var|not in 
> scope|CompileStack leak|
> |6|type-checked: pattern var used on RHS of logical-or|error on RHS|often 
> accepted|
> |7|type-checked ternary false arm uses pattern var|error|often accepted|
> |8|negated {{instanceof}} — use pattern var in then-branch|not in scope|could 
> ALOAD unassigned local (null)|
> h2. Steps to reproduce
> h3. A. Negated instanceof — else branch (should see {{{}s{}}})
> {code:groovy}
> def f = { Object o ->
>     if (!(o instanceof String s)) {
>         return 'not'
>     } else {
>         return s.toUpperCase()   // expected: OK when o is String
>     }
> }
> assert f('hi') == 'HI'
> {code}
> h3. B. Early return after negation (should see {{s}} after if)
> {code:groovy}
> def f = { Object o ->
>     if (!(o instanceof String s)) return 'early'
>     return s.toUpperCase()       // expected: OK when o is String
> }
> assert f('hi') == 'HI'
> {code}
> h3. C. Leak after declaration (must *not* see {{{}s{}}})
> {code:groovy}
> class C {
>     Object m(Object o) {
>         boolean b = (o instanceof String s)
>         return s                 // expected: MissingPropertyException / 
> undeclared
>     }
> }
> new C().m('hi')
> {code}
> h3. D. Type-checked {{||}} RHS must not see true-path binding
> {code:groovy}
> @groovy.transform.TypeChecked
> class C {
>     static void m(Object o) {
>         if (o instanceof String s || s.length() > 0) {
>             // expected: undeclared / apparent variable s on RHS of ||
>         }
>     }
> }
> {code}
> h2. Expected behaviour
> Align with Java JEP 394 flow scoping for the common shapes:
>  * true-path bindings (e.g. {{{}e instanceof T t{}}}) live in then-blocks, 
> {{&&}} RHS, and ternary true arm
>  * false-path bindings (e.g. {{{}!(e instanceof T t){}}}) live in 
> else-blocks, after abrupt then, and the matching ternary arm
>  * pattern variables do not leak past the introducing statement (declaration 
> RHS, expression statement, …)
>  * VariableScope (names) and CompileStack (locals) agree on which path a 
> pattern local is live
> h2. Actual behaviour (before fix)
>  * Lexical push/pop approximated “no leak past statement” but not true/false 
> path polarity.
>  * CompileStack could keep pattern slots after VariableScope had dropped the 
> name (silent local load vs property miss).
>  * Negation and abrupt-completion cases from Java were not supported.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to