On Fri, 11 Dec 2020 17:58:33 GMT, Mandy Chung <mch...@openjdk.org> wrote:
> This is a follow-up on JDK-8255342 that removes non-specified JVM checks on > classes with RecordComponents attributes. > > See the discussion at > https://mail.openjdk.java.net/pipermail/amber-spec-experts/2020-December/002670.html > > That fixes trusting final fields of records to align with the JLS definition > of a record class. `InstanceKlass::is_record` is fixed to check a record > class must be final and a direct subclass of `java.lang.Record` with the > presence of the `Record` attribute. There is no change to JVM class file > validation. I also propose clearly define: > - `JVM_IsRecord` returns true if the given class is a record i.e. final and > direct subclass of `java.lang.Record` with `Record` attribute > - `JVM_GetRecordComponents `returns an `RecordComponents` array if `Record` > attribute is present; otherwise, returns NULL. This does not check if it's a > record class or not. So it may return non-null on a non-record class if it > has `Record` attribute. So `JVM_GetRecordComponents` returns the content of > the classfile. > > tier1-tier3 and jck-runtime:vm and jck-runtime:lang tests all passed. Hi Mandy, The changes look good. Thanks for running the tests and changing the attribute name in the comments. Could you also change the two comments that you added to jvm.c with these comments: Comment for JVM_IsRecord(): // A class is a record if and only if it is final and a direct subclass of // java.lang.Record and has a Record attribute; otherwise, it is not a record. Comment for JVM_GetRecordComponents(): // Returns an array containing the components of the Record attribute, // or NULL if the attribute is not present. // // Note that this function returns the components of the Record attribute // even if the class is not a Record. Thanks, Harold ------------- Marked as reviewed by hseigel (Reviewer). PR: https://git.openjdk.java.net/jdk16/pull/14
