RFR: 7904079: Fix setting up length parameter for CXUnsavedFiles Struct [v4]

[Jorn Vernee]

On Thu, 18 Sep 2025 12:17:48 GMT, Varada M <varadam at openjdk.org> wrote:

>> CXUnsavedFile struct contains three members, first two are pointers and last one is unsigned long. When jextract makes downcall to clang_reparseTranslationUnit_Impl it sets up the third parameter in the struct incorrectly. It sets INT instead of LONG so on big endian platform the parameter will be set differently. 
>> 
>> JBS Issue : [CODETOOLS-7904079](https://bugs.openjdk.org/browse/CODETOOLS-7904079)
>
> Varada M has updated the pull request incrementally with one additional commit since the last revision:
> 
>   7904079: Fix setting up length parameter for CXUnsavedFiles Struct

Thanks for the updates! Some more changes are needed though. Tests on Windows currently fail with a class cast exception when initializing the `CXUnsavedFile` class:


...
Caused by: java.lang.ClassCastException: class jdk.internal.foreign.layout.ValueLayouts$OfIntImpl cannot be cast to class java.lang.foreign.ValueLayout$OfLong (jdk.internal.foreign.layout.ValueLayouts$OfIntImpl and java.lang.foreign.ValueLayout$OfLong are in module java.base of loader 'bootstrap')
        at org.openjdk.jextract at 23/org.openjdk.jextract.clang.libclang.CXUnsavedFile.<clinit>(CXUnsavedFile.java:156)
        ... 39 more


I think it would be best to modify `CXUnsavedFile` to tweak the layout based on the size of `C_LONG`. Besides the changes I mentioned inline, I have the following changes locally to make this work:


diff --git a/src/main/java/org/openjdk/jextract/clang/libclang/CXUnsavedFile.java b/src/main/java/org/openjdk/jextract/clang/libclang/CXUnsavedFile.java
index 12d25de..a0ebb98 100644
--- a/src/main/java/org/openjdk/jextract/clang/libclang/CXUnsavedFile.java
+++ b/src/main/java/org/openjdk/jextract/clang/libclang/CXUnsavedFile.java
@@ -52,11 +52,20 @@ public class CXUnsavedFile {
         // Should not be called directly
     }
 
-    private static final GroupLayout $LAYOUT = MemoryLayout.structLayout(
-        Index_h.C_POINTER.withName("Filename"),
-        Index_h.C_POINTER.withName("Contents"),
-        Index_h.C_LONG.withName("Length")
-    ).withName("CXUnsavedFile");
+    private static final GroupLayout $LAYOUT = (switch (Index_h.C_LONG) {
+            case OfInt _ -> MemoryLayout.structLayout(
+                    Index_h.C_POINTER.withName("Filename"),
+                    Index_h.C_POINTER.withName("Contents"),
+                    Index_h.C_LONG.withName("Length"),
+                    MemoryLayout.paddingLayout(4)
+            );
+            case OfLong _ -> MemoryLayout.structLayout(
+                    Index_h.C_POINTER.withName("Filename"),
+                    Index_h.C_POINTER.withName("Contents"),
+                    Index_h.C_LONG.withName("Length")
+            );
+            default -> throw new IllegalStateException("Unhandled layout: " + Index_h.C_LONG);
+        }).withName("CXUnsavedFile");
 
     /**
      * The layout of this struct
@@ -153,7 +162,7 @@ public class CXUnsavedFile {
         struct.set(Contents$LAYOUT, Contents$OFFSET, fieldValue);
     }
 
-    private static final OfLong Length$LAYOUT = (OfLong)$LAYOUT.select(groupElement("Length"));
+    private static final ValueLayout Length$LAYOUT = (ValueLayout) $LAYOUT.select(groupElement("Length"));
 
     /**
      * Layout for field:
@@ -161,7 +170,7 @@ public class CXUnsavedFile {
      * unsigned long Length
      * }
      */
-    public static final OfLong Length$layout() {
+    public static final ValueLayout Length$layout() {
         return Length$LAYOUT;
     }
 
@@ -184,7 +193,11 @@ public class CXUnsavedFile {
      * }
      */
     public static long Length(MemorySegment struct) {
-        return struct.get(Length$LAYOUT, Length$OFFSET);
+        return switch (Length$LAYOUT) {
+            case OfInt l -> struct.get(l, Length$OFFSET);
+            case OfLong l -> struct.get(l, Length$OFFSET);
+            default -> throw new IllegalStateException("Unhandled layout: " + Length$LAYOUT);
+        };
     }
 
     /**
@@ -194,7 +207,11 @@ public class CXUnsavedFile {
      * }
      */
     public static void Length(MemorySegment struct, long fieldValue) {
-        struct.set(Length$LAYOUT, Length$OFFSET, fieldValue);
+        switch (Length$LAYOUT) {
+            case OfInt l -> struct.set(l, Length$OFFSET, Math.toIntExact(fieldValue));
+            case OfLong l -> struct.set(l, Length$OFFSET, fieldValue);
+            default -> throw new IllegalStateException("Unhandled layout: " + Length$LAYOUT);
+        }
     }
 
     /**


Please also update the copyright years of the edited files.

I'll see about getting the GitHub actions working on Windows again.

src/main/java/org/openjdk/jextract/clang/TranslationUnit.java line 96:

> 94:                 else {
> 95:                    start.set((ValueLayout.OfLong) C_LONG, LENGTH_OFFSET, (long) inMemoryFiles[i].contents.length());
> 96:                 }

There is some history here, but looking at this again, I think it would be better to use the setters that jextract generates here.


CXUnsavedFile.Filename(start, arena.allocateFrom(inMemoryFiles[i].file));
CXUnsavedFile.Contents(start, arena.allocateFrom(inMemoryFiles[i].contents));
CXUnsavedFile.Length(start, inMemoryFiles[i].contents.length());

src/main/java/org/openjdk/jextract/clang/libclang/Index_h.java line 111:

> 109:         }
> 110:     }
> 111: 

I think you can just cast here
Suggestion:

    public static final ValueLayout C_LONG = (ValueLayout) Linker.nativeLinker().canonicalLayouts().get("long");

-------------

PR Review: https://git.openjdk.org/jextract/pull/289#pullrequestreview-3239416895
PR Review Comment: https://git.openjdk.org/jextract/pull/289#discussion_r2359484229
PR Review Comment: https://git.openjdk.org/jextract/pull/289#discussion_r2359037109