Skip to content

Conversation

@jpobst
Copy link
Contributor

@jpobst jpobst commented Nov 4, 2020

Context: #5251
Context: dotnet/java-interop#745

dotnet/java-interop@c3c3575...99897b2

Adds a test for static methods on an interface that is implemented by a class. Our existing static test was an interface that only contained static members, and thus we did not try to implement the interface. Implementing the interface exposed a JCW error.

@jpobst jpobst changed the title [Binding tests] Add test for mixing static and regular interface methods [Binding tests] Add test for implementing interface which contains static methods Nov 5, 2020
jonpryor pushed a commit to dotnet/java-interop that referenced this pull request Nov 10, 2020
Context: 855b7e9
Context: dotnet/android#5251
Context: dotnet/android#5262

When an Java interface contains a `static` method in API-30+, we use
C# 8.0's Default Interface Methods support to place corresponding
`static` methods in the C# interface binding:

	// Java
	public /* partial */ interface Comparator<T> {}
	    int compare(T o1, T o2);

	    static <T>
	    Comparator<T> comparingDouble(ToDoubleFunction<? super T> keyExtractor) {…}

	    // …
	}

	// C# binding
	public partial interface IComparator {
	    [Register ("compare", "(Ljava/lang/Object;Ljava/lang/Object;)I", "GetCompare_Ljava_lang_Object_Ljava_lang_Object_Handler:Java.Util.IComparatorInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null")]
	    int Compare (Java.Lang.Object? o1, Java.Lang.Object? o2);

	    [Register ("comparingDouble", "(Ljava/util/function/ToDoubleFunction;)Ljava/util/Comparator;", "", ApiSince = 24)]
	    [global::Java.Interop.JavaTypeParameters (new string [] {"T"})]
	    public static unsafe Java.Util.IComparator? ComparingDouble (Java.Util.Functions.IToDoubleFunction? keyExtractor) {…}
	}

This works until you (1) attempt to implement the C# interface:

	public partial class CompareSizesByArea : Java.Lang.Object, Java.Util.IComparator
	{
	    public int Compare (Java.Lang.Object lhs, Java.Lang.Object rhs) {…}
	}

and then (2) *instantiate* that class:

	var comparator = new CompareSizesByArea();

The result is an `ArgumentException`:

	System.ArgumentException: Couldn't bind to method ''.
	  at System.Delegate.GetCandidateMethod (System.Type type, System.Type target, System.String method, System.Reflection.BindingFlags bflags, System.Boolean ignoreCase, System.Boolean throwOnBindFailure)
	  at System.Delegate.CreateDelegate (System.Type type, System.Type target, System.String method, System.Boolean ignoreCase, System.Boolean throwOnBindFailure)
	  at System.Delegate.CreateDelegate (System.Type type, System.Type target, System.String method)
	  at Android.Runtime.AndroidTypeManager.RegisterNativeMembers (Java.Interop.JniType nativeClass, System.Type type, System.String methods)
	  at Android.Runtime.JNIEnv.RegisterJniNatives (System.IntPtr typeName_ptr, System.Int32 typeName_len, System.IntPtr jniClass, System.IntPtr methods_ptr, System.Int32 methods_len)
	  /* missing stack entries */
	  at (wrapper managed-to-native) Java.Interop.NativeMethods.java_interop_jnienv_alloc_object(intptr,intptr&,intptr)
	  at Java.Interop.JniEnvironment+Object.AllocObject (Java.Interop.JniObjectReference type)
	  at Java.Interop.JniType.AllocObject ()
	  at Java.Interop.JniPeerMembers+JniInstanceMethods.StartCreateInstance (System.String constructorSignature, System.Type declaringType, Java.Interop.JniArgumentValue* parameters)
	  at Java.Lang.Object..ctor ()
	  at App16.MainActivity+CompareSizesByArea..ctor ()

Why this fails is because of the intermediate Java Callable Wrapper,
which emits a method override for every method in the interface,
*including `static` methods*:

	// Java callable wrapper
	public /* partial */ class CompareSizesByArea extends java.lang.Object, implements java.util.Comparator {
	    public static final String __md_methods;
	    static {
	        __md_methods =
	            "n_compare:(Ljava/lang/Object;Ljava/lang/Object;)I:GetCompare_Ljava_lang_Object_Ljava_lang_Object_Handler:Java.Util.IComparatorInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null\n" +
	            … +
	            "n_comparingDouble:(Ljava/util/function/ToDoubleFunction;)Ljava/util/Comparator;:\n" +
	            …
	        mono.android.Runtime.register ("CompareSizesByArea, …", CompareSizesByArea.class, __md_methods);
	    }

	    public int compare (java.lang.Object p0, java.lang.Object p1)
	    {
	        return n_compare (p0, p1);
	    }

	    private native int n_compare (java.lang.Object p0, java.lang.Object p1);


	    public java.util.Comparator comparingDouble (java.util.function.ToDoubleFunction p0)
	    {
	        return n_comparingDouble (p0);
	    }

	    private native java.util.Comparator n_comparingDouble (java.util.function.ToDoubleFunction p0);
	}

(Note: The `/* missing stack entries */` line (above) is from the
Java-side `CompareSizesByArea` static constructor execution, which
calls `Runtime.register()`, which in turn calls
`JNIEnv.RegisterJniNatives()`.)

The `Runtime.register()` invocation eventually hits
[`AndroidTypeManager.RegisterNativeMembers()`][0], which "splits"
`__md_methods` into lines, and each line into `:`-separated values.
One of those values is expected to be a "connector method", obtainable
via Reflection.  In the case of `n_comparingDouble`, the connector
method *is not specified*: note that the line ends with `:`, unlike
the entry for `n_compare`, which lists `GetCompare_…`.

This "missing" connector method is the cause of the
`ArgumentException`.

Fix the problem by *skipping* `static` methods found from interfaces
when emitting Java Callable Wrappers.  This ensures that `__md_methods`
won't contain entries with missing "connector" methods.

An integration test is part of dotnet/android#5262, as we're
not currently able to use C#8 Default Interface Methods in a `net472`
target framework for unit testing within the xamarin/Java.Interop repo.

[0]: https://github.com/xamarin/xamarin-android/blob/15b37758e9d65c82aebe1a327747b285f38ce791/src/Mono.Android/Android.Runtime/AndroidRuntime.cs#L394-L413
@jpobst jpobst changed the title [Binding tests] Add test for implementing interface which contains static methods Bump to xamarin/java.interop@99897b2 Nov 10, 2020
@jpobst jpobst marked this pull request as ready for review November 11, 2020 14:58
@jpobst jpobst requested a review from jonpryor as a code owner November 11, 2020 14:58
@jonpryor jonpryor merged commit ca9a0eb into master Nov 11, 2020
@jonpryor jonpryor deleted the ji-static-jcw branch November 11, 2020 16:56
@github-actions github-actions bot locked and limited conversation to collaborators Jan 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants