RFR: JEP 359: Records (Preview) (full code)

[Vicente Romero]

Hi David,

On 12/2/19 7:36 PM, David Holmes wrote:
> Hi Vicente,
>
> I have also re-examined all the hotspot related code and everything 
> looks fine - and very neat (kudos to you and Harold!).

thanks, yep Harold has made most of the work here, I just wrote the 
initial prototype a while ago,

>
> A couple of nits:
>
> src/hotspot/share/classfile/classFileParser.cpp
> src/hotspot/share/prims/jvmtiClassFileReconstituter.cpp
>
> Typo:
>
> + //    u2 attributs_count;
>
> ---
>
> src/hotspot/share/classfile/javaClasses.cpp
>
> You are using CHECK_0 in RecordComponent::create but that method 
> returns oop. That seems wrong to me, but I see many other oop 
> returning methods also use CHECK_0 so I'll take that up separately. 
> (It's only a cosmetic issue.)
>
> ---
>
> test/hotspot/jtreg/runtime/records/abstractRecord.jcod
>
> 27 // This test is an Record marked as abstract.
>
> s/an/a/
>
> ---
>
> Thanks,
> David

Thanks for the review,
Vicente

> -----
>
>
> On 28/11/2019 2:37 pm, Vicente Romero wrote:
>> Hi,
>>
>> Please review the code for the records feature at [1]. This webrev 
>> includes all: APIs, runtime, compiler, serialization, javadoc, and 
>> more! Must of the code has been reviewed but there have been some 
>> changes since reviewers saw it. Also this is the first time an 
>> integral webrev is sent out for review. Last changes on top of my 
>> mind since last review iterations:
>>
>> On the compiler implementation:
>> - it has been adapted to the last version of the language spec [2], 
>> as a reference the JVM spec is at [3]. This implied some changes in 
>> determining if a user defined constructor is the canonical or not. 
>> Now if a constructor is override-equivalent to a signature derived 
>> from the record components, then it is considered the canonical 
>> constructor. And any canonical constructor should satisfy a set of 
>> restrictions, see section 8.10.4 Record Constructor Declarations of 
>> the specification.
>> - It was also added a check to make sure that accessors are not generic.
>> - And that the canonical constructor, if user defined, is not 
>> explicitly invoking any other constructor.
>> - The list of forbidden record component names has also been updated.
>> - new error messages have been added
>>
>> APIs:
>> - there have been some API editing in java.lang.Record, 
>> java.lang.runtime.ObjectMethods and 
>> java.lang.reflect.RecordComponent, java.io.ObjectInputStream, 
>> javax.lang.model (some visitors were added)
>>
>> On the JVM implementation:
>> - some logging capabilities have been added to classFileParser.cpp to 
>> provide the reason for which the Record attribute has been ignored
>>
>> Reflection:
>> - there are several new changes to the implementation of 
>> java.lang.reflect.RecordComponent apart from the spec changes 
>> mentioned before.
>>
>> bug fixes in
>> - compiler
>> - serialization,
>> - JVM, etc
>>
>> As a reference the last iteration of the previous reviews can be 
>> found at [4] under folders: compiler, hotspot_runtime, javadoc, 
>> reflection and serialization,
>>
>> TIA,
>> Vicente
>>
>> [1] 
>> http://cr.openjdk.java.net/~vromero/records.review/all_code/webrev.00/
>> [2] 
>> http://cr.openjdk.java.net/~gbierman/jep359/jep359-20191125/specs/records-jls.html 
>>
>> [3] 
>> http://cr.openjdk.java.net/~gbierman/jep359/jep359-20191125/specs/records-jvms.html 
>>
>> [4] http://cr.openjdk.java.net/~vromero/records.review/
>>