On Fri, 5 Apr 2024 13:09:56 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> 
wrote:

>> Jan Lahoda has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   JavaCompiler cleanup
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 2117:
> 
>> 2115:          *  position.
>> 2116:          */
>> 2117:         protected VarAndDeclarationTree[] vars;
> 
> So... normally, a `JCVariableDecl` would have a symbol attached, which is how 
> the old code used to work. Is it correct that this change is needed because 
> the `componentLocalVariables` list is a list of symbols and not of 
> JCVariableDecl? Would it be too complex to create a declaration in Attr and 
> save those in the derived creation tree?

Generating the `JCVariableDecl`s in Attr in 
https://github.com/openjdk/jdk/pull/18509/commits/146513580f96abd5feb7886bd1191805cc93403b.
Thanks for the comment!

> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 4443:
> 
>> 4441:     }
>> 4442: 
>> 4443:     @Override
> 
> As usual, I suggest to add some brief comment with the shape of the generated 
> code.

Done here:
https://github.com/openjdk/jdk/pull/18509/commits/146513580f96abd5feb7886bd1191805cc93403b#diff-bc0df6dce7f74078bfca1e90bec75d7bb3d8b338933be725da78f0a8a7a0c78dR4445

Please let me know if that needs some tweaks.

Thanks for the comment!

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1554103472
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1554104437

Reply via email to