Support Kotlin's unsigned types #539

jpobst · 2019-12-12T17:04:34Z

This adds support for Kotlin's unsigned types (uint, ushort, ulong, ubyte). In order to do this, we pretend that they are native Java types and just translate a few places where we need to tell Java the real type.

Class Parse

When we read the Kotlin metadata in the Java bytecode, if we come across one of these types we store it as an additional KotlinType that we can use later. When we are generating the xml we check this additional flag and if it's one of our types we emit it instead of the native Java type. For example:

<method
  abstract="false"
  deprecated="not deprecated"
  final="false"
  name="unsignedAbstractMethod-WZ4Q5Ns"
  native="false"
  return="uint"
  jni-return="I"
  static="false"
  synchronized="false"
  visibility="public"
  bridge="false"
  synthetic="false"
  jni-signature="(I)I">
  <parameter
    name="value"
    type="uint"
    jni-type="I" />
</method>

ApiXmlAdjuster

Update JavaTypeReference to contain unsigned types:

UInt = new JavaTypeReference ("uint");
UShort = new JavaTypeReference ("ushort");
ULong = new JavaTypeReference ("ulong");
UByte = new JavaTypeReference ("ubyte");

Generator

generator has the 4 new types added to the SymbolTable as SimpleSymbols:

AddType (new SimpleSymbol ("0", "uint", "uint", "I", returnCast: "(uint)"));
AddType (new SimpleSymbol ("0", "ushort", "ushort", "S", returnCast: "(ushort)"));
AddType (new SimpleSymbol ("0", "ulong", "ulong", "J", returnCast: "(ulong)"));
AddType (new SimpleSymbol ("0", "ubyte", "byte", "B", returnCast: "(byte)"));

There are 2 fixups we have to make because we use GetIntValue (...), etc. instead of having unsigned versions:

Override name of which method to call, ie: GetIntValue instead of GetUintValue. (link)
Cast the int value returned to uint. This is accomplished via the new ReturnCast property on ISymbol. (link)

jonpryor · 2019-12-20T18:03:33Z

Incredibly important meta-question: given that Kotlin unsigned types are EXPERIMENTAL, should we special-case things at all at this time? Or should we wait for Kotlin to stabilize things?

jonpryor · 2019-12-20T18:08:43Z

src/Xamarin.Android.Tools.Bytecode/Kotlin/KotlinFixups.cs

+			// Handle erasure of Kotlin unsigned types
+			if (jvmType == "I" && kotlinClass == "kotlin/UInt;")
+				return "uint";
+			if (jvmType == "[I" && kotlinClass == "kotlin/UIntArray;")


