[foreign] RFR:8223105

[Maurizio Cimadamore]

Overall looks good!

Curious: why were all decls in DuplicateDeclarationHandler changed from 
List<XYZ> to ArrayList<XYZ> ?

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.

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