[foreign-memaccess+abi] RFR: Refactor compound layouts such as SequenceLayout and StructLayout [v2]

Maurizio Cimadamore mcimadamore at openjdk.org
Tue Sep 13 14:09:05 UTC 2022


On Tue, 13 Sep 2022 13:19:43 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Per Minborg has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Revert name of GroupLayout implementation
>
> src/java.base/share/classes/java/lang/foreign/CompoundLayout.java line 6:
> 
>> 4: import java.util.stream.Stream;
>> 5: 
>> 6: /**
> 
> I believe there's no reason to keep both CompoundLayout and GroupLayout in the public API, since they are both umbrellas for the same sets of public types (for now at least). I think we can just move these methods down to `GroupLayout` and remove `CompoundLayout`.
> 
> It's true that a `SequenceLayout` can also be viewed as a compound layout with the elements replicated N times, but, I'm having an hard time as to why a client would e.g. treat a SequenceLayout as an Iterable. I think SequenceLayout should mostly stay as is for now. As long as the `elementCount` name matches, we have room at a later point (if we so decide) to bring `SequenceLayout` under `GroupLayout` (or under some other common supertype).

I guess that, more fundamentally, even though the idea of treating sequence layout and group layouts more uniformly sounded cool (at least to me), it also occurred to me that clients tend to deal with sequence layouts in very differently from how they deal with group layouts (the way you recurse on the components is fundamentally different).

For these reasons, I'm not sure having e.g. `Iterable` on `SequenceLayout` will be useful at all. And, as for other changes, such as removing `memberLayouts` and replacing it with a iterator/stream accessor, an element accessor and a length accessor works well in some cases (e.g. enhanced for), but might work less well in others (e.g. if a client needs to pass a list of the member layouts to some other API, in which case an extra `toList` is needed). Overall, none of these things is a big deal, but, at the same time, I guess I'm no longer convinced of the benefits in terms of client code (compared to where the API is now).

-------------

PR: https://git.openjdk.org/panama-foreign/pull/719


More information about the panama-dev mailing list