[foreign-memaccess+abi] RFR: 8299181: PaddingLayout unable to return byteAlignment value

[Maurizio Cimadamore]

On Wed, 11 Jan 2023 10:25:47 GMT, Per Minborg <pminborg at openjdk.org> wrote:

> This PR proposes bit-aligning `PaddingLayout` to 8 by default as opposed to the current  value of 1.
> 
> More generally, the PR restricts `PaddingLayout` bit-alignment to non-negative values that are an even multiple of 8 (i.e. 0, 8, 16, ...). There are no mechanism to derive `VarHandle` objects for sub-byte access anyhow and trying to obtain a miss-aligned `VarHandle` would throw an exception. 
> 
> If integrated, all `MemoryLayout` types would be byte-aligned.

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 619:

> 617:      */
> 618:     static PaddingLayout paddingLayout(long bitSize) {
> 619:         if (bitSize < 0 || bitSize % 8 != 0) {

If I'm not mistaken, this is the same check that should be applied when we call `withBitAlignment`, right? It might be useful to consolidate (both impl-wise and doc-wise).

src/java.base/share/classes/java/lang/foreign/MemoryLayout.java line 712:

> 710:     static StructLayout structLayout(MemoryLayout... elements) {
> 711:         Objects.requireNonNull(elements);
> 712:         return wrapOverflow(() -> StructLayoutImpl.of(List.of(elements)));

Why was Objects.requireNonNull dropped? I believe that NPE  will be thrown deeper inside the implementation - which is why we had the eager check (to make stack trace more readable).

src/java.base/share/classes/jdk/internal/foreign/layout/AbstractLayout.java line 141:

> 139:     }
> 140: 
> 141:     private static long requirePowerOfTwoAndGreaterOrEqualToEight(long value) {

Should this stay here, or be merged inside MemoryLayoutUtil? It seems the same as `requireBitSizeValid`, except for the zero behavior?

test/jdk/java/foreign/TestLayouts.java line 203:

> 201:     @Test
> 202:     public void testNullMember() {
> 203:         var illegalLayouts = new MemoryLayout[]{JAVA_INT, null, JAVA_INT};

this is covered by TestNulls. That test will try to pass null values to all API methods accepting arrays, or collections, and will do so in an automatic fashion.

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

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