RFR: 8324651: Compiler Implementation for Derived Record Creation (Preview) [v3]

Jan Lahoda jlahoda at openjdk.org
Thu Mar 28 13:45:59 UTC 2024


On Thu, 28 Mar 2024 11:00:42 GMT, Aggelos Biboudis <abimpoudis at 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


More information about the compiler-dev mailing list