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

[Sundararajan Athijegannathan]

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

Removed JtregJextract driver use in Srcgentest.java.

-Sundar

On 20/03/19, 8:29 PM, Sundararajan Athijegannathan wrote:
> 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