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

Maurizio Cimadamore mcimadamore at openjdk.java.net
Wed May 6 14:44:57 UTC 2020


> 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 two additional commits since the last revision:

 - Update src/jdk.incubator.jextract/share/classes/jdk/incubator/jextract/tool/OutputFactory.java
   
   Co-authored-by: Jorn Vernee <JornVernee at users.noreply.github.com>
 - Update src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/LayoutUtils.java
   
   Co-authored-by: Jorn Vernee <JornVernee at users.noreply.github.com>

This pull request has now been integrated.

Changeset: e5a42f23
Author:    Maurizio Cimadamore <mcimadamore at openjdk.org>
URL:       https://git.openjdk.java.net/panama-foreign/commit/e5a42f23
Stats:     2879 lines in 52 files changed: 863 ins; 347 del; 1669 mod

JDK-8244270: reorganize ABI-dependent layout constants (second attempt)

Reviewed-by: jvernee

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

Changes:
  - all: https://git.openjdk.java.net/panama-foreign/pull/141/files
  - new: https://git.openjdk.java.net/panama-foreign/pull/141/files/bbdfae0c..97a59172

Webrevs:
 - full: https://webrevs.openjdk.java.net/panama-foreign/141/webrev.05
 - incr: https://webrevs.openjdk.java.net/panama-foreign/141/webrev.04-05

  Stats: 2 lines in 2 files changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/panama-foreign/pull/141.diff
  Fetch: git fetch https://git.openjdk.java.net/panama-foreign pull/141/head:pull/141

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


More information about the panama-dev mailing list