On Tue, 5 Nov 2024 14:24:20 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>>> What happens here if an element is already aligned, and `padding.byteSize()
>>> == element.byteAlignment()`? e.g.
>>> `MemoryLayout.structLayout(MemoryLayout.paddingLayout(4),
>>> ValueLayout.JAVA_INT)`?
>>
>> This is an error that existed before this PR. On the main branch:
>>
>>
>> @Test
>> public void alignedByInitialPadding() {
>> Linker linker = Linker.nativeLinker();
>> var struct = MemoryLayout.structLayout(
>> MemoryLayout.paddingLayout(Integer.BYTES),
>> JAVA_INT);
>> var fd = FunctionDescriptor.of(struct, struct, struct);
>> linker.downcallHandle(fd);
>> }
>>
>>
>> would produce:
>>
>>
>> test TestLinker.alignedByInitialPadding(): failure
>> java.lang.IllegalArgumentException: Member layout 'i4', of '[x4i4]' found at
>> unexpected offset: 4 != 0
>> at
>> java.base/jdk.internal.foreign.abi.AbstractLinker.checkMemberOffset(AbstractLinker.java:235)
>> at
>> java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayoutRecursive(AbstractLinker.java:195)
>> at
>> java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayout(AbstractLinker.java:176)
>> at java.base/java.util.Optional.ifPresent(Optional.java:178)
>> at
>> java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayouts(AbstractLinker.java:167)
>> at
>> java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle0(AbstractLinker.java:98)
>> at
>> java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle(AbstractLinker.java:92)
>> at TestLinker.alignedByInitialPadding(TestLinker.java:160)
>> at
>> java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
>
> I've raised a separate issue for this:
> https://bugs.openjdk.org/browse/JDK-8343620
Looks like you removed the check here:
https://github.com/openjdk/jdk/pull/21041/commits/0c134821d6ad7d3c7668d9cc88f7d329b5648827
I think that is good. We already check whether struct fields are aligned when
creating a struct layout.
The existing `checkMemberOffset` does the right thing for this case. If tries
to manually compute the expected offset of the field (using minimal amount of
padding), and then checks if the field actually appears at that offset.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21041#discussion_r1829682340
