8323746: Add PathElement hashCode and equals

Reviewed-by: mcimadamore

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.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 @@ sealed interface PathElement permits LayoutPath.PathElementImpl {
          * @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 @@ static PathElement groupElement(String name) {
          * @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 @@ static PathElement groupElement(long index) {
          * @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 @@ static PathElement sequenceElement(long index) {
          * @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,19 +927,15 @@ static PathElement sequenceElement(long start, long step) {
          * {@code 0 <= I < C}.
          */
         static PathElement sequenceElement() {
-            return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_ELEMENT,
-                                                  LayoutPath::sequenceElement,
-                                                  "sequenceElement()");
+            return LayoutPath.SequenceElement.instance();
         }
 
         /**
          * {@return a path element that dereferences an address layout as its
          * {@linkplain AddressLayout#targetLayout() target layout} (where set)}
          */
         static PathElement dereferenceElement() {
-            return new LayoutPath.PathElementImpl(PathKind.DEREF_ELEMENT,
-                                                  LayoutPath::derefElement,
-                                                  "dereferenceElement()");
+            return LayoutPath.DereferenceElement.instance();
         }
     }
 

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 @@ private String breadcrumbs() {
                 .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<LayoutPath> {
+    public record GroupElementByName(String name)
+            implements MemoryLayout.PathElement, UnaryOperator<LayoutPath> {
 
-        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");
+        // Assert invariants
+        public GroupElementByName {
+            Objects.requireNonNull(name);
+        }
+
+        @Override
+        public LayoutPath apply(LayoutPath layoutPath) {
+            return layoutPath.groupElement(name);
+        }
 
-            final String description;
+        @Override
+        public String toString() {
+            return "groupElement(\"" + name + "\")";
+        }
+    }
 
-            PathKind(String description) {
-                this.description = description;
+    public record GroupElementByIndex(long index)
+            implements MemoryLayout.PathElement, UnaryOperator<LayoutPath> {
+
+        // Assert invariants
+        public GroupElementByIndex {
+            if (index < 0) {
+                throw new IllegalArgumentException("Index < 0");
             }
+        }
+
+        @Override
+        public LayoutPath apply(LayoutPath layoutPath) {
+            return layoutPath.groupElement(index);
+        }
 
-            public String description() {
-                return description;
+        @Override
+        public String toString() {
+            return "groupElement(" + index + ")";
+        }
+
+    }
+
+    public record SequenceElementByIndex(long index)
+            implements MemoryLayout.PathElement, UnaryOperator<LayoutPath> {
+
+        // Assert invariants
+        public SequenceElementByIndex {
+            if (index < 0) {
+                throw new IllegalArgumentException("Index < 0");
             }
         }
 
-        final PathKind kind;
-        final UnaryOperator<LayoutPath> pathOp;
-        final String stringRepresentation;
+        @Override
+        public LayoutPath apply(LayoutPath layoutPath) {
+            return layoutPath.sequenceElement(index);
+        }
 
-        public PathElementImpl(PathKind kind,
-                               UnaryOperator<LayoutPath> pathOp,
-                               String stringRepresentation) {
-            this.kind = kind;
-            this.pathOp = pathOp;
-            this.stringRepresentation = stringRepresentation;
+        @Override
+        public String toString() {
+            return "sequenceElement(" + index + ")";
         }
 
+    }
+
+    public record SequenceElementByRange(long start, long step)
+            implements MemoryLayout.PathElement, UnaryOperator<LayoutPath> {
+
+        // 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<LayoutPath> {
+
+        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<LayoutPath> {
+
+        private static final DereferenceElement INSTANCE = new DereferenceElement();
+
         @Override
         public LayoutPath apply(LayoutPath layoutPath) {
-            return pathOp.apply(layoutPath);
+            return layoutPath.derefElement();
         }
 
-        public PathKind kind() {
-            return kind;
+        // 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 stringRepresentation;
+            return "dereferenceElement()";
         }
+
+        public static MemoryLayout.PathElement instance() {
+            return INSTANCE;
+        }
+
     }
+
 }

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.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<L extends AbstractLayout<L> & MemoryLayout>
         permits AbstractGroupLayout, PaddingLayoutImpl, SequenceLayoutImpl, ValueLayouts.AbstractValueLayout {
@@ -174,12 +173,14 @@ class Holder {
 
     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 VarHandle arrayElementVarHandle(PathElement... elements) {
 
     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> Z computePathOp(LayoutPath path, Function<LayoutPath, Z> finalizer,
-                                       Set<PathKind> badKinds, PathElement... elements) {
+                                       Set<Class<?>> 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<LayoutPath> pathOp = (UnaryOperator<LayoutPath>) e;
+            path = pathOp.apply(path);
         }
         return finalizer.apply(path);
     }

test/jdk/java/foreign/TestLayoutPaths.java

@@ -311,6 +311,13 @@ public void testOffsetHandle(MemoryLayout layout, PathElement[] pathElements, lo
         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);