[lworld] RFR: 8273018: [lworld] Property annotation propagation to <init> lacks in primitive records [v2]

[Srikanth Adayapalam]

On Tue, 7 Sep 2021 23:37:52 GMT, Jesper Steen Møller <jespersm at openjdk.org> wrote:

>> This fix copies parameter names and annotations from constructor into primitive class factory.
>
> Jesper Steen Møller has updated the pull request incrementally with one additional commit since the last revision:
> 
>   - Emit parameter names (described JDK-8273202)
>   - Keep type parameters
>   - Provide useful test for primitive records in "RecordReading"

Changes requested by sadayapalam (Committer).

If you want to close more than issue with the same PR, I think you need to add the other issue here with /issue 

Also please mention what tests have been run. Thanks

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java line 1022:

> 1020:                 || m.isConstructor() && (m.flags_field & RECORD) != 0
> 1021:                 || m.isPrimitiveObjectFactory() && (m.flags_field & RECORD) != 0)) {
> 1022:             if (!m.isLambdaMethod()) // Per JDK-8138729, do not emit parameters table for lambda bodies.

Can you fold the two checks for (m.flags_field & RECORD) != 0 into one here ??

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/TransPrimitiveClass.java line 394:

> 392:         MethodType factoryType = new MethodType(init.type.getParameterTypes(),
> 393:                                                 init.owner.type.asValueType(),
> 394:                                                 init.type.getThrownTypes(),

I am lost and need a bit of help here. What exactly is the change and why is it needed ?

src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/TransPrimitiveClass.java line 406:

> 404:                 init.type.getThrownTypes(),
> 405:                 init.owner.type.tsym);
> 406:         factory.setAttributes(init);

Again, I need some help with this change. I understand the part of parameters being carried over, but the dance around recomputation of erasure field - what exactly is the material change there ?? I can see the RecordReading test failing if I backout the changes here (except for factory.params = init.params) but need some explanation - TIA

test/langtools/tools/javac/records/RecordReading.java line 131:

> 129:     @Test
> 130:     public void testPrimitiveRecordClassFileReading(Path base) throws Exception {
> 131:         Path src = base.resolve("src");

Could you please update the copyright to 2021 and also add a @bug id line to the test spec ? Thanks

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

PR: https://git.openjdk.java.net/valhalla/pull/541