[foreign-jextract] RFR: 8250518: Jextract should process macros in bulk

[Maurizio Cimadamore]

On Mon, 27 Jul 2020 10:26:22 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> This patch adds bulk processing of macros; that is, instead of having a reparsing step for every macro constant, this
>> patch tweaks jextract so that all unparsed macros are processed in one go.
>> The logic is quite convoluted and this work led to a major refactoring of MacroParserImpl; there is now a *table* where
>> unparsed macros are kept - elements in this table are reparsed, and, based on the contents of the reparsed cursors, the
>> table state is updated accordingly (the code has quite a bit of doc documenting the possible state transitions).  This
>> results in some 2x performance boost in some of the heaviest extraction runs.
>> One thing to notice is that one of the test required some tweaks, as the new code seems to be able to infer a sharper
>> type for a macro - e.g. `foo*` instead of `void*`. I suspect this is related to `TypeMaker::resolveTypeReferences` but
>> I'm not 100% sure - @slowhog  can you please take a look?
>
> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/MacroParserImpl.java line 323:
> 
>> 322:                                 null : typeMaker.makeType(decl.type());
>> 323:                         newEntry = entry.failure(type);
>> 324:                     }
> 
> Not sure what's going on here exactly, but AFAICS passing null to `failure` will either return null, making newEntry
> null, or throw an exception.
> Would it be clearer to assign null to newEntry directly? (Also, maybe using a switch expression?)

Not sure I understand. If the old entry is an UnparsedMacro, then failure(null) is != from failure(T) where T != null;
the former returns null (meaning no entry), while the latter creates a new recoverable failure entry. The exception you
mention is thrown regardless of the type - and occurs when failure is called on an entry for which no state transition
associated to failure exists.

I can update the code to use a switch expression, but I think that encapsulating the state transition logic into the
methods of the entry leads to better code - this is essentially an OO implementation of a finite state machine.

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

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