JDK-8300691 - final variables in for loop headers should accept updates

[Maurizio Cimadamore]

This is great info! Thanks.

Funny to see the FOREACHLOOP popping out. This code is (at least) from 
17 years ago (!!) and predates any form of effectively final analysis - 
back then even for-each required to hoist the variable in a separate 
|final| one :-)

While looking at the results I realized we have another issue in the 
analysis. Look at this:

https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/javax/xml/datatype/FactoryFinder.java#L177

In this case the dummy variable is reported inside the TRY, which is 
obviously correct. But, I realize, the important info is not where the 
dummy variable is declared. It's the declaration of the variable it 
points to in the initializer that matters!

In this case, while the dummy variable does appear inside a TRY block, 
the variable it snapshots is a method parameter, so this should probably 
say METHODDEF, not TRY.

This misclassification might lead to underestimating buckets. For 
instance, if you have a `for` loop, and inside the for loop you have an 
`if`, and inside the `if` we declare a dummy variable that captures a 
variable in the enclosing loop, this will show up as `IF` in the current 
code.

Cheers
Maurizio

On 17/10/2024 00:11, Archie Cobbs wrote:

> Hi Maurizio,
>
> On Wed, Oct 16, 2024 at 4:37 PM Maurizio Cimadamore 
> <maurizio.cimadamore at oracle.com> wrote:
>
>     Instead, it would be better if you just created a new visitor in
>     Flow whose only responsibility is that of populating the
>     DummyVariable map you already have.
>
>     This new visitor might keep track of the innermost "interesting"
>     enclosing context, and when you create the dummy variable, you
>     note that context down.
>
> Nice idea... see 08c8b1f3c64 
> <https://urldefense.com/v3/__https://github.com/openjdk/jdk/compare/master...archiecobbs:jdk:dummy-variable-detector?expand=1__;!!ACWV5N9M2RV99hQ!OJ_q0-1gIl1E9FlfmaDgk_N-b8wcVW0lavO2dpthLq2Pb_REl0rByaXcH1e7xTsJkQrFA0mPjW97QvvVgHkkv236SiTqyw$> 
> for a stab at it.
>
> I added special logic to include pseudo-tags for STATIC_INITIALIZER 
> and INSTANCE_INITIALIZER.
>
> There are surely some trade-offs regarding what types are 
> "interesting", might be worth tweaking.
>
> Here are the stats from the JDK:
>
>   42 METHODDEF
>   32 IF
>   21 TRY
>   10 FORLOOP
>    3 DOLOOP
>    2 WHILELOOP
>    2 CASE
>    1 FOREACHLOOP
>
> -Archie
>
> -- 
> Archie L. Cobbs
​
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20241017/00fb76f9/attachment-0001.htm>