[foreign-jextract] [Rev 02] RFR: JDK-8244270: reorganize ABI-dependent layout constants (second attempt)

[Jorn Vernee]

On Wed, 6 May 2020 10:45:34 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This is another attempt to reorganize ABI-dependent layout constants. The constant reorg is identical to the one
>> proposed in:
>>  https://git.openjdk.java.net/panama-foreign/pull/97
>> 
>> But this time I've also integrated the changes into the jextract branch (as illustrated in this PR).
>> 
>> The main realization which led to this work was that we don't need the ABI type enum: we already have the jextract
>> primitive type enum. So, we could just define constants for the common ABI types which are useful for ABI
>> classification in SystemABI - and then we could start from here in jextract, and augment these layouts by attaching an
>> extra attribute with jextract's Primitve/Kind enum.  I've simplified the API a bit by moving the layout inside the enum
>> itself (so that the layout can be accessed directly from the enum constant) - and also simplified the primitive type
>> factory.  Everything worked out as expected - note how much duplicated layout creation we managed to get rid of.
>> One problem I had was that the loader we use inside jextract test was not delegating correctly (was using `null` as
>> parent loader) so I had to fix that.
>> I'm not 100% sure that this jextract/type extra annotation makes sense - at the end of the day it is now only used
>> inside tests (as it makes layout comparison easier - since layouts might have additional attributes such as name and
>> such).  So I'm open to suggestion on whether we want to keep it or not - I believe that tests which look for specific
>> layouts should instead probably check for specific types instead. Anyway, we can also start from this cleanup, and take
>> care of these issues later; the important thing is that, with this rewrite we are finally able to cleanup the ABI
>> constants the way we wanted.
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove jextract/type attribute (as it was only really used in tests).

Marked as reviewed by jvernee (Committer).

src/jdk.incubator.jextract/share/classes/jdk/incubator/jextract/Type.java line 175:

> 174:                         Optional.empty() :
> 175:                         Optional.of(layout);
> 176:             }

Suggestion:

                return Optional.ofNullable(layout);

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

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