dotnet / java-interop

Java.Interop provides open-source bindings of Java's Java Native Interface (JNI) for use with .NET managed languages such as C#
Other
201 stars 52 forks source link

`generator` Symbol Table semantics #1268

Open jonpryor opened 15 hours ago

jonpryor commented 15 hours ago

Context? https://github.com/dotnet/java-interop/pull/1266 Context: https://github.com/dotnet/java-interop/pull/1267#issuecomment-2427499866

Currently, generators Symbol Table uses Java names as the keys:

https://github.com/dotnet/java-interop/blob/23e9e0487ad952e12d136b2225797d2941e00eca/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Symbols/SymbolTable.cs#L59

Keys will be strings such as Java builtin types like boolean and int, and "fully qualified dotted" Java names such as java.lang.Object and android.app.Activity.

There are (at least?) two issues with this setup:

  1. Nested type semantics
  2. Potential ambiguity with JNI names

For nested type semantics, consider dotnet/java-interop#1267:

[Register ("com/mypackage/FieldClass")]
public class FieldClass : Java.Lang.Object {
    public NestedFieldClass field;

    public class NestedFieldClass : Java.Lang.Object {
    }
}

The above could be a hand-written C# snippet added to a binding assembly, and then referenced by generator when emitting a new set of bindings.

The (long-standing so not necessarily a "real") concern is that when generator processes the above, the symbol table will have:

This nested name is wrong; it should instead be one of:

Then there's #1266: somehow -- we're not quite sure we fully understand yet -- JniTypeSignatureAttribute.SimpleReference is being used as a key for the Symbol Table.

Given:

https://github.com/dotnet/java-interop/blob/23e9e0487ad952e12d136b2225797d2941e00eca/src/Java.Interop/Java.Interop/JavaPrimitiveArrays.cs#L349-L350

and a Java type:

// Java
public abstract /* partial */ class androidx.work.WorkRequest.Builder<
    B extends androidx.work.WorkRequest$Builder<B, ?>,
    W extends androidx.work.WorkRequest>
{
  public final B setId(java.util.UUID);
}

then the return value of WorkRequest.Builder.setId() is being detected as JavaSByteArray instead of WorkRequest.Builder.

Aside: how?!

https://github.com/dotnet/java-interop/blob/23e9e0487ad952e12d136b2225797d2941e00eca/src/Java.Interop.Tools.JavaCallableWrappers/Java.Interop.Tools.JavaCallableWrappers/CecilExtensions.cs#L75-L96

certainly looks like [JniTypeSignature("B", ArrayRank=1, IsKeyword=true)] should result in a Register attribute with a name of [B. How/where would this value be "cleaned up"?

The scenario described in #1266 should not happen. An idea to do that would be to alter the symbol table keys to instead be JNI type signatures. B would thus only be "byte", while LB; would be a class B{} in the global package, and Ljava/lang/Object; would be java.lang.Object.

jpobst commented 8 hours ago

Aside: how?!

I think we're conflating different processes here that have different codebases. The linked CecilExtensions.cs code is used for jcw-gen, but not for generator. generator simply looks at the first constructor argument for [Register] and uses it:

https://github.com/dotnet/java-interop/blob/2a1e180086889a33c844e337a865093ab65193e7/tools/generator/Java.Interop.Tools.Generator.Importers/CecilApiImporter.cs?plain=1#L118-L124

Thus something like [JniTypeSignature] is not going to work in generator without modification, as it uses the first constructor parameter unmodified which is B:

[JniTypeSignature ("B", ArrayRank=1, IsKeyword=true, GenerateJavaPeer=false)] 

Note for generator purposes we would not want a [JniTypeSignature] with byte to exist either, as byte is a built-in type and no class should be trying to register itself as byte:

// Also incorrect
[JniTypeSignature ("byte", ArrayRank=1, IsKeyword=true, GenerateJavaPeer=false)] 

https://github.com/dotnet/java-interop/blob/2a1e180086889a33c844e337a865093ab65193e7/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Symbols/SymbolTable.cs#L61


There are (at least?) two issues with this setup:

  1. Potential ambiguity with JNI names

I do not think this one is an issue. The generator symbol table is all Java names, all the time. If a JNI name key is added to the table or the table is queried for a JNI name, then the table is being used incorrectly and the caller needs to be fixed. (#1266 fixed a case of this.)

We do have a bug here that the symbol table also should not be queried for a generic type parameter name like B, but that tends to just get swept under the "we don't do generics well" rug.