openjdk / panama-foreign Public
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8254156: Simplify ABI classification logic #371
8254156: Simplify ABI classification logic #371
Conversation
Remove CValueLayout Simplify ABI classification by just exposing single TypeKind enum
|
src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/PlatformLayouts.java
Outdated
Show resolved
Hide resolved
src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/TypeClass.java
Outdated
Show resolved
Hide resolved
@mcimadamore This change now passes all automated pre-integration checks. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the
|
/integrate |
@mcimadamore Pushed as commit 6663aab. |
/** | ||
* A kind corresponding to the a C pointer type | ||
*/ | ||
POINTER(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pointer or pointer-sized? We'd also use this one for size_t
for instance, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pointer; e.g. values of this layout will be associated with the MemoryAddress.class carrier. So, no, for size_t, you need something different (either LONG or LONGLONG, depending on the platform). We could probably add a SIZE_T in there, since it's frequent enough.
This patch reverts recent changes to introduce a public CValueLayout class; the realization here is that, after all, CValueLayout exposes a public
Type
enum which is used by ABI classification. So, instead of using subclassing (which is messy and tedious with immutable data structures with covariant overrides), let's just double down on layout attributes, get rid of CValueLayout and simply expose the Type enum (now renamed to TypeKind).Progress
Testing
Issue
Reviewers
Download
$ git fetch https://git.openjdk.java.net/panama-foreign pull/371/head:pull/371
$ git checkout pull/371