Kotlin & method overloading & name mangling, oh my!

[jonpryor]

Context: #505

PR #505 improves support for binding certain Kotlin constructs.

In "the perfect is the enemy of the good" manner, we will be merging PR #505 even though there's a known issue with it, in part because we don't currently know how to fix this scenario:

Method overloads and "name mangling".

In various situations, Kotlin will emit member names which are not valid Java identifiers. For example, consider this Kotlin class, which overloads the ExampleBase.foo() method to accept both Int and UInt parameters:

package example;

public open class ExampleBase {
	public open fun foo(value : Int) {
	}
	public open fun foo(value : UInt) {
	}
}

The use of a UInt parameter type causes Kotlin to "name mangle" the method name, generating a method name which is not a valid Java identifier:

$ kotlinc hello.kt
$ javap -cp . example.ExampleBase
Compiled from "hello.kt"
public class example.ExampleBase {
  public void foo(int);
  public void foo-WZ4Q5Ns(int);
  public example.ExampleBase();
}

With PR #505 we can create an API description:

<api
  api-source="class-parse">
  <package
    name="example"
    jni-name="example">
    <class
      abstract="false"
      deprecated="not deprecated"
      jni-extends="Ljava/lang/Object;"
      extends="java.lang.Object"
      extends-generic-aware="java.lang.Object"
      final="false"
      name="ExampleBase"
      jni-signature="Lexample/ExampleBase;"
      source-file-name="hello.kt"
      static="false"
      visibility="public">
      <constructor
        deprecated="not deprecated"
        final="false"
        name="ExampleBase"
        static="false"
        visibility="public"
        bridge="false"
        synthetic="false"
        jni-signature="()V" />
      <method
        abstract="false"
        deprecated="not deprecated"
        final="false"
        name="foo"
        native="false"
        return="void"
        jni-return="V"
        static="false"
        synchronized="false"
        visibility="public"
        bridge="false"
        synthetic="false"
        jni-signature="(I)V">
        <parameter
          name="value"
          type="int"
          jni-type="I" />
      </method>
      <method
        abstract="false"
        deprecated="not deprecated"
        final="false"
        name="foo-WZ4Q5Ns"
        native="false"
        return="void"
        jni-return="V"
        static="false"
        synchronized="false"
        visibility="public"
        bridge="false"
        synthetic="false"
        jni-signature="(I)V">
        <parameter
          name="value"
          type="int"
          jni-type="I" />
      </method>
    </class>
  </package>
</api>

The problem comes when we attempt to generate C# source to bind the above API description. It cannot compile, because ExampleBase.Foo(int) is emitted twice:

namespace Example {
	[global::Android.Runtime.Register ("example/ExampleBase", DoNotGenerateAcw=true)]
	public partial class ExampleBase : Java.Lang.Object {

		[Register ("foo", "(I)V", "GetFoo_IHandler")]
		public virtual unsafe void Foo (int value)
		{
			const string __id = "foo.(I)V";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue (value);
				_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
			} finally {
			}
		}

		[Register ("foo-WZ4Q5Ns", "(I)V", "GetFoo_IHandler")]
		public virtual unsafe void Foo (int value)
		{
			const string __id = "foo-WZ4Q5Ns.(I)V";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue (value);
				_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
			} finally {
			}
		}
	}
}

Attempting to compile the above will cause a CS0111 error to be generated:

src/Example.ExampleBase.cs(98,30): error CS0111: Type 'ExampleBase' already defines a member called 'Foo' with the same parameter types

This can be fixed with Metadata.xml, to "rename" one of the members. (This isn't necessarily ideal, but it's possible.)

<!-- Untested, but go with it for expository purposes -->
<attr path="/api/package[@name='example']/class[@name='ExampleBase']/method[@name='foo-WZ4Q5Ns']" name="managedName">Foo2</attr>

Were we to do so, we'll find that subclassing won't work:

class MyExample : Example.ExampleBase {
    public override void Foo2 (int value) {}
}

Subclassing cannot work, because we use Java as an intermediary, and the Java intermediary -- as generated by src/Java.Interop.Tools.JavaCallableWrappers -- uses the [Register] attribute, which will still specify foo-WZ4Q5Ns. We would thus (presumably; untested) get a Java Callable Wrapper for MyExample of:

