[foreign] RFR 8212560 : jextract should generate a static forwarder class

[Samuel Audet]

That looks good, thanks Sundar! Though, eventually, it would be great to 
have those generated at runtime somehow, so that we don't need to start 
parsing stuff when we just need to call single functions, like we can do 
with JNA or JavaCPP, for example:
https://gist.github.com/saudet/1bf14a000e64c245675cf5d4e9ad6e69#file-nativebenchmark-java

Samuel

On 10/18/2018 02:35 AM, Sundararajan Athijegannathan wrote:
> Updated: http://cr.openjdk.java.net/~sundar/8212560/webrev.02/index.html
> 
> Added --static-forwarder boolean option (default true)
> 
> -Sundar
> 
> On 17/10/18, 9:55 PM, Sundararajan Athijegannathan wrote:
>>
>> comments below..
>>
>> On 17/10/18, 9:13 PM, Jorn Vernee wrote:
>>> Sundararajan Athijegannathan schreef op 2018-10-17 14:37:
>>>> On option: do we need an explicit option? given that one file extra
>>>> and that too only when -l is specified. (too many options already? ;)
>>>> )
>>>
>>> Maybe having an option is nice, but have it be turned on by default? 
>>> That way if someone really wanted to not generate the forwarder they 
>>> could, and for the general use case they don't have to bother with 
>>> setting an option every time.
>>>
>> okay.
>>
>>>> On naming: we've derived automatic names elsewhere. I think we could
>>>> revisit all naming options by another round - perhaps even java
>>>> convention for header interface name.
>>>>
>>>> On subclassing: Nice suggestion. I made it as AsmCodeFactoryExt class
>>>> and extended it from AsmCodeFactory. visit methods now return Boolean
>>>> to tell if the particular tree was handled or not. This is needed so
>>>> that subclass can avoid the same tree if super class avoided it (for
>>>> eg. function-like macros, repeated definitions..)
>>>
>>> I'm wondering if this runs into trouble later on when adding more 
>>> code generation extensions/options. Let's say you add `foo` and `bar` 
>>> options later on. You'd have to make an extension subclass for every 
>>> combination of options (StaticFooBar, StaticFoo, StaticBar, FooBar, 
>>> Static, Foo, Bar).
>>>
>>> Maybe you could have something like a `CodeFactory`, which consists 
>>> of an `AsmCodeFactory` and a list of `CodeFactoryExt`, of which the 
>>> static forwarder generator would be one. And have this kind of pattern:
>>>
>>> ```
>>> @Override
>>> public Void visitVar(VarTree varTree, JType jt) {
>>>     if(asmCodeFactory.visitVar(varTree, jt)) {
>>>         for(CodeFactoryExt cfe : extensions) {
>>>             cfe.visitVar(varTree, jt);
>>>         }
>>>     }
>>>     return null;
>>> }
>>> ```
>>>
>>> I think that also allows you to get rid of the if/else statements in 
>>> the extension class.
>>>
>>> It's not really needed right now since there is only 1 extension, but 
>>> it might be nice idea for the future.
>>
>> Right. We don't want to go there yet - but yes, if we happened to add 
>> one more, we can revisit & refactor the code.
>>
>> -Sundar
>>>
>>> Jorn
>>>
>>>> Updated: 
>>>> http://cr.openjdk.java.net/~sundar/8212560/webrev.01/index.html
>>>>
>>>> Thanks,
>>>> -Sundar
>>>>
>>>> On 17/10/18, 3:25 PM, Maurizio Cimadamore wrote:
>>>>> I like it! Few questions/comments:
>>>>>
>>>>> * should it be enabled/disabled with explicit option
>>>>> * should the name of the statics class be customizable
>>>>> * I like the code organization - have you thought of pushing it 
>>>>> further and make StaticForwarderGenerator a _subclass_ of 
>>>>> AsmCodeFactory - each visitor could maybe delegate to 
>>>>> super.visitXyz() first and then do its own bit? Then when you setup 
>>>>> the pipeline, depending on what info is available on the context 
>>>>> (e.g. presence of -l) you can decide whether to use a 'bare' 
>>>>> ASMCodeFactory or the 'embellished' one. That should remove all the 
>>>>> 'if staticFwd != null ...' sections from AsmCodeFactory.
>>>>>
>>>>> Maurizio
>>>>>
>>>>>
>>>>> On 17/10/2018 06:35, Sundararajan Athijegannathan wrote:
>>>>>> Please review.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8212560
>>>>>> Webrev: 
>>>>>> http://cr.openjdk.java.net/~sundar/8212560/webrev.00/index.html
>>>>>>
>>>>>> Thanks,
>>>>>> -Sundar