From the "obvious question is obvious" department -- transcending into the "oh god what horrors does this provoke?!" department -- is it possible to be given a [[I or [[[I or...?

There is no kotlin.UIntArrayArray type. However, we "could" do:

public open fun uintArray(value: Array<UIntArray>) { }

...which isn't actually special-cased by the compiler. Meaning it "leaks", with javap showing!

public void uintArray(kotlin.UIntArray[]);

WTF is a kotlin.UIntArray type? (It's an actual type!)

So in some contexts, UIntArray becomes a JVM int[], and in other contexts it remains a UIntArray.

I'm still not sure how to mentally process this. I'm not sure what this should mean wrt binding at all.

jonpryor · 2019-12-20T18:11:44Z

...Tests/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypeMethodsClass.txt

@@ -0,0 +1,90 @@
+// Metadata.xml XPath class reference: path="/api/package[@name='java.code']/class[@name='MyClass']"


Why is this a .txt file? It means we don't get C# highlighting on GitHub. :-(

...because of SDK-style (short-form) .csproj project usage. If these were .cs files, they'd be auto-included into the .csproj build, which is not what we want.

jonpryor · 2019-12-20T18:28:53Z

In part because of the "can we have a [[I pondering above, I'm less certain of the wisdom of tacking "unsigned types as unsigned types" at all.

Which puts things in a quandary.

The problem is this: suppose someone provides this Kotlin type:

package example;

public interface IAbstractUnsigned {
	public abstract fun m(value : UInt)
}

Corresponding Java declaration:

$ javap -cp . example/IAbstractUnsigned
public interface example.IAbstractUnsigned {
  public abstract void m-WZ4Q5Ns(int);
}

What do we do? What can we do?

See also: #525

We can bind this type:

namespace Example {
    public partial interface IAbstractUnsigned {
        [Register ("m-WZ4Q5Ns", "(I)V", "...")]
        void M(int value);  // ignoring int vs. uint for now..
    }
    internal partial class IAbstractUnsigned : Java.Lang.Object, IAbstractUnsigned {
        public void M(int value) {
            _members.InstanceMethods.InvokeAbstractObjectMethod ("(I)V", this, ...)
        }
    }
}

However, any binding cannot be implemented, so long as we use Java source code as an intermediary:

public class MyExample : Java.Lang.Object, Example.IAbstractUnsigned {
    public void M(int value) {}
}

This C# source will compile, but we cannot emit a Java Callable Wrapper for it.

We considered this to be acceptable, as the alternative was to invalidate the entire interface. At least with the above, if the Java library provides types which implement the interface or has methods which return the interface, those types & methods are usable.

Abstract types are a similar scenario: if the type contains only the abstract methods, if we removed the "invalid" abstract methods from the binding, subclasses would fail to generate JCWs in non-obvious ways.

Related: f3553f4

The problem is that use of unsigned types always introduces name mangling, leaving the question: how do we deal with unsigned types?

I'm less certain of the answer there. :-(

jonpryor · 2019-12-20T18:35:34Z

...ts/CodeGeneratorExpectedResults/JavaInterop1/WriteKotlinUnsignedArrayTypePropertiesClass.txt

+
+	protected MyClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}
+
+	public unsafe uint[] UIntProp {


This is wrong: we should never map "property-looking" methods to actual properties when the return type is an array. See e.g.

https://github.com/xamarin/java.interop/blob/d61b329cbdeaa6d42c17618de59cb850ab82ce93/tests/generator-Tests/expected/EnumerationFixup/Xamarin.Test.SomeObject2.cs#L116

https://github.com/xamarin/java.interop/blob/d61b329cbdeaa6d42c17618de59cb850ab82ce93/tools/generator/Java.Interop.Tools.Generator.ObjectModel/Method.cs#L77

Looks like unsigned types are being treated differently from signed types, such that RetVal.IsArray is returning false for this array type.

Yeah, this test is actually irrelevant because I found out you cannot create a Kotlin property with an array type, but I definitely need to dig in and figure out what's going on with IsArray returning false. 👍

Ah, IsArray is returning true like it should, this unit test just doesn't go through that logic. It is explicitly creating properties with array types rather than combining methods into a property.

https://github.com/xamarin/java.interop/blob/dd8a6a09a480700583b70976a71f0e6dd43ae892/tests/generator-Tests/Unit-Tests/CodeGeneratorTests.cs#L85

jpobst · 2019-12-20T19:29:44Z

Incredibly important meta-question: given that Kotlin unsigned types are EXPERIMENTAL, should we special-case things at all at this time? Or should we wait for Kotlin to stabilize things?

I think the issue we face is that we need to bind kotlin-stdlib.jar so that people can bind their libraries, and the stdlib already contains unsigned types.

If we leave them as-is and then attempt to fix it later once Kotlin considers them "stable" that's a break.
If we translate now and Kotlin changes them it's a break, but it's Kotlin's fault.
We could use metadata to remove them from the stdlib binding, preventing users from using them for now.

It looks like they were introduced as experimental in Kotlin 1.3 in Oct 2018 and they still exist, so my gut says they're probably not going to change at this point, but it is definitely a risk.

jpobst

The problem is that use of unsigned types always introduces name mangling, leaving the question: how do we deal with unsigned types?

I feel like calling methods is probably ~95% of usage compared to overriding/implementing them but that's just a wild guess. We do technically have a (future) solution to this in the form of #535. I think the best we can do is use the warning in f3553f4 and public feedback to prioritize this future enhancement as needed.

…[]" so we do not do any Kotlin fixups on it.

jpobst · 2020-01-06T22:53:39Z

We decided offline that Kotlin's unsigned types are probably pretty stable by now and are used by the Kotlin stdlib, so we will go ahead and commit this work. If Kotlin does change this feature in the future we will update and blame any breakage on them. 😁

Fixes: #525 Context: dotnet/android#4054 Context: https://github.com/Kotlin/KEEP/blob/13b67668ccc5b4741ecc37d0dd050fd77227c035/proposals/unsigned-types.md Context: https://kotlinlang.org/docs/reference/basic-types.html#unsigned-integers Another place where Kotlin makes use of "name mangling" -- see also commit f3553f4 -- is in the use of unsigned types such as `UInt`. At the JVM ABI level, Kotlin treats unsigned types as their signed counterparts, e.g. `kotlin.UInt` is an `int` and `kotlin.UIntArray` is an `int[]`: // Kotlin public class Example { public fun value(value: UInt) : UInt { return value } public fun array(value: UIntArray) : UIntArray { return value } } // `javap` output: public final class Example { public final int value-WZ4Q5Ns(int); public final int[] array--ajY-9A(int[]); } Kotlin uses Java Annotations to determine whether a parameter or return type is actually an unsigned type instead of a signed type. Update `Xamarin.Android.Tools.Bytecode` and `generator` to bind e.g.: * `kotlin.UInt` as `System.UInt32` * `kotlin.UIntArray` as a `System.UInt32[]` and likewise for the other unsigned types `ushort`, `ulong`, `ubyte`. In order to do this, we pretend that they are native Java types and just translate a few places where we need to tell Java the real type. ~~ Xamarin.Android.Tools.Bytecode / class-parse ~~ When we read the Kotlin metadata in the Java bytecode, if we come across one of these types we store it within an additional `FieldInfo.KotlinType` property that we can access later. When we are generating the XML we check this additional flag and if it's one of our types we emit it instead of the native Java type. For example: <method abstract="false" deprecated="not deprecated" final="false" name="unsignedAbstractMethod-WZ4Q5Ns" native="false" return="uint" jni-return="I" static="false" synchronized="false" visibility="public" bridge="false" synthetic="false" jni-signature="(I)I"> <parameter name="value" type="uint" jni-type="I" /> </method> Here we see that even though `@jni-return` is `I` -- meaning `int` -- the `@return` property is `uint`. Likewise `parameter/@jni-type` and `parameter/@type`. The JNI ABI is `int`, but we bind in C# as `uint`. ~~ ApiXmlAdjuster ~~ Update `JavaTypeReference` to contain unsigned types: UInt = new JavaTypeReference ("uint"); UShort = new JavaTypeReference ("ushort"); ULong = new JavaTypeReference ("ulong"); UByte = new JavaTypeReference ("ubyte"); ~~ generator ~~ `generator` has the 4 new types added to the `SymbolTable` as `SimpleSymbols`: AddType (new SimpleSymbol ("0", "uint", "uint", "I", returnCast: "(uint)")); AddType (new SimpleSymbol ("0", "ushort", "ushort", "S", returnCast: "(ushort)")); AddType (new SimpleSymbol ("0", "ulong", "ulong", "J", returnCast: "(ulong)")); AddType (new SimpleSymbol ("0", "ubyte", "byte", "B", returnCast: "(byte)")); There are 2 fixups we have to make because we use `GetIntValue(...)`, etc. instead of having unsigned versions: * Override name of which method to call, e.g.: `GetIntValue()` instead of `GetUintValue()`. * Cast the `int` value returned to `uint`. This is accomplished via the new `ISymbol.ReturnCast` property. ~~ A Note On API Compatibility ~~ Bindings which use Kotlin Unsigned Types will *only* work on Xamarin.Android 10.2.0 or later ("Visual Studio 16.5"). The problem is that while we *can* emit C# source code which will *compile* against older versions of Xamarin.Android, if they use arrays they will not *run* under older versions of Xamarin.Android. For example, imagine this binding code for the above Kotlin `Example.array()` method: // C# Binding of Example.array() partial class Example { public unsafe uint[] Array(uint[] value) { const string __id = "array--ajY-9A.([I)[I"; IntPtr native_value = JNIEnv.NewArray ((int[]) (object) value); // Works!...ish? JniArgumentValue* __args = stackalloc JniArgumentValue [1]; __args [0] = new JniArgumentValue (native_value); JniObjectReference r = _members.InstanceMethods.InvokeVirtualIntMethod (__id, this, __args); return (uint[]) JNIEnv.GetArray (r.Handle, JniHandleOwnership.DoNotTransfer, typeof (uint)); } } That could conceivably *compile* against older Xamarin.Android versions. However, that cannot *run* against older Xamarin.Android versions, as *eventually* `JNIEnv.GetArray()` will hit some dictionaries to determine how to marshal `IntPtr` to a `uint[]`, at which point things will fail because there is no such mapping *until* Xamarin.Android 10.2.0. We feel that a "hard" ABI requirement will have more "graceful" failure conditions than a solution which doesn't add ABI requirements. In this case, if you create a Kotlin library binding which exposes unsigned types, attempting to build an app in Release configuration against older Xamarin.Android versions will result in a linker error, as the required `JNIEnv` methods will not be resolvable. (cherry picked from commit 71afce5)

jpobst force-pushed the unsigned-types branch 4 times, most recently from a1528b4 to 1e178fd Compare December 17, 2019 15:11

jpobst added 3 commits December 18, 2019 16:19

[Bytecode] Surface Kotlin's unsigned types in .class-parse XML

3d64690

[generator] Add support for unsigned types.

7508321

Add support for Kotlin unsigned array types.

0ef3d55

jpobst force-pushed the unsigned-types branch from e783749 to 9f70796 Compare December 18, 2019 22:19

Add unit tests.

dd8a6a0

jpobst force-pushed the unsigned-types branch from 9f70796 to dd8a6a0 Compare December 19, 2019 15:16

jpobst mentioned this pull request Dec 19, 2019

[JNIEnv] Add support for Kotlin's unsigned types. dotnet/android#4054

Merged

jpobst marked this pull request as ready for review December 19, 2019 20:13

jonpryor reviewed Dec 20, 2019

View reviewed changes

jpobst commented Dec 20, 2019

View reviewed changes

Add test showing Kotlin's "Array<UIntArray>" is encoded as "UIntArray…

59f610c

…[]" so we do not do any Kotlin fixups on it.

jonpryor merged commit 71afce5 into master Jan 7, 2020

jonpryor deleted the unsigned-types branch January 7, 2020 03:28

brendanzagaeski added this to the Published: 10.2 (d16-5) milestone Mar 27, 2020

github-actions bot locked and limited conversation to collaborators Apr 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Support Kotlin's unsigned types #539

Support Kotlin's unsigned types #539

Uh oh!

jpobst commented Dec 12, 2019 •

edited

Loading

Uh oh!

jonpryor commented Dec 20, 2019

Uh oh!

jonpryor Dec 20, 2019 •

edited

Loading

Uh oh!

jonpryor Dec 20, 2019

Uh oh!

jonpryor Jan 7, 2020

Uh oh!

jonpryor commented Dec 20, 2019

Uh oh!

jonpryor Dec 20, 2019 •

edited

Loading

Uh oh!

jpobst Dec 20, 2019

Uh oh!

jpobst Dec 20, 2019 •

edited

Loading

Uh oh!

jpobst commented Dec 20, 2019

Uh oh!

jpobst left a comment

Uh oh!

jpobst commented Jan 6, 2020

Uh oh!

Uh oh!

		@@ -0,0 +1,90 @@
		// Metadata.xml XPath class reference: path="/api/package[@name='java.code']/class[@name='MyClass']"


		protected MyClass (IntPtr javaReference, JniHandleOwnership transfer) : base (javaReference, transfer) {}

		public unsafe uint[] UIntProp {

Support Kotlin's unsigned types #539

Support Kotlin's unsigned types #539

Uh oh!

Conversation

jpobst commented Dec 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Class Parse

ApiXmlAdjuster

Generator

Uh oh!

jonpryor commented Dec 20, 2019

Uh oh!

jonpryor Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonpryor Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

jonpryor Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

jonpryor commented Dec 20, 2019

Uh oh!

jonpryor Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpobst Dec 20, 2019

Choose a reason for hiding this comment

Uh oh!

jpobst Dec 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpobst commented Dec 20, 2019

Uh oh!

jpobst left a comment

Choose a reason for hiding this comment

Uh oh!

jpobst commented Jan 6, 2020

Uh oh!

Uh oh!

jpobst commented Dec 12, 2019 •

edited

Loading

jonpryor Dec 20, 2019 •

edited

Loading

jonpryor Dec 20, 2019 •

edited

Loading

jpobst Dec 20, 2019 •

edited

Loading