On Thu, 28 Mar 2024 11:00:42 GMT, Aggelos Biboudis <abimpou...@openjdk.org>
wrote:
>> Jan Lahoda has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Renaming visitReconstruction to visitDerivedInstance as suggested.
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 2228:
>
>> 2226: * index into the vars array.
>> 2227: */
>> 2228: void newVar(JCTree pos,VarSymbol sym) {
>
> This is used only twice. One from `void newVar(JCVariableDecl varDecl)` which
> is very intuitive and one with `newVar(null, component);` which I understand.
> But, is there any reason to create a `var` in the future with something else
> than `null` (unrelated to `sym`?). Maybe the comment needs to be updated to
> document what should be the relation (if any) between `pos` and `sym`.
Oops, that's actually an oversight - there should be a pos specified in all
cases. Should be fixed now.
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 3259:
>
>> 3257: startPos = tree.pos().getStartPosition();
>> 3258:
>> 3259: if (vars == null)
>
> Curly braces here?
This is a copy of the similar code for `vardecls`. I was thinking if I could
unify the codepaths, but does not seem to be so simple, so I created a
duplicate. So, it might be better to keep the code the same/similar as it was
before, to minimize disruptions.
> src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java line 3607:
>
>> 3605: public void visitLambda(JCLambda that) {
>> visitTree(that); }
>> 3606: public void visitParens(JCParens that) {
>> visitTree(that); }
>> 3607: public void visitReconstruction(JCDerivedInstance that) {
>> visitTree(that); }
>
> Maybe `visitDerivedInstance` to be in sync with the `JCDerivedInstance` world?
Done. Thanks!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1543004274
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1543006459
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1543006600
