[foreign] RFR: Make jextract Context immutable

[Jorn Vernee]

> I'm leaning on my experience with javac (and related tools) -
> sorry if what follows sounds too paternalistic :-)

Not at all. Thanks for sharing your experience!

I think what you say makes sense. I also felt like the PrintWriters for 
`out` and `err`, as well as the logger, felt like something different 
from the rest of Context. I think splitting up context into 
sub-components makes sense.

I'll do some experiments with that.

What I'd still like to see though, is that whatever thing we end up 
passing the do the jextract run is immutable. I feel it's a more robust 
design, since we can't get into a situation where one of the options is 
(accidentally) changed half-way through the run.

Jorn

Maurizio Cimadamore schreef op 2019-02-26 16:42:
> I'd like to hold back on this a bit, and look at the bigger picture.
> Again, I'm leaning on my experience with javac (and related tools) -
> sorry if what follows sounds too paternalistic :-)
> 
> To me, it seems that jextract Context is a mix of different things -
> on the one hand it acts as an holder for _parsed_ options. On the
> other hand it also acts as an helper to logging activity (since it
> encapsulates the err/out streams).
> 
> The logical consequence, to me, is that we should have *two* separate
> abstractions, one for managing parsed option values, and one for
> managing logging.
> 
> Which brings another question - how do we share 'components' instances
> between various pieces of jextract? I think that is, ultimately what a
> 'Context' should be: a collection of instances that can be reused
> across components. This is at least the notion of context we use in
> javac, javap, javadoc and related tools.
> 
> If Context acts as a sort of 'store' for all the component instances,
> then you can have all components referring to each other, on
> constructon, via a context object. Javac does this routinely - see
> this example:
> 
> http://hg.openjdk.java.net/valhalla/valhalla9/langtools/file/tip/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassWriter.java#l154
> 
> If you have this machinery in place then you end up in a place where:
> 
> * Context contains the 'shared' components that pertain to an 
> extraction run
> 
> * Log (or something like it) is a component that can be used for
> issuing error/warnings/notes
> 
> * Options is an helper component which allows clients to query options
> values in Java directly
> 
> * JextractTool is the main component that is in charge of executing
> the extraction - separate visitors which are part of the extraction
> logic could (or not!) be made components themselves
> 
> So, a jextract run (in Main) will create a Context, and start
> initializing other needed instances (Options, JextractTool), and then
> start the extraction run.
> 
> 
> This way of designing a tool has proven to be very resilient; in javac
> you can even 'override' standard components with custom ones (say, to
> have custom type-checking logic), which is a feature that has been
> used extensively by 3rd part checkers such as the ones offered by
> IDEs. I'm not saying that in jextract we need to go as far - but,
> especially if we're considering exposing a programmatic API to do
> jextract run (which would be a crucial tool to get to most of the
> civilization-related cases), it is something to keep in the back of
> our minds.
> 
> 
> Maurizio
> 
> 
> On 26/02/2019 13:50, Jorn Vernee wrote:
>> Hi,
>> 
>> This is an RFR for the earlier patch that makes Context immutable:
>> http://cr.openjdk.java.net/~jvernee/panama/webrevs/context/webrev.00/
>> 
>> To recap, this patch:
>> 
>> * Makes all fields of Context final, and wraps collections in 
>> immutable wrappers.
>> 
>> * Adds a Context.Builder to create an immutable Context.
>> 
>> * Rearranges code in Main to use the Context.Builder.
>> 
>> Thanks,
>> Jorn