[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