/* partial */ class MyExample extends example.ExampleBase /* ... */ {
    public void foo-WZ4Q5Ns(int value) {
        n_foo-WZ4Q5Ns (value);
    }
    native void n_foo-WZ4Q5Ns (int value);
}

This cannot compile.

How do we address this?

[jonpryor]

Solution 1: We could just remove them from the API binding entirely. This would give us the "same" view as Java does of a Kotlin class.

[jonpryor]

Solution 2: We could bind them as non-virtual methods.

We cannot bind "name mangled" methods as C# virtual methods, because there's no way to make the subclass scenario work as long as we're using Java as an intermediate language.

(We could fix this by using Kotlin as the intermediate language, or by emitting Java byte code directly.)

If we bind the method as a non-virtual method, we can expose the method to C# code, while preventing the subclass scenario. This allows the Kotlin ExampleBase.foo(UInt) method to be invokable.

(We'd still need to know how to name the method so that we avoid the CS0111 errors.)

[jonpryor]

Solution 2(b): Update class-parse & generator so that generator can "know" that the method takes a UInt. This would allow us to bind foo(UInt) as Foo(uint), which can be overloaded, thus avoiding the CS0111 error:

namespace Example {
	public partial class ExampleBase : Java.Lang.Object {
		[Register ("foo", "(I)V", "GetFoo_IHandler")]
		public virtual unsafe void Foo (int value) {...}

		[Register ("foo-WZ4Q5Ns", "(I)V", "GetFoo_IHandler")]
		public unsafe void Foo (uint value) {...}
	}
}

[mattleibow]

I am thinking that Solution 2(b) is actually the most correct solution - we are binding Kotlin here, and it just so happens to use the same IL as Java. All the metadata ins there, so the generator logic is more correct to check for Kotlin data, then the Java data.

[jonpryor]

Aside (via offline discussion): if we go with 2(b), we'll need to add JniArgumentValue constructor overloads for the unsigned types, so that the generated code can compile:

		[Register ("foo-WZ4Q5Ns", "(I)V", "GetFoo_IHandler")]
		public unsafe void Foo (uint value)
		{
			const string __id = "foo-WZ4Q5Ns.(I)V";
			try {
				JniArgumentValue* __args = stackalloc JniArgumentValue [1];
				__args [0] = new JniArgumentValue (value);  // hits JniArgumentValue(uint) constructor
				_members.InstanceMethods.InvokeVirtualVoidMethod (__id, this, __args);
			} finally {
			}

Alternatively, we could insert casts:

__args [0] = new JniArgumentValue (unchecked((int) value));

[mattleibow]

It might be better to add constructor overloads so that any processing is done in the core android as opposed to each library. But then this might cause issues if an old android is used and it does not have these new constructors.

And, I don't think Java is adding new types to their IL any time soon, so potentially breaking for older Android might not be worth it.

Wow, look at my brain wandering. Unless I am wrong, we can't use new constructors as these new libraries won't work on a slightly older Android?

[jonpryor]

we can't use new constructors as these new libraries won't work on a slightly older Android?

It took me some thinking to quite understand where you're going, but I think I have it now:

The problem isn't "an old Android version".

The problem is "an old Xamarin.Android version": binding assemblies built against a hypothetical newer Xamarin.Android containing a JniArgumentValue(uint) constructor -- and using that constructor -- would not be usable on all previous versions of Xamarin.Android.

That's a hell of a compat problem.

That reason alone is enough to kill the JniArgumentValue(uint) constructor overload idea, and necessitate the insertion of casts into the generated source code.

[jonpryor]

Open Questions

What happens with UIntArray?

It looks like UIntArray becomes an int[]:

package example;

open class UnsignedMethods {
	public open fun uintArray(value: UIntArray) {
	}
}

$ javap -c -s -cp . example.UnsignedMethods
Compiled from "hello.kt"
public class example.UnsignedMethods {
  public void uintArray--ajY-9A(int[]);
    descriptor: ([I)V
    Code:
       0: aload_1
       1: ldc           #30                 // String value
       3: invokestatic  #36                 // Method kotlin/jvm/internal/Intrinsics.checkParameterIsNotNull:(Ljava/lang/Object;Ljava/lang/String;)V
       6: return
}

[mattleibow]

It took me some thinking to quite understand where you're going

Sometimes I don't use my words 😑