RFR: 7903649: Field and global variables of array type should have indexed accessors

[Maurizio Cimadamore]

On Tue, 30 Jan 2024 14:43:15 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> FTR, if the constant class is called `<varName>$constants` all the ambiguities issues are resolved, and all tests pass. But, we're back with dollar names :-)
>> 
>> If we're fine with dollar names for "less common API points", we could settle by having a single extra layout accessor/descriptor accessor per var/field/function. I think the layout API should expose an handy method for computing dimensions of a possibly nested sequence layout.
>
>> we could settle by having a single extra layout accessor/descriptor accessor per var/field/function
> 
> Although, I think I like the idea of exposing multiple related constants under a single umbrella - given that in most cases we generate the holder class anyway. In many cases there seem to be multiple interesting constants for a given symbol - and if we were to expose different constant accessors, the constant accessor would quickly outnumber the _real_ API point, which would then lead us back in a situation of poor API discoverability.
> 
> In other words, while reserving the "good name" to the common API points remains a good move, if such good names are buried under several "less than good names", the API quickly becomes bloated, which is still problematic, unfortunately.

I've put together another branch where I fully explored the idea of exposing constants (for global vars, functions and struct fields) in a separate holder class:

https://github.com/openjdk/jextract/compare/panama...mcimadamore:jextract:public_holders?expand=1

For globals and functions, not much changes (as we needed an holder class even before this).

For structs, additional holder classes are generated (one per field). For instance, here's what `Point` looks like:


/**
 * {@snippet lang=c :
 * struct Point {
 *     int x;
 *     int y;
 * }
 * }
 */
public class Point {

    Point() {
        // Should not be called directly
    }

    private static final GroupLayout $LAYOUT = MemoryLayout.structLayout(
        foo_h.C_INT.withName("x"),
        foo_h.C_INT.withName("y")
    ).withName("Point");

    /**
     * The layout of this struct
     */
    public static final GroupLayout layout() {
        return $LAYOUT;
    }

    public static class x$constants {
        public static final OfInt LAYOUT = (OfInt)layout().select(groupElement("x"));
        public static final long OFFSET = layout().byteOffset(groupElement("x"));
    }

    /**
     * Getter for field:
     * {@snippet lang=c :
     * int x
     * }
     */
    public static int x(MemorySegment struct) {
        return struct.get(x$constants.LAYOUT, x$constants.OFFSET);
    }

    /**
     * Setter for field:
     * {@snippet lang=c :
     * int x
     * }
     */
    public static void x(MemorySegment struct, int fieldValue) {
        struct.set(x$constants.LAYOUT, x$constants.OFFSET, fieldValue);
    }

    public static class y$constants {
        public static final OfInt LAYOUT = (OfInt)layout().select(groupElement("y"));
        public static final long OFFSET = layout().byteOffset(groupElement("y"));
    }

    /**
     * Getter for field:
     * {@snippet lang=c :
     * int y
     * }
     */
    public static int y(MemorySegment struct) {
        return struct.get(y$constants.LAYOUT, y$constants.OFFSET);
    }

    /**
     * Setter for field:
     * {@snippet lang=c :
     * int y
     * }
     */
    public static void y(MemorySegment struct, int fieldValue) {
        struct.set(y$constants.LAYOUT, y$constants.OFFSET, fieldValue);
    }

    /**
     * Obtains a slice of {@code arrayParam} which selects the array element at {@code index}.
     * The returned segment has address {@code arrayParam.address() + index * layout().byteSize()}
     */
    public static MemorySegment asSlice(MemorySegment array, long index) {
        return array.asSlice(layout().byteSize() * index);
    }

    /**
     * The size (in bytes) of this struct
     */
    public static long sizeof() { return layout().byteSize(); }

    /**
     * Allocate a segment of size {@code layout().byteSize()} using {@code allocator}
     */
    public static MemorySegment allocate(SegmentAllocator allocator) {
        return allocator.allocate(layout());
    }

    /**
     * Allocate an array of size {@code elementCount} using {@code allocator}.
     * The returned segment has size {@code elementCount * layout().byteSize()}.
     */
    public static MemorySegment allocateArray(long elementCount, SegmentAllocator allocator) {
        return allocator.allocate(MemoryLayout.sequenceLayout(elementCount, layout()));
    }

    /**
     * Reinterprets {@code addr} using target {@code arena} and {@code cleanupAction) (if any).
     * The returned segment has size {@code layout().byteSize()}
     */
    public static MemorySegment reinterpret(MemorySegment addr, Arena arena, Consumer<MemorySegment> cleanup) {
        return reinterpret(addr, 1, arena, cleanup);
    }

    /**
     * Reinterprets {@code addr} using target {@code arena} and {@code cleanupAction) (if any).
     * The returned segment has size {@code elementCount * layout().byteSize()}
     */
    public static MemorySegment reinterpret(MemorySegment addr, long elementCount, Arena arena, Consumer<MemorySegment> cleanup) {
        return addr.reinterpret(layout().byteSize() * elementCount, arena, cleanup);
    }
}


Maybe this is too much - however I like how tidy the constants are under the dedicated constant class.

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

PR Review Comment: https://git.openjdk.org/jextract/pull/198#discussion_r1471469825