8323601: Improve LayoutPath.PathElement::toString

Reviewed-by: jvernee
openjdk · Jan 31, 2024 · ec56c72 · ec56c72 · openjdk-notifier · Jan 31, 2024
1 parent f7121de
commit ec56c72
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 7 deletions.
diff --git a/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java b/src/java.base/share/classes/java/lang/foreign/MemoryLayout.java
@@ -864,7 +864,8 @@ sealed interface PathElement permits LayoutPath.PathElementImpl {
         static PathElement groupElement(String name) {
             Objects.requireNonNull(name);
             return new LayoutPath.PathElementImpl(PathKind.GROUP_ELEMENT,
-                                                  path -> path.groupElement(name));
+                                                  path -> path.groupElement(name),
+                                                  "groupElement(\"" + name + "\")");
         }
 
         /**
@@ -879,7 +880,8 @@ static PathElement groupElement(long index) {
                 throw new IllegalArgumentException("Index < 0");
             }
             return new LayoutPath.PathElementImpl(PathKind.GROUP_ELEMENT,
-                    path -> path.groupElement(index));
+                                                  path -> path.groupElement(index),
+                                                  "groupElement(" + index + ")");
         }
 
         /**
@@ -894,7 +896,8 @@ static PathElement sequenceElement(long index) {
                 throw new IllegalArgumentException("Index must be positive: " + index);
             }
             return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_ELEMENT_INDEX,
-                                                  path -> path.sequenceElement(index));
+                                                  path -> path.sequenceElement(index),
+                                                  "sequenceElement(" + index + ")");
         }
 
         /**
@@ -927,7 +930,8 @@ static PathElement sequenceElement(long start, long step) {
                 throw new IllegalArgumentException("Step must be != 0: " + step);
             }
             return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_RANGE,
-                                                  path -> path.sequenceElement(start, step));
+                                                  path -> path.sequenceElement(start, step),
+                                                  "sequenceElement(" + start + ", " + step + ")");
         }
 
         /**
@@ -940,7 +944,8 @@ static PathElement sequenceElement(long start, long step) {
          */
         static PathElement sequenceElement() {
             return new LayoutPath.PathElementImpl(PathKind.SEQUENCE_ELEMENT,
-                                                  LayoutPath::sequenceElement);
+                                                  LayoutPath::sequenceElement,
+                                                  "sequenceElement()");
         }
 
         /**
@@ -949,7 +954,8 @@ static PathElement sequenceElement() {
          */
         static PathElement dereferenceElement() {
             return new LayoutPath.PathElementImpl(PathKind.DEREF_ELEMENT,
-                    LayoutPath::derefElement);
+                                                  LayoutPath::derefElement,
+                                                  "dereferenceElement()");
         }
     }
 

diff --git a/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java b/src/java.base/share/classes/jdk/internal/foreign/LayoutPath.java
@@ -389,10 +389,14 @@ public String description() {
 
         final PathKind kind;
         final UnaryOperator<LayoutPath> pathOp;
+        final String stringRepresentation;
 
-        public PathElementImpl(PathKind kind, UnaryOperator<LayoutPath> pathOp) {
+        public PathElementImpl(PathKind kind,
+                               UnaryOperator<LayoutPath> pathOp,
+                               String stringRepresentation) {
             this.kind = kind;
             this.pathOp = pathOp;
+            this.stringRepresentation = stringRepresentation;
         }
 
         @Override
@@ -403,5 +407,10 @@ public LayoutPath apply(LayoutPath layoutPath) {
         public PathKind kind() {
             return kind;
         }
+
+        @Override
+        public String toString() {
+            return stringRepresentation;
+        }
     }
 }
diff --git a/test/jdk/java/foreign/TestLayoutPaths.java b/test/jdk/java/foreign/TestLayoutPaths.java
@@ -311,6 +311,42 @@ public void testOffsetHandle(MemoryLayout layout, PathElement[] pathElements, lo
         assertEquals(actualByteOffset, expectedByteOffset);
     }
 
+    @Test
+    public void testGroupElementIndexToString() {
+        PathElement e = PathElement.groupElement(2);
+        assertEquals(e.toString(), "groupElement(2)");
+    }
+
+    @Test
+    public void testGroupElementNameToString() {
+        PathElement e = PathElement.groupElement("x");
+        assertEquals(e.toString(), "groupElement(\"x\")");
+    }
+
+    @Test
+    public void testSequenceElementToString() {
+        PathElement e = PathElement.sequenceElement();
+        assertEquals(e.toString(), "sequenceElement()");
+    }
+
+    @Test
+    public void testSequenceElementIndexToString() {
+        PathElement e = PathElement.sequenceElement(2);
+        assertEquals(e.toString(), "sequenceElement(2)");
+    }
+
+    @Test
+    public void testSequenceElementRangeToString() {
+        PathElement e = PathElement.sequenceElement(2, 4);
+        assertEquals(e.toString(), "sequenceElement(2, 4)");
+    }
+
+    @Test
+    public void testDerefereceElementToString() {
+        PathElement e = PathElement.dereferenceElement();
+        assertEquals(e.toString(), "dereferenceElement()");
+    }
+
     @DataProvider
     public static Object[][] testLayouts() {
         List<Object[]> testCases = new ArrayList<>();