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

[Jorn Vernee]

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>.

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