RFR: 7903586: Revisit jextract constant classes
Maurizio Cimadamore
mcimadamore at openjdk.org
Wed Nov 29 11:11:06 UTC 2023
On Wed, 29 Nov 2023 11:05:24 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:
>> This PR removes the use of jextract constant classes, and falls back to a simpler code generation strategies which move the various constants closer to where they belong:
>>
>> * for structs and callbacks, the constants are added to the class/interface being generated;
>> * for the header class, we use a local holder class per function to hold the method handle and function descriptor constants.
>>
>> This scheme results in much less generated code (as no constant class is emitted), but with the same performance and laziness as the old scheme.
>>
>> When working with the code, I've decided to simplify the handling of constant generation, in the spirit of what was recently done with the introduction of string templates. That is, instead of using a magic `Constant` class, which has methods to generate getters/setters for a given constant, I converted much of the code into "static" string templates, so that it's easier to see which code gets generated when staring at the jextract code. As a result, the `Constant` class has been removed.
>
> src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java line 167:
>
>> 165: \{!header.isEmpty() ? STR." * \{header}\n" : ""}\
>> 166: * {@snippet lang=c :
>> 167: \{CDeclarationPrinter.declaration(decl, " * ")}
>
> This fix is unrelated - but I noticed that all comment lines had some skewed indentation - probably a result of the fact that now the whole comment is added in a `appendLines` call with a single string template.
This also brought in some changes in `CDeclarationPrinter` which was adding newlines on its own, which now conflicted with string template usage (e.g I could see extra _blank_ lines being inserted in the javadoc).
> src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java line 209:
>
>> 207: }
>> 208:
>> 209: private void layoutString(int textBoxIndent, StringBuilder builder, MemoryLayout l) {
>
> Note: the logic is a bit convoluted because we have to add some indentation to the resulting string, as it can span multiple lines. And so that it is correctly aligned inside the text box in which it appears into.
Ideally, we could avoid all this by just using low-level append calls - but that would make it hard for clients to just embed the layout inside a string template, and make the code less readable, so I went for this instead. (longer term, I think perhaps we should have a "string builder with indentation" that we can reuse as a building block for things like this).
-------------
PR Review Comment: https://git.openjdk.org/jextract/pull/145#discussion_r1409123772
PR Review Comment: https://git.openjdk.org/jextract/pull/145#discussion_r1409120096
More information about the jextract-dev
mailing list