[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