December 2023 - jextract-dev

RFR: 7903613: Bad nested names are sometimes attached to structs
by Maurizio Cimadamore 11 Jan '24

11 Jan '24

The `NameMangler` visitor is used to compute the Java name of a jextract declaration. This is implemented as a declaration visitor. Unfortunately, the logic that computes the Java name can be sensitive to the order in which declarations are visited (because this visitor features a "parent" declaration, whose contents affect as to whether a "nested" struct name is generated or not). In reality, the logic of the name mangler needs to be able to disambiguate between structs that are either anonymous, or already declared somewhere else, and structs that are declared as part of a typedef, variable, function parameter/return declaration. In the former case, we either need no Java name (anonymous struct) or a toplevel Java name. In the latter we need a nested struct name (as the struct class will be nested inside some other class). This PR introduces a new visitor which tags all struct/union/enum declarations which fall in the latter bucket. This is done with an algorithm which: 1. visits all declarations in a toplevel header 2. remembers which scoped declarations have been seen *directly* (e.g. as part of the visit) 3. keeps track of which scoped declarations can be seen *indirectly* (e.g. because they are behind some declared type) 4. subtracts the declarations in (2) from the declarations in (3), and visits the declarations in the remaining set 5. keeps performing (2), (3), (4) until there's no declaration in (3) All scoped declarations that appear exclusively as part of some declared type are augmented with the `NestedDecl` attribute, which is then read when calling `Utils::nestedDeclarationFor`. This ensures that all the jextract visitor only recurse on a scoped declaration attached to a type which is known not to have been seen anywhere else. As a result, the behavior of the name mangler is independent of the order in which declarations are seen. It should be possible, in principle, to leverage this infrastructure to define a declaration visitor that automatically looks inside "nested declarations" (so that subsequent visitors don't really need to concern with following declared types). I've tested this change with windows.h, which works as expected. ------------- Commit messages: - Drop spurious code changes - Make nested declaration finder logic more robust - Fix out of order test - Add tests - Tweak nested decl finder to look inside struct members that are themselves structs - Add new nested decl scanner - Drop unused field in OutputFactory - All tests pass Changes: https://git.openjdk.org/jextract/pull/167/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=167&range=00 Issue: https://bugs.openjdk.org/browse/CODETOOLS-7903613 Stats: 329 lines in 12 files changed: 266 ins; 31 del; 32 mod Patch: https://git.openjdk.org/jextract/pull/167.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/167/head:pull/167 PR: https://git.openjdk.org/jextract/pull/167

2 25

RFR: 7903615: Jextract should generate javadoc contents using libclang pretty printer
by Maurizio Cimadamore 10 Jan '24

10 Jan '24

