Review Request JDK-8206121: [lworld] ensure non-nullable on setting value field via reflection and VarHandle (original) (raw)

mandy chung mandy.chung at oracle.com
Fri Jul 6 05:44:36 UTC 2018

Frederic points out that only ACC_FLATTENABLE field whose type is listed in ValueTypes attribute is non-nullable and a field of value type may be nullable in LW1 (e.g. static value field). I have revised the patch to be consistent with JVMS.

Updated webrev:

http://cr.openjdk.java.net/~mchung/valhalla/webrevs/8206121/webrev.01/index.html

MemberName::isFlattenable and Field::isFlattenable only looks at the modifiers/flags set by the JVM. Frederic - I believe the type of the field has been resolved and validated with the field's declaring class's ValueTypes attribute. Is that correct? Therefore the library does not inspect ValueTypes attribute.

VarHandleObjects.ValueFieldReadOnly and ValueFieldReadWrite classes may be better to rename to FlatValueFieldReadOnly and FlatValueFieldReadWrite to be explicit. I can do it another day.

Comments inlined below.

On 7/5/18 11:29 AM, Paul Sandoz wrote:

Hi Mandy,

It looks reasonable as an initial solution but i think the right long term solution should be for Class.cast to fail. Perhaps that is already tracked under a different issue?

As we chatted offline, a non-flattenable field of a value type does not need the null check. For example C.f of type T and T is not value type when compiled and hence C.f is not flattenable. T can be combined later separately as a value type. In that case, C.f is nullable.

Both the VarHandle code and the LF code perform cast checks for refs so we should be able to piggy back off them for values without an explicit null check.

That's a good suggestion. I added ValueAccessor and StaticValueAccessor subclass of the ref accessor class to override checkCast method. No need to change LF code.

s/ensureImmutable/ensureNonNullable/

It's checking immutability that throws IAE (UOE for VarHandle case). I changed the test to the field to non-null that may help avoid the confusion.

s/ensureNullableOrNot/ensureNullable/

+1

? AFAICT you are not checking finality of a value type held in a ref but the the assignment of null.

I'll add that.

Mandy

More information about the valhalla-dev mailing list