[foreign] RFR:8223105

[Henry Jen]

> On May 2, 2019, at 3:27 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> Overall looks good!
> 
> Curious: why were all decls in DuplicateDeclarationHandler changed from List<XYZ> to ArrayList<XYZ> ?

I tried to save the order of encounter and use set when duplicate is encountered again. Don’t think that matters though.

> 
> JavaSourceFactory: I'm not super convinced of the logic here - it seems like in some cases (e.g. functions) we build a layout with the right 'name' annotation on it and then we lean on that layout to extract the name info. In other cases, e.g. global variables, we leave the layout as is, and we then have to tweak it on the fly while generating the code. I believe it would be better/more uniform if the 'name' annotation would be injected on all layouts when the tree is created? If you go down that path, I believe that would make the addition of the VarTree::label method useless?
> 
> FunctionTree:
> 
> return label.isPresent() ? fn.withAnnotation("name", label.get()) : fn;
> 
> 
> 
> This code could use the static NAME field in Layout.
> 

I noticed that inconsistency, but because the static name field of VarTree is use for the accessor function name, we cannot use that field. But we can put in the name annotation of layout. The reason I didn’t go with that is because I am not sure if current behavior(layout without name annotation) is for a reason.

Cheers,
Henry

> Maurizio
> 
> 
> On 01/05/2019 20:43, Henry Jen wrote:
>> Hi,
>> 
>> Please review a webrev[1] for JDK-8223105[2]. This is an gcc extension[3](also supported by clang) and likely observed in system headers.
>> I don’t find same feature for Windows, so I cannot have the test case for Windows.
>> 
>> A different approach to enable alias is used for Windows(and is a portable way) using macro, but this is not the same and generated Java interface will have real name for it. Perhaps that can be a separate RFE to add alias when a macro definition is a single identifier match to a variable or function.
>> 
>> Cheers,
>> Henry
>> 
>>  
>> [1] 
>> http://cr.openjdk.java.net/~henryjen/panama/8223105/0/webrev/
>> 
>> [2] 
>> https://bugs.openjdk.java.net/browse/JDK-8223105
>> 
>> [3] 
>> https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Asm-Labels.html