RFR: Improve formatting of generated source code

Jorn Vernee jvernee at openjdk.org
Fri Dec 15 15:30:09 UTC 2023


On Fri, 15 Dec 2023 15:22:22 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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 fi...
>
> src/main/java/org/openjdk/jextract/impl/HeaderFileBuilder.java line 241:
> 
>> 239:         }
>> 240:         if (!libraries.isEmpty()) {
>> 241:             appendBlankLine();
> 
> Do we really need a blank line here? Aren't we inside a method body? (so this is code, extra space is not needed here?)

Yeah, it could be omitted. The current code has the intent to add one so I kept that.

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

PR Review Comment: https://git.openjdk.org/jextract/pull/166#discussion_r1428104232


More information about the jextract-dev mailing list