On Thu, 28 Mar 2024 14:08:44 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.
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Fixing tests.

src/java.compiler/share/classes/javax/lang/model/element/ElementKind.java line 
133:

> 131:      */
> 132:     
> @PreviewFeature(feature=PreviewFeature.Feature.DERIVED_RECORD_CREATION, 
> reflective=true)
> 133:     COMPONENT_LOCAL_VARIABLE;

I wonder if we can't just use: LOCAL_VARIABLE

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java line 2235:

> 2233:             }
> 2234:             sym.adr = nextadr;
> 2235:             vars[nextadr] = sym;

could we use a record here? these two arrays seem to be in sync

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java line 
1364:

> 1362: 
> 1363:     @Override
> 1364:     public void visitDerivedInstance(JCDerivedInstance tree) {

I was expecting this code to belong to Lower, not saying that it is wrong here 
though but probably more naturally in Lower I think

src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java line 
1574:

> 1572:             @Override
> 1573:             public void visitDerivedInstance(JCDerivedInstance tree) {
> 1574:                 hasPatterns |= true;

it could be thought as a pattern but it is not a pattern nor it is internally 
converted to a pattern, unless I'm missing something

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1550678834
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1552065329
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1552076515
PR Review Comment: https://git.openjdk.org/jdk/pull/18509#discussion_r1552088772

Reply via email to