[foreign] RFR 8221154: jextract should generate java source files

[Sundararajan Athijegannathan]

Updated: https://cr.openjdk.java.net/~sundar/8221154/webrev.01/

Changes:

* Refactored JavaSourceBuilder.addMethod as you suggested.

* yes, getSourceSignature(true) should have been used for arguments in 
addMethod. Thanks for catching! Fixed it.

* On AsmCodeFactory/Ext duplication: yes, will get rid of ASM based 
generation in a future patch

* Added basic test to dump sources and compile the same using javac 
ToolProvider. That uncovered an issue with NativeStruct annotation :)

I've not modified JType's import interaction yet. I'll address that in a 
future patch - perhaps when removing ASM dependency.

Thanks,
-Sundar

On 20/03/19, 4:39 PM, Maurizio Cimadamore wrote:
> Hit send too fast; what about smoke-testing the feature by comparing 
> the output against a golden source file?
>
> Maurizio
>
> On 20/03/2019 11:08, Maurizio Cimadamore wrote:
>> Hi,
>> looks a solid piece of work! Comments:
>>
>> 1) In JType there seems to be a dependency on what the 
>> sourceSignature method generates and the set of imports generated by 
>> the source factory. I wonder if we could make the dependency explicit 
>> by adding another virtual method to JType which lists the packages it 
>> depends on (so that the source factory can collect them all in a set, 
>> and add an import at the end).
>>
>> 2) In JavaSourceBuilder::addMethod (both variant) shouldn't we use 
>> getSourceSignature(TRUE) for the parameter types?
>>
>> 3) Seems like there's a lot of duplication between the two 
>> JavaSourceBuilder::addMethod. The main differences seem to be where 
>> the function name comes from and the fact that the tree-based version 
>> also has (optional) parameter names; maybe we can rebase both methods 
>> on top of a third version which is more low level?
>>
>> The rest looks good; there's obvious duplication with 
>> AsmCodeFactory/Ext, but I think that's mostly by design (e.g. you 
>> started from those builders and replaced ASM calls with SourceBuilder 
>> calls, which is a fine approach).
>>
>> Maurizio
>>
>> On 20/03/2019 10:29, Sundararajan Athijegannathan wrote:
>>> Please review.
>>>
>>> This is initial change for moving jextract towards source 
>>> generation. The option --src-dump-dir will most likely be 
>>> changed/removed later. It is a development/debug option for now. 
>>> Reasons for source instead of direct .class generation are 
>>> documented in the bug report.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8221154
>>> Webrev: https://cr.openjdk.java.net/~sundar/8221154/webrev.00/
>>>
>>> Thanks,
>>> -Sundar