[foreign-jextract] RFR: 8253725: Jextract fails to extract big monolithic headers [v2]

[Maurizio Cimadamore]

On Mon, 28 Sep 2020 17:10:43 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/ToplevelBuilder.java line 75:
>> 
>>> 73:         FunctionalInterfaceBuilder builder = new FunctionalInterfaceBuilder(this, name, mtype, desc, type);
>>> 74:         builder.classBegin();
>>> 75:         builder.classEnd();
>> 
>> This looks a little suspicious at first glance. It seems like it would just be a class open and close like:
>> class MyClass {
>> }
>> But actually the classEnd() method is adding some other things as a side effect. In other words, it looks like this
>> code _should_ tell me what is being added by the builder, but actually part of it is hidden.
>> I think this should either be made explicit here, e.g. having
>> builder.classBegin();
>> builder.addStuff();
>> builder.classEnd();
>> Or, we say that the entire building process is the responsibility of the FunctionalInterfaceBuilder, and here we just
>> have:
>> builder.build(); // does all 3
>> WDYT?
>
> I see what you mean. Right now, one piece of messiness is that the class contents are attached to either class
> begin/end, w/o much discipline. I'll see what I can do.

I did some investigation here - and it doesn't seem like adding extra structure here is worth it. Adding build might
risk (if we make it general) to conflict with HeaderBuilder.build which does a different task.

As for `addStuff` while in principle that's obvious, in practice that's a bundle of static code that we want to
generate with the class - so if we go for that we're asking all clients to do classBegin/addStuff/classEnd
(OutputFactory would be affected badly).

So, yes, as is it looks awful, but I think we should try to fix it in a more radical rewriting of this code.

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

PR: https://git.openjdk.java.net/panama-foreign/pull/357