[lworld] RFR: lworld aaload/aastore

[John Rose]

On Mar 21, 2018, at 6:28 AM, David Simms <david.simms at oracle.com> wrote:
> 
> Updated version of aaload/aastore and copy_array for the interpreter...
> 
> http://cr.openjdk.java.net/~dsimms/valhalla/lworld_value_aaloadstore/webrev3/
> 
> Assumes the current thinking that all value type arrays are flat, but are covariant with Object array and interface arrays:
> 
> * aaload/aastore: works with both objArrayOop and valueArrayOop (flat)
>     o currently assumes always flat semantics for value type element
>       class even for objArrayOop (if value deemed unsuitable as flat),
>       i.e. no null-ability
>     o objArrayOop with values works as vanilla array, i.e. has
>       null-ability, for non-value type classes
> * aaload JVMS changes:
>     o If source array element type is value class (flat array
>       semantics), the result is never null, but equivalent to the
>       "defaultvalue" bytecode.
> * aastore JVMS changes:
>     o May NPE if the target array element type is value class (flat
>       array semantics)
>     o May OOM if the source value is "buffered" and target array is
>       element type is not value class (ref array semantics)
> * covariance
>     o V[] <: I[]: Object[] <: Object where V = value type class, and I
>       = any one of V's interface super types.
>         + E.g. "__ByValue Point implements Comparable", therefore:
>           Point[] <: Comparable[]
>         + instanceof/checkcast, functions as described above
>         + System.arraycopy() (interpreter implementation) now accepts
>           as described above
>             # May NPE when attempting to store null into an array with
>               element type of value class (flat array semantics)

This is all good; I think we are converging on the correct semantics
for these things.

> There are number of tests added to "ValueTypeArray.java", including some sanity test of some "java.util" array related classes. Collections are a larger topic which the test does not try to address fully. But the sanity tests do raise a few interesting observations when fitting value arrays into current JDK collections:
> 
> * ArrayList and ArrayDeque specifically use "Object[] elementData",
>   and accepts no subtype (see "ArrayList(Collection<? extends E> c)",
>   will even copy the source array if not Object[])
>     o So given System.arraycopy from value array to object array now
>       works, as does aaload/store of a value to and from Object array,
>       it all works (in so far as the sanity check tests). Even
>       ArrayList.remove(), since the backing store is always a
>       reference array.
> * Arrays.toList() produces its own ArrayList with a backing array of
>   the sharp type, so potentially a value type array without
>   null-ability characteristics
>     o ...this is fine, doesn't implement "remove()".

This is all logical, and I think the user model for generics + value arrays
is tractable, so far.

> Given what I said about ArrayList backing being Object[] ("Collection.toArray()" must return component type Object), and the API allowing nulls, our current javac behavior is a little weird. Check this code out:
> 
>     ArrayList<MyInt> aList = new ArrayList<MyInt>(Arrays.asList(copyMyInts));
> 
>     aList.add((MyInt)getNull()); // <<< Some issues here, should this be legal ? With "ArrayList<>", yes but maybe not with "ArrayList<Value-Type>"

In recompiled code, casts to value types should always reject nulls.
There's no upside to allowing null to mix with value types in new code,
unless the user is working with supertypes of value types (Object, interfaces).

> Currently javac is inserting "java/util/Objects.requireNonNull". At first I wanted to argue that check-cast should handle null with value class automatically. But wait, generic erasure means I see "ArrayList.add(LObject;)" in the VM, so null is perfectly fine...but the intention is "ArrayList<MyInt>.add(LMyInt;)...so my bad ?

Yes, the fact that aList is physically able to store nulls (of reference types)
is an implementation detail.  The 'add' call to ArrayList<MyInt> must not be
reached with any value that is *not* a MyInt, including null.  Handing in a
null to that generic ArrayList would be as invalid as handing in a String.
The javac-injected casts guard against this, as do the static type rules
enforced by javac.  If MyInt is a value, the casts, explicit and implicit,
must reject null.

Here's another oddity:  According to the API spec, List.toArray(a) will try
to store a single null into the array a if it is bigger than the list.  This will
fail if the array is a value type array.  And it should fail, since null, although
it is a good sentinel value for reference types, it is invalid for value types.
This is one of the places where the collection types use null as a special
sentinel value; Map.get is another.

Good work!
— John