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

Maurizio Cimadamore mcimadamore at openjdk.java.net
Wed May 6 14:11:08 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 with a new target base due to a merge or a rebase. The incremental
webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits
since the last revision:

 - Merge branch 'foreign-abi' into jextract+abireorg
 - Update src/jdk.incubator.jextract/share/classes/jdk/incubator/jextract/Type.java
   
   Co-authored-by: Jorn Vernee <JornVernee at users.noreply.github.com>
 - Remove jextract/type attribute (as it was only really used in tests).
 - Fix merge issue
 - Remove trailing whitespaces
 - All jextract tests pass
 - Fixed jextract sources
   TODO: jextract tests needs fixing
 - Removed unsigned ABI constants
 - First refactoring of constants - all ABI tests pass
 - JDK-8244270: reorganize ABI-dependent layout constants (second attempt)
   
   Reviewed-by: sundar

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

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

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

  Stats: 317 lines in 8 files changed: 259 ins; 50 del; 8 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