This PR removes CDeclarationPrinter and, instead, uses libclang pretty-printing capabilities to print the javadoc contents for a given declaration. The new logic will compute the comment string at parse time, in `TreeMaker`. The comment is then attached to the declaration using an attribute (and later retrieved when it's time to generate code). I've used some custom printing policy options, after testing several of them to try and find the best results. With these options, there are two main differences from what we used to generate before: 1. named structs/unions are re-expanded in function declarations (e.g. the javadoc of a function accepting a `struct Foo` will also contain the definition of `Foo`) 2. there is no pretty-printing equivalent for types. This means that, for functional interfaces, we instead print the "parent" declaration, e.g. the declaration whose type is a function pointer. It would be nice if we could improve on (1) by adding a new policy e.g. `IncludeAnonymousTagDefinition` - e.g. it is useful to have tag definitions inlined when the definition is anonymous (otherwise clang will generate a blob of text containing the source location of the anon definition), but it would be better perhaps not to inline the definition if it's not anonymous. Example: struct Point { int x; int y; }; void func(struct Point *pp); This generates the following strings: struct Point { int x; int y; } void func(struct Point { int x; int y; } *pp) I don't think it's a deal breaker, but it does make the javadoc more verbose. Overall the changes are quite straightforward, and the code is simplified considerably. Also, this fixes a lot of little issues that arise when we try to generate javadoc for anonymous structs for which before only `struct` was printed. There are only two situations in which I had to manually improve the clang output: * macros - in this case pretty printer prints nothing :-) so I use the macro tokens to generate the corresponding doc string; * enum constant - in this case pretty printer only prints the enum constant name, but the previous output had also the enum name and the value. I tweaked enum constants manually (also in `TreeMaker`). By doing that, I could then also remove `EnumConstantLifter`. ------------- Commit messages: - Added support for enum constants - Add macro doc support - Initial push Changes: https://git.openjdk.org/jextract/pull/169/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=169&range=00 Issue: https://bugs.openjdk.org/browse/CODETOOLS-7903615 Stats: 525 lines in 14 files changed: 67 ins; 383 del; 75 mod Patch: https://git.openjdk.org/jextract/pull/169.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/169/head:pull/169 PR: https://git.openjdk.org/jextract/pull/169

2 12

RFR: 7903616: Bad formatting of layout string for structs with nested anon structs/unions
by Maurizio Cimadamore 10 Jan '24

10 Jan '24

This PR fixes an indentation issue when generating a struct layout string. Example: struct Foo { struct { int x; union { int y; int z; }; }; }; For this, jextract generates: private static final GroupLayout $LAYOUT = MemoryLayout.structLayout( MemoryLayout.structLayout( foo_h.C_INT.withName("x"), MemoryLayout.unionLayout( foo_h.C_INT.withName("y"), foo_h.C_INT.withName("z") ).withName("$anon$7:9") ).withName("$anon$5:5") ).withName("Foo"); With the fix in this PR, the generated string is: private static final GroupLayout $LAYOUT = MemoryLayout.structLayout( MemoryLayout.structLayout( foo_h.C_INT.withName("x"), MemoryLayout.unionLayout( foo_h.C_INT.withName("y"), foo_h.C_INT.withName("z") ).withName("$anon$7:9") ).withName("$anon$5:5") ).withName("Foo"); The fix is to correctly track required indentation when descending into a scoped declaration. ------------- Commit messages: - Initial push Changes: https://git.openjdk.org/jextract/pull/170/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=170&range=00 Issue: https://bugs.openjdk.org/browse/CODETOOLS-7903616 Stats: 9 lines in 1 file changed: 1 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jextract/pull/170.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/170/head:pull/170 PR: https://git.openjdk.org/jextract/pull/170

2 5

RFR: Refresh libclang
by Maurizio Cimadamore 10 Jan '24

10 Jan '24

Following the latest jextract changes, it is now time to refresh the libclang classes used by jextract. As a result, this change will drop `RuntimeHelper` and all constant classes (as the new generation strategy no longer needs them). I encountered a couple of issues when running the updating script, which prompted me to simplify the script, and to add some more documentation. ------------- Commit messages: - Refresh libclang Changes: https://git.openjdk.org/jextract/pull/168/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=168&range=00 Stats: 7505 lines in 36 files changed: 3229 ins; 2808 del; 1468 mod Patch: https://git.openjdk.org/jextract/pull/168.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/168/head:pull/168 PR: https://git.openjdk.org/jextract/pull/168

2 3

RFR: 7903239: ofAddress factory of function pointer type is wrong for struct returns
by Marko 02 Jan '24

02 Jan '24

When generating the lambda inside the `ofAddress` factory, it is missing the `SegmentAllocator` needed when the function returns a struct. This is [implemented for regular functions](https://github.com/openjdk/jextract/blob/0582eaf1b4cdba95f0ee8c2…, but not for function pointers. ------------- Commit messages: - 7903239: ofAddress factory of function pointer type is wrong for struct returns Changes: https://git.openjdk.org/jextract/pull/58/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=58&range=00 Issue: https://bugs.openjdk.org/browse/CODETOOLS-7903239 Stats: 47 lines in 3 files changed: 44 ins; 2 del; 1 mod Patch: https://git.openjdk.org/jextract/pull/58.diff Fetch: git fetch https://git.openjdk.org/jextract pull/58/head:pull/58 PR: https://git.openjdk.org/jextract/pull/58

3 11

RFR: Add path independence
by Per Minborg 02 Jan '24

02 Jan '24

This small PR incorporated feedback from the original PR: https://github.com/openjdk/jextract/pull/66 made after it was integrated. ------------- Commit messages: - Add path independence Changes: https://git.openjdk.org/jextract/pull/68/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=68&range=00 Stats: 10 lines in 4 files changed: 2 ins; 0 del; 8 mod Patch: https://git.openjdk.org/jextract/pull/68.diff Fetch: git fetch https://git.openjdk.org/jextract pull/68/head:pull/68 PR: https://git.openjdk.org/jextract/pull/68

5 7

RFR: Add Dockerfile, zero-dep reproducible builds
by Gavin Ray 02 Jan '24

02 Jan '24

As title says, adds a Dockerfile that will fetch all necessary deps and build `jextract` for you on Ubuntu 22.10 It would be nice if some official JDK organization could put this on their Dockerhub, so that users could just do: $ docker run --rm -it openjdk/jextract:latest I've pushed it to my personal Dockerhub, so if anyone would like to test it out, you can use the README directions but prepend `gavinray/` before the image name: $ echo "struct Foo { int a; long b; }; int use_foo(struct Foo foo);" >> sample.h $ docker run --rm -it -v ${PWD}:/tmp/jextract \ gavinray/jextract:latest \ -I /tmp/jextract \ --source \ -l libfoo \ --output /tmp/jextract/generated \ --target-package com.example \ ------------- Commit messages: - Add Dockerfile, zero-dep reproducible builds Changes: https://git.openjdk.org/jextract/pull/74/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=74&range=00 Stats: 75 lines in 2 files changed: 75 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jextract/pull/74.diff Fetch: git fetch https://git.openjdk.org/jextract pull/74/head:pull/74 PR: https://git.openjdk.org/jextract/pull/74

3 6

RFR: Add third-party sample list to README, populate with java-vulkan.
by Michael Ennen 02 Jan '24

02 Jan '24

I made a more complex example (1000 lines, mostly because setting up a boilerplate Vulkan program is super lengthy) of using jextract on Vulkan and Win32 APIs. I think it may be too lengthy and too much of a maintenance burden to commit to the `/samples` directory, so I thought maybe including a section on third-party samples would be better. If this is not something you want to entertain that's totally understandable, but thought I would put it out there a bigger sample that interfaces with a pretty complex API may be helpful to others. ------------- Commit messages: - Add third-party sample list to README, populate with java-vulkan. Changes: https://git.openjdk.org/jextract/pull/93/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=93&range=00 Stats: 3 lines in 1 file changed: 2 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jextract/pull/93.diff Fetch: git fetch https://git.openjdk.org/jextract pull/93/head:pull/93 PR: https://git.openjdk.org/jextract/pull/93

3 3

RFR: Improve formatting of generated source code
by Jorn Vernee 15 Dec '23

15 Dec '23

As a main rule-of-thumb: we should generate a blank line before a multi-line declaration, or before a block of single line declarations (e.g. fields, or methods collapsed into a single line). In most cases, we can just add a blank line to the start of the String template we use. In some cases that emit single-line declarations, we have a manual call to `appendBlankLine` before emitting multiple single line decls. For declarations with comments we also add the blank line manually, as there shouldn't be a blank line between the comment and declaration. - I've also added the default constructor to every class we generate, and moved it before the layout decl in struct classes (makes a little bit more sense I think). - fixed the indentation and excess trailing whitespace after the class `}` of a class. - Fixed the way in which we add indentation to text blocks, so that we don't add trailing white space to empty lines. <details> <summary>Sample output (click to unfold)</summary> For instance for this header: struct Foo { int x; struct Bar { int y; } bar; }; typedef struct Foo (*CB)(void); Foo.java: // Generated by jextract import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.VarHandle; import java.nio.ByteOrder; import java.lang.foreign.*; import static java.lang.foreign.ValueLayout.*; /** * {@snippet lang=c : * struct Foo { * int x; * struct Bar bar; * }; * } */ public class Foo { Foo() { // Suppresses public default constructor, ensuring non-instantiability, // but allows generated subclasses in same package. } private static final GroupLayout $LAYOUT = MemoryLayout.structLayout( struct_with_fptr_h.C_INT.withName("x"), Foo.Bar.$LAYOUT().withName("bar") ).withName("Foo"); public static final GroupLayout $LAYOUT() { return $LAYOUT; } private static final long x$OFFSET = 0; /** * Getter for field: * {@snippet lang=c : * int x; * } */ public static int x$get(MemorySegment seg) { return seg.get(struct_with_fptr_h.C_INT, x$OFFSET); } public static int x$get(MemorySegment seg, long index) { return seg.get(struct_with_fptr_h.C_INT, x$OFFSET + (index * sizeof())); } /** * Setter for field: * {@snippet lang=c : * int x; * } */ public static void x$set(MemorySegment seg, int x) { seg.set(struct_with_fptr_h.C_INT, x$OFFSET, x); } public static void x$set(MemorySegment seg, long index, int x) { seg.set(struct_with_fptr_h.C_INT, x$OFFSET + (index * sizeof()), x); } /** * {@snippet lang=c : * struct Bar { * int y; * }; * } */ public static class Bar { Bar() { // Suppresses public default constructor, ensuring non-instantiability, // but allows generated subclasses in same package. } private static final GroupLayout $LAYOUT = MemoryLayout.structLayout( struct_with_fptr_h.C_INT.withName("y") ).withName("Bar"); public static final GroupLayout $LAYOUT() { return $LAYOUT; } private static final long y$OFFSET = 0; /** * Getter for field: * {@snippet lang=c : * int y; * } */ public static int y$get(MemorySegment seg) { return seg.get(struct_with_fptr_h.C_INT, y$OFFSET); } public static int y$get(MemorySegment seg, long index) { return seg.get(struct_with_fptr_h.C_INT, y$OFFSET + (index * sizeof())); } /** * Setter for field: * {@snippet lang=c : * int y; * } */ public static void y$set(MemorySegment seg, int x) { seg.set(struct_with_fptr_h.C_INT, y$OFFSET, x); } public static void y$set(MemorySegment seg, long index, int x) { seg.set(struct_with_fptr_h.C_INT, y$OFFSET + (index * sizeof()), x); } public static long sizeof() { return $LAYOUT().byteSize(); } public static MemorySegment allocate(SegmentAllocator allocator) { return allocator.allocate($LAYOUT()); } public static MemorySegment allocateArray(long len, SegmentAllocator allocator) { return allocator.allocate(MemoryLayout.sequenceLayout(len, $LAYOUT())); } public static MemorySegment ofAddress(MemorySegment addr, Arena scope) { return addr.reinterpret($LAYOUT().byteSize(), scope, null); } } private static final long bar$OFFSET = 4; private static final long bar$SIZE = 4; public static MemorySegment bar$slice(MemorySegment seg) { return seg.asSlice(bar$OFFSET, bar$SIZE); } public static long sizeof() { return $LAYOUT().byteSize(); } public static MemorySegment allocate(SegmentAllocator allocator) { return allocator.allocate($LAYOUT()); } public static MemorySegment allocateArray(long len, SegmentAllocator allocator) { return allocator.allocate(MemoryLayout.sequenceLayout(len, $LAYOUT())); } public static MemorySegment ofAddress(MemorySegment addr, Arena scope) { return addr.reinterpret($LAYOUT().byteSize(), scope, null); } } CB.java: // Generated by jextract import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.VarHandle; import java.nio.ByteOrder; import java.lang.foreign.*; import static java.lang.foreign.ValueLayout.*; /** * {@snippet lang=c : * struct Foo (*CB)(); * } */ public interface CB { MemorySegment apply(); FunctionDescriptor $DESC = FunctionDescriptor.of( Foo.$LAYOUT()); MethodHandle UP$MH = struct_with_fptr_h.upcallHandle(CB.class, "apply", $DESC); static MemorySegment allocate(CB fi, Arena scope) { return Linker.nativeLinker().upcallStub(UP$MH.bindTo(fi), $DESC, scope); } MethodHandle DOWN$MH = Linker.nativeLinker().downcallHandle($DESC); static CB ofAddress(MemorySegment addr, Arena arena) { MemorySegment symbol = addr.reinterpret(arena, null); return () -> { try { return (MemorySegment) DOWN$MH.invokeExact(symbol, (SegmentAllocator)arena); } catch (Throwable ex$) { throw new AssertionError("should not reach here", ex$); } }; } } struct_with_fptr_h.java: // Generated by jextract import java.lang.invoke.MethodHandle; import java.lang.invoke.MethodHandles; import java.lang.invoke.VarHandle; import java.nio.ByteOrder; import java.lang.foreign.*; import static java.lang.foreign.ValueLayout.*; public class struct_with_fptr_h { static final SymbolLookup SYMBOL_LOOKUP; static { SymbolLookup loaderLookup = SymbolLookup.loaderLookup(); Linker linker = Linker.nativeLinker(); SYMBOL_LOOKUP = name -> loaderLookup.find(name).or(() -> linker.defaultLookup().find(name)); } struct_with_fptr_h() { // Suppresses public default constructor, ensuring non-instantiability, // but allows generated subclasses in same package. } public static final ValueLayout.OfBoolean C_BOOL = ValueLayout.JAVA_BOOLEAN; public static final ValueLayout.OfByte C_CHAR = ValueLayout.JAVA_BYTE; public static final ValueLayout.OfShort C_SHORT = ValueLayout.JAVA_SHORT; public static final ValueLayout.OfInt C_INT = ValueLayout.JAVA_INT; public static final ValueLayout.OfLong C_LONG_LONG = ValueLayout.JAVA_LONG; public static final ValueLayout.OfFloat C_FLOAT = ValueLayout.JAVA_FLOAT; public static final ValueLayout.OfDouble C_DOUBLE = ValueLayout.JAVA_DOUBLE; public static final AddressLayout C_POINTER = ValueLayout.ADDRESS .withTargetLayout(MemoryLayout.sequenceLayout(java.lang.Long.MAX_VALUE, JAVA_BYTE)); public static final ValueLayout.OfInt C_LONG = ValueLayout.JAVA_INT; public static final ValueLayout.OfDouble C_LONG_DOUBLE = ValueLayout.JAVA_DOUBLE; static MemorySegment findOrThrow(String symbol) { return SYMBOL_LOOKUP.find(symbol) .orElseThrow(() -> new UnsatisfiedLinkError("unresolved symbol: " + symbol)); } static MemoryLayout[] inferVariadicLayouts(Object[] varargs) { MemoryLayout[] result = new MemoryLayout[varargs.length]; for (int i = 0; i < varargs.length; i++) { result[i] = variadicLayout(varargs[i].getClass()); } return result; } static MethodHandle upcallHandle(Class<?> fi, String name, FunctionDescriptor fdesc) { try { return MethodHandles.lookup().findVirtual(fi, name, fdesc.toMethodType()); } catch (ReflectiveOperationException ex) { throw new AssertionError(ex); } } static MethodHandle downcallHandleVariadic(String name, FunctionDescriptor baseDesc, MemoryLayout[] variadicLayouts) { FunctionDescriptor variadicDesc = baseDesc.appendArgumentLayouts(variadicLayouts); Linker.Option fva = Linker.Option.firstVariadicArg(baseDesc.argumentLayouts().size()); return SYMBOL_LOOKUP.find(name) .map(addr -> Linker.nativeLinker().downcallHandle(addr, variadicDesc, fva) .asSpreader(Object[].class, variadicLayouts.length)) .orElse(null); } // Internals only below this point private static MemoryLayout variadicLayout(Class<?> c) { // apply default argument promotions per C spec // note that all primitives are boxed, since they are passed through an Object[] if (c == Boolean.class || c == Byte.class || c == Character.class || c == Short.class || c == Integer.class) { return JAVA_INT; } else if (c == Long.class) { return JAVA_LONG; } else if (c == Float.class || c == Double.class) { return JAVA_DOUBLE; } else if (MemorySegment.class.isAssignableFrom(c)) { return ADDRESS; } throw new IllegalArgumentException("Invalid type for ABI: " + c.getTypeName()); } } </details> ------------- Commit messages: - switch constructor and first header preamble - add newline before constructor - polish - improve blank link usage in generated sources - fix indentation and placement of private default constructor - don't indent closing brace of class decl - remove indentation of trailing whitespace Changes: https://git.openjdk.org/jextract/pull/166/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=166&range=00 Stats: 68 lines in 7 files changed: 45 ins; 7 del; 16 mod Patch: https://git.openjdk.org/jextract/pull/166.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/166/head:pull/166 PR: https://git.openjdk.org/jextract/pull/166

2 9

RFR: Don't chase typedefs in Utils::declarationFor
by Maurizio Cimadamore 15 Dec '23

15 Dec '23

The fix for https://git.openjdk.org/jextract/pull/161 introduced a method, namely `Utils::declarationFor` which gives us a scoped declaration from a given type.This routine was implemented "the right way", but that revealed issues with the underlying IR, especially when extracting big header files like windows.h. In some cases the order in which cursors are seen is slightly different, and that leads to the wrong name to be attached to the typedeff'ed entity. This problem is also existing in the jdk22 branch, but given that branch has a much more primitive logic for visiting declaration types, it was never exposed in full. For now, the solution is to dumb down `Utils::declarationFor`. Of course a longer term solution is to make sure that declarations that need to be visited can be seen in the declaration tree, so that chasing types is no longer necessary. ------------- Commit messages: - Initial push Changes: https://git.openjdk.org/jextract/pull/165/files Webrev: https://webrevs.openjdk.org/?repo=jextract&pr=165&range=00 Stats: 3 lines in 1 file changed: 2 ins; 1 del; 0 mod Patch: https://git.openjdk.org/jextract/pull/165.diff Fetch: git fetch https://git.openjdk.org/jextract.git pull/165/head:pull/165 PR: https://git.openjdk.org/jextract/pull/165

2 4