RFR: 8340205: Native linker allows MemoryLayout consisting of only PaddingLayout [v3]

[Maurizio Cimadamore]

On Mon, 23 Sep 2024 10:51:10 GMT, Per Minborg <pminborg at openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java line 225:
>> 
>>> 223:     // check elements are not all padding layouts and for trailing padding
>>> 224:     private void checkGroup(GroupLayout gl, long maxUnpaddedOffset) {
>>> 225:         if (gl.memberLayouts().stream().allMatch(e -> e instanceof PaddingLayout)) {
>> 
>> I'm not 100% sure that this check (which I agree with) can be desumed from the current `Linker` javadoc. E.g. if I have:
>> 
>> `groupLayout(paddingLayout(1))`
>> 
>> The Linker says:
>> * there cannot be any intermediate padding more than necessary to align non-padding elements. But there's no non-padding elements here, so <shrug> :-)
>> * there cannot be more padding at the end, more than necessary to make the group size a multiple of its alignment - but again, since there's no non-padding layout, it is not clear what this means here. We have a padding of 1, which makes the group have size 1, which is a multiple of the alignment of the struct. So, ok?
>
> Previously, the check threw an `IllegalArgumentException` when presented with the above (or other) group layout with only padding layouts, but the actual message differed. So, this change only changes the exception message and does not change the current behavior. However, the question remains: Should we update the docs to reflect this behavior?

Note: I know the behavior in this particular case is not affected by the PR. That said, since you already need CSR for the "sequence with padding" behavioral change, I think it could make sense to piggy back on this PR, and clarify the javadoc for the other case as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21041#discussion_r1771216722