[foreign] RFR 8219932: Split jextract's Context into multiple (immutable) sub-components

[Maurizio Cimadamore]

Very nice, I like it. You manage to keep things relatively simple, 
without over-engineering things too much, which is always hard with 
these kind of cleanups.

My #1 request is that we encapsulate logging methods a bit more. I don't 
like that clients have to (morally) do:

Log.stderr.println(Log.format(...))

It feels like 'println' should be an instance method of Log.

Note also that jextract always seems to use stderr for reporting 
diagnostics, so Log::println should probably use that (and keep Log::out 
around in case you need it).

Number #2 request, also Log-related, is that not all diagnostics are 
created equals - some are errors we should always report, other are 
warnings, other are notes that are only enabled in debugging mode. It 
would be nice if Log had different methods for these:

printError
printWarning
printNote

which then allows us to do centralized filtering of the diagnostic that 
are reported by jextract.

Maurizio


On 28/02/2019 16:02, Jorn Vernee wrote:
> Hi,
>
> Continuing this from an earlier thread [1]. Here is the first 
> iteration of the ideas that were discussed.
>
> This patch splits up Context into multiple sub-components. These are:
>
> - Options, a collection of options for a jextract run.
> - Log, logging PrintWriters + Logger
> - A List<Path> with jextract source (.h) files.
>
> Context is then used as an argument to a particular jextract run.
>
> I've held off on adding JextractTool as a sub-component as well, since 
> that would create a circular dependency, and I'm not sure what the 
> advantage of adding it would be at this time, since it's not accessed 
> by other internal parts of the jextract run.
>
> I've moved some functions to other classes, where I felt they made 
> more sense;
>
> - Main.format -> Log.format
> - Main.getBuiltinHeadersDir -> Context.getBuiltinHeadersDir
> - Context.findLibraryPath -> Utils.findLibraryPath
>
> To support the immutability of the Options sub-component I created a 
> builder for it, and moved the code that creates it to a separate 
> method in Main. In some cases the code wanted to exit early and return 
> an exit code, this is now replaced with throwing a custom exception, 
> which is caught in the `int run(String[] args)` method.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8219932
> Webrev: 
> http://cr.openjdk.java.net/~jvernee/panama/webrevs/8219932/webrev.00/
>
> Thanks,
> Jorn
>
> [1] : 
> https://mail.openjdk.java.net/pipermail/panama-dev/2019-February/004627.html