[foreign-jextract] RFR: 8240300: jextract produces non compilable code with repeated declarations

[Athijegannathan Sundararajan]

On Mon, 2 Mar 2020 18:45:12 GMT, Henry Jen <henryjen at openjdk.org> wrote:

>> Filtering repeated global variables and functions in code generator (added necessary hashCode, equals for decls, types)
>
> src/jdk.incubator.jextract/share/classes/jdk/incubator/jextract/tool/StaticWrapperSourceFactory.java line 102:
> 
>> 101:     public Void visitVariable(Declaration.Variable tree, Declaration parent) {
>> 102:         if (parent == null && !(variables.add(tree))) {
>> 103:             return null;
> 
> Why check parent == null? Isn't that the case for global variable or you have top-level for header file as parent?

Yes. Struct fields will result in the same method being called (please check parent check in the same method). We just want to check globals for repeated decls. Note that struct field re-declaration will be prevented by compiler anyway (cannot have two fields of same name in the same struct)

> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/DeclarationImpl.java line 200:
> 
>> 199:             FunctionImpl function = (FunctionImpl) o;
>> 200:             return params.equals(function.params) &&
>> 201:                     type.equals(function.type);
> 
> Type would have covered both parameter type and return type. Compare params depends on how Variable implements equals and may cause same type declaration with different argument name to fail(not the case here).

Agree. Function parameters should not be compared. Only type and (function) name should be. Will fix this and expand test to cover this case (void func(int x) and void func(int y) as duplicate declarations)

> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/DeclarationImpl.java line 198:
> 
>> 197:             if (this == o) return true;
>> 198:             if (o == null || getClass() != o.getClass()) return false;
>> 199:             FunctionImpl function = (FunctionImpl) o;
> 
> I am not sure if we should force same class or check at the interface level by only using interface APIs. Although we only have one implementation class now and perhaps will remain so in foreseeable future. Same applies to other classes.

Going to add equals and hashCode to Declaration and Type as suggested by Maurizio. Will fix equality of type with instanced check.

-------------

PR: https://git.openjdk.java.net/panama-foreign/pull/37