Re: RFR: JEP 359-Records: hotspot runtime and serviceability code

Thu, 24 Oct 2019 06:30:46 -0700 

Thanks Serguei!

I will make the changes that you suggest.

Harold

On 10/24/2019 2:01 AM, serguei.spit...@oracle.com wrote:

Hi Vicente and Harold,

The fix looks good to me.
Nice set of tests!

I have a couple of nits besides what other reviewers already commented.

http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/jdk/java/lang/instrument/RedefineRecordAttr/HostBA/redef/Host.java.html

   29     public Host(int A, long B, char C) {
   30         this.A = A;
   31         this.B = B;
   32     }
The lines 30 and 31 needs to be swapped to follow the other such variants style. 

For instance the version HostBAC:
http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/jdk/java/lang/instrument/RedefineRecordAttr/HostBAC/redef/Host.java.html

has constructor:
   29     public Host(int A, long B, char C) {
   30         this.B = B;
   31         this.A = A;
   32         this.C = C;
   33     }
http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/jdk/java/lang/instrument/RedefineRecordAttr/TestRecordAttr.java.html
   50 The basic test class is call Host and we have variants that have zero or 
more

http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/test/jdk/java/lang/instrument/RedefineRecordAttrGenericSig/TestRecordAttrGenericSig.java.html
   46 The basic test class is call Host and we have variants that have record 
components

It seems there is a typo in the comments above: 'is call' => 'is called'.
Maybe, I did not get it correctly.


Thanks,
Serguei


On 10/18/19 11:44, Vicente Romero wrote:

Hi,
Please review the hotspot runtime and serviceability code for JEP 359 (Records). 

Thanks in advance for the feedback,
Vicente

PS, Thanks to Harold for the development
[1] http://cr.openjdk.java.net/~vromero/records.review/hotspot_runtime/webrev.00/

Reply via email to