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

[Samuel Audet]

And while we're at it, I think the runtime component could also take 
care of abstracting away the details of the layouts, so gettimeofday() 
could look something like this, again without having to parse anything 
for simple cases:

     @Struct
     interface Timeval {
         @SizeT long tv_sec();
         Timeval tv_sec(long tv_sec);

         long tv_usec();
         Timeval tv_usec(long tv_usec);
     }

     interface Dummy {
         int gettimeofday(Timeval tv, Pointer timezone);
     }

     import static Dummy.*;

     public static void main(String[] args) {
	InitMagicBindings(Dummy.class, Timeval.class);
         Timeval t = Timeval.create()
	gettimeofday(t, null);
     }
}

Now that would be awesome :)

Samuel

On 10/18/2018 09:16 AM, Samuel Audet wrote:
> 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
>