[foreign] RFR: Make jextract Context immutable

[Maurizio Cimadamore]

On 27/02/2019 15:05, Jorn Vernee wrote:
>> 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.

Ok - that works for me, obviously

Maurizio

>
> 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