RFR: 7903590: Refactor jextract source code generation

[Jorn Vernee]

On Wed, 22 Nov 2023 07:38:29 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> We currently use `ClassSourceBuilder` both for top level classes and nested classes. That means for instance that we need to check whether to generate package declarations and import statements, and for nested classes we also emit code into a separate intermediate buffer that is copied into the enclosing buffer when we end the nested class. This PR proposes splitting part of the logic in `ClassSourceBuilder` into a separate `SourceFileBuilder`, which handles package decl/imports, and holds the underling `StringBuilder` which allows a nested class to share the `StringBuilder` with its enclosing class. Each `SourceFileBuilder` produces exactly one `JavaFileObject`. `TopLevelBuilder` and `Constants` now aggregate over `SourceFileBuilder`s instead of `JavaSourceBuilder`s. This also allows reducing the number of classes that implement `JavaSourceBuilder` to just `TopLevelBuilder` and `StructBuilder`, and allows dropping several methods from the interface. Other builders now wrap `Source
 FileBuilder`, and emit code into its buffer.
> 
> Jextract source code generation related code currently relies on method overriding to allow sub classes to inject behavior into the generation code in a super class. This, however, makes it hard to follow what method implementation is actually being called when reading code. And, any method call may become 'suspicious' because it might do something non-obvious. For example, `emitConstructor` is overridden to do nothing in several sub-classes. But, this is not obvious when seeing the `emitConstructor()` call in `ClassSourceBuilder::begin`.
> 
> Furthermore, we use method overriding for instance to specify the class modifiers for a `ClassSourceBuilder` subclass, which means that control flow will flow through several class levels when generating code. This, and other cases, could be replaced by a simple (`String`) value passed to `ClassSourceBuilder` constructor. Using a direct field value in the generation code makes it clear that there is nothing special going on (as the method call might imply).
> 
> This PR proposes to remove most uses of method overriding, to make it easier to follow control flow when reading the code. This has a few implications:
> -  Direct field access is used where possible, and data is passed to *Builder constructors instead of through a virtual method call to a sub class implementation
> - `emitConstructor` was not needed in all cases. So, I've moved it from `ClassSourceBuilder::classBegin` to callers of that method inst...

src/main/java/org/openjdk/jextract/impl/Constants.java line 89:

> 87:         if (currentBuilder.constantIndex > CONSTANTS_PER_CLASS) {
> 88:             currentBuilder.classEnd();
> 89:             currentBuilder = startNewBuilder(packageName, constantBuilders.size());

`currentBuilder` is never `null`, so we don't need to check for that.

src/main/java/org/openjdk/jextract/impl/Constants.java line 231:

> 229:             return kind == ClassSourceBuilder.Kind.CLASS ?
> 230:                     "static final " : "";
> 231:         }

Note that `Builder`s always have the `CLASS` kind, so we can just use a `static final String` for the modifiers.

src/main/java/org/openjdk/jextract/impl/Constants.java line 251:

> 249:                 return constantName;
> 250:             }
> 251: 

This method was unused, so I've removed it.

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

PR Review Comment: https://git.openjdk.org/jextract/pull/141#discussion_r1401624670
PR Review Comment: https://git.openjdk.org/jextract/pull/141#discussion_r1401625856
PR Review Comment: https://git.openjdk.org/jextract/pull/141#discussion_r1401625134