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

Maurizio Cimadamore mcimadamore at openjdk.java.net
Tue Mar 24 17:10:37 UTC 2020


On Tue, 24 Mar 2020 16:22:07 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
>
> Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Addressed review comments + added basic tests

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/SystemABI.java line 190:

> 189:          * @return the retrieved ABI type
> 190:          * @throws IllegalArgumentException if the given layout does not have an ABI type annotation
> 191:          */

Suggestion:

         * @throws IllegalArgumentException if the given layout does not have an ABI type attribute

src/jdk.incubator.foreign/share/classes/jdk/incubator/foreign/MemoryLayout.java line 303:

> 302:      */
> 303:     default <T extends Constable> Optional<T> attribute(String name, Class<T> type) {
> 304:         return attribute(name).filter(type::isInstance).map(type::cast);

Not sure I like this method. While it works assuming all constable implementation are non-generic, it will stop working
correctly as soon as you have a generic class implementing a constable. Maybe save the sugar for later?

(btw this was there in the previous iteration too but I've missed - guess because it was a default method and I focused
on AbstractLayout)

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

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


More information about the panama-dev mailing list