[foreign-jextract] RFR: 8261642: improve jextract source generation mode and remove class generation [v2]

[Maurizio Cimadamore]

On Mon, 15 Feb 2021 12:09:51 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove template for @C annotation
>>   Remove debugging/parameter info from jextract compilation
>
> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/ConstantBuilder.java line 178:
> 
>> 176:         Constant constant = namesGenerated.get(lookupName);
>> 177:         if (constant == null) {
>> 178:             constant = constantFactory.get();
> 
> Maybe a check or assert could be put here to check if the created constant has the expected `kind`? (Just to be sure)

good idea, will do

> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/ConstantBuilder.java line 267:
> 
>> 265:             append(")");
>> 266:         } else if (l instanceof GroupLayout) {
>> 267:             if (((GroupLayout) l).isStruct()) {
> 
> Good place to use `instanceof` pattern matching.

true

> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/JavaSourceBuilder.java line 270:
> 
>> 268:             }
>> 269:             constant_counter = 0;
>> 270:             constantBuilder = new ConstantBuilder(this, Kind.CLASS, javaName + "_constants");
> 
> Using the java name of some constant to uniqueify the name of the constant class seems a bit strange to me... Couldn't there be collisions? (e.g. if struct and unction are named the same?) Why not just use an id/number here like before?

there could be collisions, yes - not sure I get what you propose by using "id/number"

> src/jdk.incubator.jextract/share/classes/jdk/internal/jextract/impl/StructBuilder.java line 57:
> 
>> 55:         this.structType = structType;
>> 56:         prefixElementNames = new ArrayDeque<>();
>> 57:         classBegin();
> 
> Kinda strange that the constructor here calls `classBegin()` but OutputFactory calls `classEnd()`. Could both calls be moved to `OutputFactory`?

I can fix this

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

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