[foreign-abi] RFR: 8254156: Simplify ABI classification logic
Jorn Vernee
jvernee at openjdk.java.net
Wed Oct 7 12:05:27 UTC 2020
On Wed, 7 Oct 2020 11:39:50 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
> 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).
Marked as reviewed by jvernee (Committer).
src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/PlatformLayouts.java line 100:
> 98: * @return the newly created ValueLayout
> 99: */
> 100: public static ValueLayout ofPointer(ByteOrder order, long bitSize) {
Looks like this can be private? (and in that case I think it's okay to drop the javadoc as well, since none of the
other factories have it either.)
src/jdk.incubator.foreign/share/classes/jdk/internal/foreign/abi/aarch64/TypeClass.java line 46:
> 44: private static TypeClass classifyValueType(ValueLayout type) {
> 45: CLinker.TypeKind kind = (CLinker.TypeKind)type.attribute(CLinker.TypeKind.ATTR_NAME).orElseThrow(
> 46: () -> { throw new IllegalStateException("Unexpected value layout: could not determine ABI class");
> });
Note that orElseThrow takes a `Supplier<Throwable>` (basically), so this can be simplified to return the new exception
instance instead of throwing it.
Also, this code is copied in the other TypeClass classes, so might be good to create a helper method in SharedUtils for
it instead?
-------------
PR: https://git.openjdk.java.net/panama-foreign/pull/371
More information about the panama-dev
mailing list