[foreign-jextract] RFR: API refresh - part two (jextract edition)

[Duncan Gittins]

I'm no expert on this: occasionally I've seen posts that suggest that this
test won't always return true on Windows in certain languages:

     private static final boolean IS_WINDOWS = System.getProperty("os.name
").startsWith("Windows");

Other suggestions I've seen have used
     .toLowerCase().contains("windows")
or:
     .toLowerCase(Locale.ENGLISH).contains("windows")

Do you have definitive knowledge of which is correct?

Kind regards

Duncan


On Thu, 7 Oct 2021 at 19:37, Paul Sandoz <psandoz at openjdk.java.net> wrote:

> On Thu, 7 Oct 2021 17:06:20 GMT, Maurizio Cimadamore <
> mcimadamore at openjdk.org> wrote:
>
> > This patch fixes jextract to work with the latest API changes.
> >
> > The patch reintroduces overloads for foreign function wrappers which
> take a scope (since ResourceScope is no longer a subtype of
> SegmentAllocator).
> >
> > We will consider later whether to drop these overloads; it is likely
> that the majority of jextract clients will already have an allocator
> somewhere, so the extra code is not necessary. On the other hand, if we
> want overloads, then I think it would also be useful to add an overload
> that takes a `MemorySegment` and converts it into a `prefixAllocator` (so
> that clients can store result of foreign function which returns a
> struct-by-value in a pre-existing segments).
> >
> > I've made some improvements to the string conversion logic; in a lot of
> places we were creating a scope, then doing a malloc to allocate a
> temporary CXString, then extract a Java String from the CXString, and
> dispose/free the CXString. Since jextract is single-threaded, I replaced
> all that with an high-order function which takes a segment supplier, and
> uses a prefix allocator under the hood. This should be way more efficient
> than what we had - and string conversion is quite frequent in libclang
> (e.g. `CXCursor::spelling`).
> >
> > The patch passes all tests.
>
> src/jdk.incubator.jextract/share/classes/jdk/internal/clang/LibClang.java
> line 50:
>
> > 48:     private static final boolean IS_WINDOWS = System.getProperty("
> os.name").startsWith("Windows");
> > 49:
> > 50:     final static SegmentAllocator IMPLICIT_ALLOCATOR =
>
> That's might be a useful addition as a constant on `SegmentAllocator`.
>
> src/jdk.incubator.jextract/share/classes/jdk/internal/clang/LibClang.java
> line 94:
>
> > 92:      * conversion. The size of the prefix segment is set to 256,
> which should be enough to hold a CXString.
> > 93:      */
> > 94:     private final static SegmentAllocator stringAllocator =
> SegmentAllocator.prefixAllocator(
>
> Suggestion:
>
>     private final static SegmentAllocator STRING_ALLOCATOR =
> SegmentAllocator.prefixAllocator(
>
> -------------
>
> PR: https://git.openjdk.java.net/panama-foreign/pull/594
>