On Wed, 27 Mar 2024 10:24:51 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:

> This is a patch for javac, that adds the Derived Record Creation expressions. 
> The current draft specification for the feature is:
> https://cr.openjdk.org/~gbierman/jep468/jep468-20240326/specs/derived-record-creation-jls.html
> 
> The current CSR is here:
> https://bugs.openjdk.org/browse/JDK-8328637
> 
> The patch is mostly straightforward, with two notable changes:
>  - there is a new `ElementKind.COMPONENT_LOCAL_VARIABLE`, as the 
> specification introduces this term, and it seems consistent with 
> `ElementKind.BINDING_VARIABLE` that was introduced some time ago.
>  - there are a bit broader changes in `Flow`, to facilitate the introduction 
> of variables without an explicit declaration for definite assignment and 
> effectively final computation.

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`.

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?

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?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1542712222
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1542713984
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1542746919

Reply via email to