[foreign-abi] RFR: 8241504: Expose MemoryLayout annotations/attributes in the public API

Maurizio Cimadamore mcimadamore at openjdk.java.net
Tue Mar 24 14:31:41 UTC 2020


On Tue, 24 Mar 2020 12:03:15 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> Hi,
> 
> This PR exposed MemoryLayout attributes in the public API.
> 
> A summary of the changes:
> - Renamed annotation -> attribute everywhere to avoid confusion with Java language annotations
> - Made sure that annotations were being serialized as part of a MemoryLayout's ConstantDesc, as well as the alignment
>   which was previously left out. (The latter also meant changing PaddingLayout::hasNaturalAlignment to always return
>   `true`, so serializing alignment is skipped for padding). Removed special casing for the abiType attribute. This can
>   now instead be handled through the generic attribute mechanism.
> 
> Thanks,
> Jorn

Couple of suggestions, the rest looks really solid.

test/jdk/java/foreign/TestLayoutConstants.java line 111:

> 110:                 { MemoryLayouts.BITS_32_LE.withBitAlignment(8) },
> 111:                 { MemoryLayouts.BITS_32_LE.withAttribute("xyz", "xyz") },
> 112:         };

Maybe change the test to have name != value, so that we also detect bugs where we serialize the name for both.

src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/CallArranger.java line 204:

> 203:
> 204:         var optAbiType = baseType.attribute(SystemABI.NATIVE_TYPE, SystemABI.Type.class);
> 205:         ArgumentClassImpl baseArgClass = optAbiType.map(AArch64ABI::argumentClassFor).orElse(null);

Since all ABI layouts are going to have an annotation of this kind, would it make sense to add a static helper function
in the SystemABI.Type enum to get the ABI type from a layout e.g.

Type fromLayout(Layout layout) throws IllegalArgumentException //if not an ABI layout

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

PR: https://git.openjdk.java.net/panama-foreign/pull/64


More information about the panama-dev mailing list