[foreign-memaccess] RFR 8223768: Rethink relationship between Sequence vs. Group layouts

[Maurizio Cimadamore]

On 13/05/2019 14:40, Jorn Vernee wrote:
> Hi,
>
> One comment:
>
> - LayoutPathImpl::sequenceElement
>
>   Currently, looking up sequence elements as contents of a Value does 
> not work. e.g.
>
>   Value v = Value.ofUnsignedInt(64).withContents(Sequence.of(8, 
> Value.ofUnsignedInt(8)));
>   VarHandle vh = 
> v.toPath().sequenceElement().dereferenceHandle(byte.class); // Boom! 
> IllegalStateException in LayoutPathImpl::sequenceElement
>
>   If this case is not meant to be supported, I think the type of 
> Value::contents should just be Optional<Group> instead of 
> Optional<Sequence>.

There are two parts to this: I think constructing paths to sub-value 
contents should work. But I also think that dereferencing sub-value 
content should not be supported and best left to clients (as that almost 
always involve some bitmasking and a different set of carriers that is 
hard to be expressed in a single the API).

So, the idea is that deferencing a path that points inside the guts of 
some value should throw some sane exception (I have some code which 
makes this more explicit).

Of course, when we get to a more general version of 
compoundElement(long) which works on both sequences and groups, we 
should make sure that, given your layout, this works:

v.toPath().compoundElement(2);

A separate question is: given we don't intend to support dereference of 
value innards, should we keep the Value::contents API?

Regardless of the decision, I think the top type 'Compound' is an useful 
abstraction.

Maurizio

>
> Cheers,
> Jorn
>
> Maurizio Cimadamore schreef op 2019-05-13 14:14:
>> Hi,
>> as discussed previously, the relationship between sequences and groups
>> is a bit shaky, as there are sequences that cannot be expressed as
>> groups (sequences whose size is unbound).
>>
>> This patch models groups and sequences independently:
>>
>> * they all have a (minimal) common supertype called Compound (which
>> supports only a Stream<Layout> elements() method)
>>
>> * Group implements Iterable, always has a size, and you can access
>> elements by index
>>
>> * Sequence doesn't implement Iterable, has an optional size, and has
>> an accessor to retrieve the element layout (there's only one of them,
>> regardless of the size)
>>
>> I think this is much saner.
>>
>> Webrev:
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8223768/
>>
>> P.S.
>> of course Compound should be renamed to CompoundLayout in a later
>> webrev (as for all other layout names).
>>
>> P.P.S.
>> in a separate patch I'll explore generalizing this method:
>>
>> LayoutPath::groupElement(long index)
>>
>> with this:
>>
>> LayoutPath::compoundElement(long index)
>>
>> Since now we should be able to implement indexed access on both group
>> and sequences.
>>
>> Maurizio