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

[mandy chung]

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