diff --git a/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java b/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java index 7cacfd39f62..acd3546f335 100644 --- a/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java +++ b/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -32,7 +32,6 @@ import java.util.Optional; import java.util.stream.Stream; import jdk.internal.foreign.LayoutPath; -import jdk.internal.foreign.LayoutPath.PathElementImpl.PathKind; import jdk.internal.foreign.Utils; import jdk.internal.foreign.layout.MemoryLayoutUtil; import jdk.internal.foreign.layout.PaddingLayoutImpl; @@ -847,7 +846,13 @@ public sealed interface MemoryLayout * * @since 22 */ - sealed interface PathElement permits LayoutPath.PathElementImpl { + sealed interface PathElement + permits LayoutPath.DereferenceElement, + LayoutPath.GroupElementByIndex, + LayoutPath.GroupElementByName, + LayoutPath.SequenceElement, + LayoutPath.SequenceElementByIndex, + LayoutPath.SequenceElementByRange { /** * {@return a path element which selects a member layout with the given name in a @@ -862,10 +867,7 @@ public sealed interface MemoryLayout * @param name the name of the member layout to be selected */ static PathElement groupElement(String name) { - Objects.requireNonNull(name); - return new LayoutPath.PathElementImpl(PathKind.GROUP_ELEMENT, - path -> path.groupElement(name), - "groupElement(\"" + name + "\")"); + return new LayoutPath.GroupElementByName(name); } /** @@ -876,12 +878,7 @@ public sealed interface MemoryLayout * @throws IllegalArgumentException if {@code index < 0} */ static PathElement groupElement(long index) { - if (index < 0) { - throw new IllegalArgumentException("Index < 0"); - } - return new LayoutPath.PathElementImpl(PathKind.GROUP_ELEMENT, - path -> path.groupElement(index), - "groupElement(" + index + ")"); + return new LayoutPath.GroupElementByIndex(index); } /** @@ -892,12 +889,7 @@ public sealed interface MemoryLayout * @throws IllegalArgumentException if {@code index < 0} */ static PathElement sequenceElement(long index) { - if (index < 0) { - throw new IllegalArgumentException("Index must be positive: " + index); - } - return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_ELEMENT_INDEX, - path -> path.sequenceElement(index), - "sequenceElement(" + index + ")"); + return new LayoutPath.SequenceElementByIndex(index); } /** @@ -923,15 +915,7 @@ public sealed interface MemoryLayout * @throws IllegalArgumentException if {@code start < 0}, or {@code step == 0} */ static PathElement sequenceElement(long start, long step) { - if (start < 0) { - throw new IllegalArgumentException("Start index must be positive: " + start); - } - if (step == 0) { - throw new IllegalArgumentException("Step must be != 0: " + step); - } - return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_RANGE, - path -> path.sequenceElement(start, step), - "sequenceElement(" + start + ", " + step + ")"); + return new LayoutPath.SequenceElementByRange(start, step); } /** @@ -943,9 +927,7 @@ public sealed interface MemoryLayout * {@code 0 <= I < C}. */ static PathElement sequenceElement() { - return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_ELEMENT, - LayoutPath::sequenceElement, - "sequenceElement()"); + return LayoutPath.SequenceElement.instance(); } /** @@ -953,9 +935,7 @@ public sealed interface MemoryLayout * {@linkplain AddressLayout#targetLayout() target layout} (where set)} */ static PathElement dereferenceElement() { - return new LayoutPath.PathElementImpl(PathKind.DEREF_ELEMENT, - LayoutPath::derefElement, - "dereferenceElement()"); + return LayoutPath.DereferenceElement.instance(); } } diff --git a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java index d389519a514..5eee17573ee 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java +++ b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -363,54 +363,141 @@ public class LayoutPath { .collect(joining(", selected from: ")); } - /** - * This class provides an immutable implementation for the {@code PathElement} interface. A path element implementation - * is simply a pointer to one of the selector methods provided by the {@code LayoutPath} class. - */ - public static final class PathElementImpl implements MemoryLayout.PathElement, UnaryOperator { + public record GroupElementByName(String name) + implements MemoryLayout.PathElement, UnaryOperator { - public enum PathKind { - SEQUENCE_ELEMENT("unbound sequence element"), - SEQUENCE_ELEMENT_INDEX("bound sequence element"), - SEQUENCE_RANGE("sequence range"), - GROUP_ELEMENT("group element"), - DEREF_ELEMENT("dereference element"); - - final String description; - - PathKind(String description) { - this.description = description; - } - - public String description() { - return description; - } - } - - final PathKind kind; - final UnaryOperator pathOp; - final String stringRepresentation; - - public PathElementImpl(PathKind kind, - UnaryOperator pathOp, - String stringRepresentation) { - this.kind = kind; - this.pathOp = pathOp; - this.stringRepresentation = stringRepresentation; + // Assert invariants + public GroupElementByName { + Objects.requireNonNull(name); } @Override public LayoutPath apply(LayoutPath layoutPath) { - return pathOp.apply(layoutPath); - } - - public PathKind kind() { - return kind; + return layoutPath.groupElement(name); } @Override public String toString() { - return stringRepresentation; + return "groupElement(\"" + name + "\")"; } } + + public record GroupElementByIndex(long index) + implements MemoryLayout.PathElement, UnaryOperator { + + // Assert invariants + public GroupElementByIndex { + if (index < 0) { + throw new IllegalArgumentException("Index < 0"); + } + } + + @Override + public LayoutPath apply(LayoutPath layoutPath) { + return layoutPath.groupElement(index); + } + + @Override + public String toString() { + return "groupElement(" + index + ")"; + } + + } + + public record SequenceElementByIndex(long index) + implements MemoryLayout.PathElement, UnaryOperator { + + // Assert invariants + public SequenceElementByIndex { + if (index < 0) { + throw new IllegalArgumentException("Index < 0"); + } + } + + @Override + public LayoutPath apply(LayoutPath layoutPath) { + return layoutPath.sequenceElement(index); + } + + @Override + public String toString() { + return "sequenceElement(" + index + ")"; + } + + } + + public record SequenceElementByRange(long start, long step) + implements MemoryLayout.PathElement, UnaryOperator { + + // Assert invariants + public SequenceElementByRange { + if (start < 0) { + throw new IllegalArgumentException("Start index must be positive: " + start); + } + if (step == 0) { + throw new IllegalArgumentException("Step must be != 0: " + step); + } + } + + @Override + public LayoutPath apply(LayoutPath layoutPath) { + return layoutPath.sequenceElement(start, step); + } + + @Override + public String toString() { + return "sequenceElement(" + start + ", " + step + ")"; + } + + } + + public record SequenceElement() + implements MemoryLayout.PathElement, UnaryOperator { + + private static final SequenceElement INSTANCE = new SequenceElement(); + + @Override + public LayoutPath apply(LayoutPath layoutPath) { + return layoutPath.sequenceElement(); + } + + @Override + public String toString() { + return "sequenceElement()"; + } + + public static MemoryLayout.PathElement instance() { + return INSTANCE; + } + + } + + public record DereferenceElement() + implements MemoryLayout.PathElement, UnaryOperator { + + private static final DereferenceElement INSTANCE = new DereferenceElement(); + + @Override + public LayoutPath apply(LayoutPath layoutPath) { + return layoutPath.derefElement(); + } + + // Overriding here will ensure DereferenceElement will have a hash code + // that is different from the hash code of SequenceElement. + @Override + public int hashCode() { + return 31; + } + + @Override + public String toString() { + return "dereferenceElement()"; + } + + public static MemoryLayout.PathElement instance() { + return INSTANCE; + } + + } + } diff --git a/src/java.base/share/classes/jdk/internal/foreign/layout/AbstractLayout.java b/src/java.base/share/classes/jdk/internal/foreign/layout/AbstractLayout.java index 3c0cf3902bb..f72009cf690 100644 --- a/src/java.base/share/classes/jdk/internal/foreign/layout/AbstractLayout.java +++ b/src/java.base/share/classes/jdk/internal/foreign/layout/AbstractLayout.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -26,7 +26,6 @@ package jdk.internal.foreign.layout; import jdk.internal.foreign.LayoutPath; -import jdk.internal.foreign.LayoutPath.PathElementImpl.PathKind; import jdk.internal.foreign.Utils; import java.lang.foreign.GroupLayout; @@ -40,11 +39,11 @@ import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.MethodType; import java.lang.invoke.VarHandle; -import java.util.EnumSet; import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.UnaryOperator; public abstract sealed class AbstractLayout & MemoryLayout> permits AbstractGroupLayout, PaddingLayoutImpl, SequenceLayoutImpl, ValueLayouts.AbstractValueLayout { @@ -174,12 +173,14 @@ public abstract sealed class AbstractLayout & Memory public long byteOffset(PathElement... elements) { return computePathOp(LayoutPath.rootPath((MemoryLayout) this), LayoutPath::offset, - EnumSet.of(PathKind.SEQUENCE_ELEMENT, PathKind.SEQUENCE_RANGE, PathKind.DEREF_ELEMENT), elements); + Set.of(LayoutPath.SequenceElement.class, LayoutPath.SequenceElementByRange.class, LayoutPath.DereferenceElement.class), + elements); } public MethodHandle byteOffsetHandle(PathElement... elements) { return computePathOp(LayoutPath.rootPath((MemoryLayout) this), LayoutPath::offsetHandle, - EnumSet.of(PathKind.DEREF_ELEMENT), elements); + Set.of(LayoutPath.DereferenceElement.class), + elements); } public VarHandle varHandle(PathElement... elements) { @@ -197,23 +198,27 @@ public abstract sealed class AbstractLayout & Memory public MethodHandle sliceHandle(PathElement... elements) { return computePathOp(LayoutPath.rootPath((MemoryLayout) this), LayoutPath::sliceHandle, - Set.of(PathKind.DEREF_ELEMENT), elements); + Set.of(LayoutPath.DereferenceElement.class), + elements); } public MemoryLayout select(PathElement... elements) { return computePathOp(LayoutPath.rootPath((MemoryLayout) this), LayoutPath::layout, - EnumSet.of(PathKind.SEQUENCE_ELEMENT_INDEX, PathKind.SEQUENCE_RANGE, PathKind.DEREF_ELEMENT), elements); + Set.of(LayoutPath.SequenceElementByIndex.class, LayoutPath.SequenceElementByRange.class, LayoutPath.DereferenceElement.class), + elements); } private static Z computePathOp(LayoutPath path, Function finalizer, - Set badKinds, PathElement... elements) { + Set> badTypes, PathElement... elements) { Objects.requireNonNull(elements); for (PathElement e : elements) { - LayoutPath.PathElementImpl pathElem = (LayoutPath.PathElementImpl)Objects.requireNonNull(e); - if (badKinds.contains(pathElem.kind())) { - throw new IllegalArgumentException(String.format("Invalid %s selection in layout path", pathElem.kind().description())); + Objects.requireNonNull(e); + if (badTypes.contains(e.getClass())) { + throw new IllegalArgumentException("Invalid selection in layout path: " + e); } - path = pathElem.apply(path); + @SuppressWarnings("unchecked") + UnaryOperator pathOp = (UnaryOperator) e; + path = pathOp.apply(path); } return finalizer.apply(path); } diff --git a/test/jdk/java/foreign/TestLayoutPaths.java b/test/jdk/java/foreign/TestLayoutPaths.java index c5eee36cd9c..e6bfe8450a0 100644 --- a/test/jdk/java/foreign/TestLayoutPaths.java +++ b/test/jdk/java/foreign/TestLayoutPaths.java @@ -311,6 +311,13 @@ public class TestLayoutPaths { assertEquals(actualByteOffset, expectedByteOffset); } + @Test + public void testHashCodeCollision() { + PathElement sequenceElement = PathElement.sequenceElement(); + PathElement dereferenceElement = PathElement.dereferenceElement(); + assertNotEquals(sequenceElement.hashCode(), dereferenceElement.hashCode()); + } + @Test public void testGroupElementIndexToString() { PathElement e = PathElement.groupElement(2);