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