RFR: 7903590: Refactor jextract source code generation

[Maurizio Cimadamore]

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::classBegin`.
> 
> 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...

src/main/java/org/openjdk/jextract/impl/ClassSourceBuilder.java line 57:

> 55:     private final String superName;
> 56: 
> 57:     ClassSourceBuilder(SourceFileBuilder builder, String modifiers, Kind kind, String className, String superName, List<String> enclosingClassNames) {

If this models a class, and classes can be nested, it is not clear to me as to why we wouldn't have a pointer to the enclosing class, rather than having just a list of names?

src/main/java/org/openjdk/jextract/impl/SourceFileBuilder.java line 31:

> 29: import java.util.function.Function;
> 30: 
> 31: public class SourceFileBuilder {

Should this class be final?

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

PR Review Comment: https://git.openjdk.org/jextract/pull/141#discussion_r1406062404
PR Review Comment: https://git.openjdk.org/jextract/pull/141#discussion_r1406063946