RFR: 7903344: refactor OutputFactory to introduce various Declaration Visitor classes [v2]

[Maurizio Cimadamore]

On Mon, 3 Oct 2022 15:25:35 GMT, Athijegannathan Sundararajan <sundar at openjdk.org> wrote:

>> Separate visitors for each activity (duplicate removal, include symbol filtering, name mangling and output generation)
>
> Athijegannathan Sundararajan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   moved getAsSignedOrUnsigned to Utils. Using Utils function isStructOrUnion instead of enum check.

The patch improves the code, by separating orthogonal concerns into different visitor passes. It would of course be great if we could go fully there with name mangling as well, but regardless, it looks like a solid step forward (perhaps name mangling can be tackled separately).

samples/libffmpeg/compile.sh line 3:

> 1: jextract -t libffmpeg \
> 2:   -I /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include \
> 3:   -I /usr/local/Cellar/ffmpeg at 4/4.4.2_4/include \

Are these changes deliberate?

src/main/java/org/openjdk/jextract/impl/CodeGenerator.java line 40:

> 38:         var nameMangler = new NameMangler(headerName);
> 39:         var transformedDecl = Stream.of(decl).
> 40:             map(new IncludeFilter(includeHelper)::transform).

Nice pipeline!

src/main/java/org/openjdk/jextract/impl/IncludeFilter.java line 45:

> 43: 
> 44:     @Override
> 45:     public Declaration.Scoped transform(Declaration.Scoped header) {

Good to see this stuff factored away

src/main/java/org/openjdk/jextract/impl/NameMangler.java line 43:

> 41:  * java safe names via lookup methods.
> 42:  *
> 43:  * NOTE: Unlike other transforming tree visitors, this one is just identify

Can't we drop the TreeTransformer implementation?

src/main/java/org/openjdk/jextract/impl/NameMangler.java line 47:

> 45:  * Subsequent code generation steps can check the collected names using getters.
> 46:  */
> 47: final class NameMangler implements TreeTransformer, Declaration.Visitor<Void, Declaration> {

As discussed offline, there is still coupling between this class and OutputFactory. We have tried several approaches none of which "clicked", so we ended up collecting state in the mangler.

A cleaner solution would be to decorate declarations with "name" attributes, but adding an attribute creates a new declaration, which means there will be types in the tree which would need fixing up. Which is also tricky.

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

PR: https://git.openjdk.org/jextract/pull/81