New subject: RFR: Improve formatting of generated source code [v2]

15 Dec 2023

      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

RFR: Improve formatting of generated source code

Jorn Vernee

Maurizio Cimadamore

Jorn Vernee

Maurizio Cimadamore

Jorn Vernee

Jorn Vernee

Jorn Vernee

Jorn Vernee

Maurizio Cimadamore

Jorn Vernee

tags

participants (2)