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

Wed, 23 Oct 2019 23:05:38 -0700

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