Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Java 8 Interface Binding #25

Closed
jonpryor opened this issue Apr 26, 2016 · 13 comments
Closed

Java 8 Interface Binding #25

jonpryor opened this issue Apr 26, 2016 · 13 comments
Milestone

Comments

@jonpryor
Copy link
Member

jonpryor commented Apr 26, 2016

Android N uses new Java 8 language features such as interface default methods and interface static methods, which are features that tools/generator hasn't had to deal with before.

The question: how should they be dealt with?

    // Java
    public interface HelloJava8 {
        public static final int VALUE = 42;
        public void a ();
        public default void defaultMethod() {
        }
        public static void staticMethod() {
        }
    }

There are at least three ways to do so:

  1. Ignore them.
  2. Expose them.
  3. Treat them specially.

Ignore them

Interface default methods don't need to be implemented, and static methods aren't implementable. The binding mechanism could thus simply ignore them entirely.

    // C#
    public interface IHelloJava8 {
        public void A ();
    }
    public static class HelloJava8 {
        public const int Value = 42;
    }

This isn't entirely desirable, because it also means default interface methods can't be called.

    IHelloJava8 value = ...
    value.DefaultMethod(); // error
    // No way to invoke HelloJava8.staticMethod()

On the plus side, implementors don't need to worry about them:

    public class WootJava8 : Java.Lang.Object, IHelloJava8 {
        public void A () {}
    }

Expose them

The "simple" version of "expose them" is do the simple thing: static methods are bound in the constant-containing static class, and default methods are bound as normal methods:

    // C#
    public interface IHelloJava8 {
        public void A ();
        public void DefaultMethod ();
    }
    public static partial class HelloJava8 {
        public const int Value = 42;
        public void StaticMethod ()
        {...}
    }

This allows calling both methods:

    IHelloJava8 value = ...
    value.DefaultMethod(); // works
    HelloJava8.StaticMethod(); // works

However, this means that C# subclasses need to implement all the default methods:

    public class WootJava8 : Java.Lang.Object, IHelloJava8 {
        public void A () {}
        public void DefaultMethod () {...}
    }

This might not seem bad in this example, but java.util.Collection contains five default methods (one inherited from Iterable).

This might be fine and acceptable, and we could help this scenario by providing access to the default method implementation:

    public static partial class HelloJava8 {
        public void InvokeDefaultMethod(IHelloJava8 self)
        {
            /* Non-virtual invocation of HelloJava8.defaultMethod() */
        }
    }

    public partial class WootJava8 {
        public void DefaultMethod ()
        {
            HelloJava8.InvokeDefaultMethod (this);
        }
    }

However, such "help" will require documentation and training to know about, complicating use.

Treat them specially

To a large extent, the concern is around C# implementations of the interface. It is seen that requiring that optional methods be implemented will complicate the C# experience. Perhaps we can fix this by splitting out the optional methods?

Then, we can use extension methods to invoke the default methods:

    // C#
    public interface IHelloJava8 {
        public void A ();
    }
    public static partial class HelloJava8 {
        public const int Value = 42;
        public void StaticMethod ()
        {...}

        public void DefaultMethod (this IHelloJava8 self)
        {
            // *Virtual* dispatch to invoke self.defaultMethod()
        }

        public interface IDefaultMethods {
            public void DefaultMethod ();
        }
    }

This allows calling both methods:

    IHelloJava8 value = ...
    value.DefaultMethod(); // works via extension method
    HelloJava8.StaticMethod(); // works

It also means that C# types don't need to implement the default methods:

    public partial class WootJava8 : Java.Lang.Object, IHelloJava8 {
        public void A () {}
    }

...but if the C# type does want to implement them, they can all be implemented by using the IDefaultMethods interface:

    public partial class WootJava8 : HelloJava8.IDefaultMethods {
        public void DefaultMethod () {}
    }
@jonpryor
Copy link
Member Author

At present, I'm leaning toward (2) Expose Them, if only because it's implementation is simple (though the documentation effort regarding the presence of HelloJava8.InvokeDefaultMethod() isn't simple).

@marukami
Copy link

I don't think ignoring them is an option.
I don't like (2) Expose Them. It adds unneeded name mangling. The documentation issue is also a major challenge.
I like (3) Treat them specially, It feels much cleaner from a consumer perspective.

@jonpryor
Copy link
Member Author

jonpryor commented May 1, 2016

I don't like (2) Expose Them. It adds unneeded name mangling.

I don't understand what you mean by "unneeded name mangling." What name mangling? The Invoke helpers?

The documentation issue is also a major challenge.

Agreed. However, (3) also has documentation issues, just different documentation issues.

@marukami
Copy link

marukami commented May 2, 2016

Yes, the Invoke helper. Mangling might not have been the right language there.

What documentation do you think will be needed? I can understand that (2) Invoke Requiring docs

@jonpryor
Copy link
Member Author

jonpryor commented May 2, 2016

What documentation do you think will be needed [in (3)]?

The presence and use of the IDefaultMethods interface. It'll be a situation of "Java declares a HelloJava8.defaultMethod() interface method, and I can call that method from C#, but how do I implement that method in C#? It's not present in the IHelloJava8 interface." It's in the HelloJava8.IDefaultMethods interface, which code completion won't know about, and who's going to know to look for that interface without...documentation. :-)

(2) answers that question: it's an interface method, therefore you implement it. It's only when you don't want to implement the default interface method that documentation is required, or when you want to invoke the default behavior.

Both (2) and (3) raise the question of what to do with our "missing abstract method generator" (@radekdoulik? ;-) -- presumably it should check for and use the default interface method if present, but we'd need a way for the generator to know that (likely a new custom attribute).

jonpryor added a commit to jonpryor/java.interop that referenced this issue May 25, 2016
If we bind those methods into interfaces, our users will have to implement
those methods unlike Java8, and that's more annoying than missing them.

Related: dotnet#25
@jonpryor
Copy link
Member Author

Ignore them and Treat them specially don't interact well with Java versioning: default interface methods aren't restricted to new methods; existing methods can be made default!

Case in point: [java.lang.reflect.AnnotatedElement.html.isAnnotationPresent()](https://developer.android.com/reference/java/lang/reflect/AnnotatedElement.html#isAnnotationPresent%28java.lang.Class<? extends java.lang.annotation.Annotation>%29) is non-default in API-23 and is now default in API-24. The Ignore them and Treat them specially approaches result in removing the IsAnnotationPresent() method from the IAnnotatedElement interface, which would break all implementations.

This can be fixed in Mono.Android.dll -- we know what the previous API is, we can manually fixup such removed members -- but I'm dubious of the effects on 3rd party .jar bindings such as the Android Support library. I believe that most .jar bindings will consist of as little effort as possible to get a compiling binding, and if the .jar changes interface methods to default interface methods between versions, this can break existing C# interface implementations.

I'm very dubious of solutions (1) and (3) right now.

@jonpryor
Copy link
Member Author

The Expose them (2) approach has a different problem: if default methods are added to an interface over time, that means that previously built C# classes implementing the interface will break.

Xamarin.Android 6.1 added some support to "fixup" added interface methods with an implementation that throws AbstractMethodError, which made sense then, but doesn't make sense in the context of default interface methods.

What we can instead do is place a custom attribute on the default method which refers to the class with the Invoke method wrapper:

public interface IHelloJava8 {
    public void A ();
    [JavaDefaultInterfaceMethod (typeof (HelloJava8))]
    public void DefaultMethod ();
}
partial public class HelloJava8 {
    public void InvokeDefaultMethod(IHelloJava8 self) {...}
}

The abstract method fixup code could then fixup "missing" interface methods to invoke the default method:

/* IHelloJava8 vPrev lacks defaultMethod(), for arguments sake */
class MyHello : IHelloJava8 {
    public void A () {}
    // NO implementation of DefaultMethod()
}

During Xamarin.Android packaging, we would fix MyHello to be:

class MyHello : IHelloJava8 {
    public void A () {}
    public void DefaultMethod()
    {
        HelloJava8.InvokeDefaultMethod (this);
    }
}

This would extend the existing support for versioned interfaces in a natural manner, causing the generated method implementation to invoke the default method instead of throwing an exception.

@atsushieno
Copy link
Contributor

While all of those choices have problems, their severity on the issues are different.

I believe 1) is the only choice we can take with least problem so far (and that's what we do right now). It is the most conservative and the safest on future API breakage than other choices. By choosing 2) or 3) you would have already brought troubles regarding IAudioRouting fixes.
dotnet/android@576fb98

Default interface methods that "used to be non-default" can be "fixed" by binding authors to change "abstract" attribute on the method i.e.:

<attr path="/path/to/the/method" name="abstract">true</attr>

It should be also noted that those binding authors should be responsible to API changes. They can be the only responsible party.

  1. Apart from the known obvious issue, IAudioRouting.OnRoutingChangedListener also exposed another issue: it is possible for Java8 to add a new base interfaces to an existing interface without API breakage when such new base interfaces only have default methods. If you blindly bind those default methods in C#, it (of course) results in API breakage (i.e. missing method implementation) to any implementor of the interface.

It should be noted that once we expose extraneous methods to implement, especially at generator level, and we find any fatal problem (like we did for IAudioRouting), we cannot go back to the original state because it would already bring API breakage to everyone. It is the worst scenario.

  1. is almost 1), plus that there is slight chance for some problem that extension method could bring future compatibility issue regarding member resolution, especially because it resides in the same namespace as the primary interface.

Having said that 3) is worth implementing and probably even providing as an opt-in feature.

@ddobrev
Copy link

ddobrev commented Oct 12, 2018

Please fix because we cannot use the latest ExoPlayer.

@atsushieno
Copy link
Contributor

atsushieno commented Oct 13, 2018

You should report specific, individual, actual issues as a new issue, not here without details.

The support for default interface methods is implemented at #341 (when I was at Xamarin)

jonpryor pushed a commit that referenced this issue Aug 1, 2019
Context: #341
Context: #341 (comment)
Context: #25

Java 8 introduced support for [interface default methods][0]:

	// Java
	public interface Example {
	  int defaultInterfaceMethod() {
	    return 42;
	  }
	}

The question is, how should we bind them?

Currently, we *don't* bind interface default methods:

	// C#
	public interface IExample : IJavaObject {
	  // No `DefaultInterfaceMethod()` method
	}

This means that C# types which implement `IExample` don't need to
implement `Example.defaultInterfaceMethod()`, reducing the work that
needs to be done:

	// C#
	public class MyExample : Java.Lang.Object, IExample {
	  // No need to implement Example.defaultInterfaceMethod()!
	}

However, if a C# type *does* wish to implement
`Example.defaultInterfaceMethod()`, they *can* do so by using
[`ExportAttribute`][1], but this can be painful, as it requires
ensuring that the Java side and C# sides are consistent, without
compiler assistance:

	// C#
	partial class MyExample : Java.Lang.Object, IExample {
	  [Java.Interop.Export ("defaultInterfaceMethod")]
	  // If the above string is wrong, e.g. via typo, the method isn't overridden!
	  public int DefaultInterfaceMethod()
	  {
	    return 42*2;
	  }
	}

We want to improve support for this scenario by binding Java
interface default methods as [C#8 Default Interface Members][2],
as C#8 Default Interface Methods have similar semantics as Java 8
interface default methods, and means that `[ExportAttribute]` would
no longer be necessary to override them:

	// C#8?
	partial class MyExample : Java.Lang.Object, IExample {
	  // Just Works™, and the compiler will complain if there are typos.
	  public override int DefaultInterfaceMethod()
	  {
	    return 42*2;
	  }
	}

However, a question arises: how do we *do* that?

	// C#8
	public interface IExample : IJavaObject {
	  [Register ("defaultInterfaceMethod", "()I", ...)]
	  int DefaultInterfaceMethod()
	  {
	    // How do we call `Example.defaultInterfaceMethod()` here?
	  }
	}

The desirable thing would be for `IExample.DefaultInterfaceMethod()`
to have the same implementation as any other method binding, e.g.
when using `generator --codegen-target=XAJavaInterop1`:

	// C#8
	partial interface IExample {
	  [Register ("defaultInterfaceMethod", "()I", ...)]
	  void DefaultInterfaceMethod()
	  {
	    const string __id = "defaultInterfaceMethod.()I";
	    return _members.InstanceMethods.InvokeVirtualInt32Method (__id, this, null);
	  }
	}

The problem is twofold:

 1. There is no `_members` field to access here, and

 2. Even if there were a `_members` field,
    `JniPeerMembers.JniInstanceMethods.InvokeVirtualInt32Method()`
    requires an `IJavaPeerable` instance, and `IExample` doesn't
    implement `IJavaPeerable`.

(1) is straight forwardly solvable, as C#8 Default Interface Members
also adds support for static members of all sorts, so it should be
possible to add a static `_members` field, if deemed necessary.

(2) is the holdup, and has a simple solution: "Just" have every
bound interface *also* implement `IJavaPeerable`!

	- public partial interface IExample : IJavaObject
	+ public partial interface IExample : IJavaObject, IJavaPeerable

"But!", someone cries, "What about API compatibility?!"

Isn't adding `IJavaPeerable` to the implements list of an interface a
Breaking Change?

In theory and *most* practice, *Yes*, this *would* be a
Breaking Change, and thus something to be avoided.

*However*, Xamarin.Android is *not* "most" practice.

It has been an XA4212 error since ab3c2b2 / Xamarin.Android 8.0 to
implement an interface that implements `IJavaObject` *without*
inheriting from `Java.Lang.Object` or `Java.Lang.Throwable`:

	// elicits XA4212 error
	class MyBadClass : IExample {
	  // Implements IJavaObject.Handle
	  public IntPtr Handle {
	    get {return IntPtr.Zero;}
	  }
	}

Meanwhile, both `Java.Lang.Object` and `Java.Lang.Throwable` have
implemented `IJavaPeerable` since Xamarin.Android *6.1*.

*Because* it has been an error to manually implement `IJavaObject`
for nearly two years now, it should be Reasonably Safe™ to update
interfaces to implement *both* `IJavaObject` *and* `IJavaPeerable`.
Doing so should not break any code -- unless they've overridden the
`$(AndroidErrorOnCustomJavaObject)` MSBuild property to False, in
which case they deserve what happens to them.  (It's not really
possible to implement `IJavaObject` in a sane manner, and the
"straightforward" approach means that passing instances of e.g.
`MyBadClass` to Java code will result in passing *`null`* to Java,
which almost certainly is NOT what is intended, hence XA4212!)

[0]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
[1]: https://docs.microsoft.com/en-us/xamarin/android/platform/java-integration/working-with-jni#exportattribute-and-exportfieldattribute
[2]: https://github.com/dotnet/csharplang/blob/f7952cdddf85316a4beec493a0ecc14fcb3241c8/proposals/csharp-8.0/default-interface-methods.md
jonpryor pushed a commit to dotnet/android that referenced this issue Aug 24, 2019
steveisok pushed a commit that referenced this issue Aug 30, 2019
Fixes: #25

Context: #341

Java 8 supports [interface default methods][0]:

	public interface HelloJava8 {
	    public void a ();
	    public default int getFoo () {
	        return 8;
	    }
	    public default void setFoo (int newValue) {
	        throw new UnsupportedOperationException();
	    }
	}

With C#8, C# also supports [default interface members][1].

Add a new `generator --lang-features=default-interface-methods` flag
which takes advantage of C#8 default interface members to bind Java 8
default interface methods:

	// C# binding of HelloJava8
	public partial interface IHelloJava8 : IJavaObject, IJavaPeerable {
	    static new readonly JniPeerMembers _members = new JniPeerMembers ("HelloJava8", typeof (IHelloJava8));

	    void A();

	    virtual unsafe int Foo {
	        [Regsiter ("getFoo", "()I", "…")]
	        get {
	            return _members.InstanceMethods.InvokeVirtualInt32Method ("getFoo.()I", this, null);
	        }
	        [Regsiter ("setFoo", "(I)V", "…")]
	        set {
	            JniArgumentValue* __args = stackalloc JniArgumentValue [1];
	            __args [0] = new JniArgumentValue (value);
	            return _members.InstanceMethods.InvokeVirtualVoidMethod ("setFoo.(I)V", this, __args);
	        }
	    }
	}

C#8 Default Interface Members cannot be used with legacy
`generator --codegen-target=XamarinAndroid`, as they require the
`IJavaPeerable` infrastructure in order to work.

Connector Methods are emitted within the interface binding, and not
within the corresponding `*Invoker` type.

If a Java default interface method is "invalid", we just skip binding
the method instead of invalidating the entire interface, just as we
do with classes and non-`abstract` methods.

Finally, the default interface method implementation uses `virtual`
dispatch, *not* non-`virtual` dispatch, in order to support Java-side
versioning.  For example, imagine `exampele.jar` v1:

	// Java
	public interface Fooable {
	    default void foo() {
	        System.out.println ("Fooable.foo");
	    }
	}
	public class Example implements Fooable {
	}

In v1, `Example` does *not* contain an `Example.foo()` method, though
`foo()` can be invoked on it, because of `Fooable.foo()`:

	Fooable value = new Example();
	value.foo();  // Invokes Fooable.foo()

In v2, `Example` overrides `Fooable.foo`:

	public class Example implements Fooable {
	    public void foo() {
	        System.out.println ("Example.foo");
	    }
	}

If our binding used non-`virtual` dispatch for `IFooable.Foo()`, and
bound `example.jar` v1, then if we updated `example.jar` to v2
*without* producing a new binding -- and why should a new binding
be required? -- then we would continue invoking `Fooable.foo()` when
we *should* be invoking `Example.foo()`.  Use of `virtual` dispatch
thus ensures we support Java-side versioning.

[0]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
[1]: https://github.com/dotnet/csharplang/blob/f7952cdddf85316a4beec493a0ecc14fcb3241c8/proposals/csharp-8.0/default-interface-methods.md
jonpryor pushed a commit to dotnet/android that referenced this issue Sep 6, 2019
…3533)

Context: dotnet/java-interop#25

Use [C#8 Default Interface Members][0] to improve bindings of Java 8
interface constructs, such as [default interface methods][1] and
static methods.

This allows the following Java type:

	// Java
	publc interface Example {
	    void requiredMethod();

	    default void optionalMethod() {
	    }

	    public static void staticMethod() {
	    }
	}

to be bound as:

	// C#
	public partial interface IExample : IJavaPeerable {
	    [Register("requiredMethod", "()V", …)]
	    void RequiredMethod ();
	    
	    [Register("optionalMethod", "()V", …)]
	    public void OptionalMethod()
	    {
		    /* … */
	    }
	    
	    [Register("staticMethod", "()V", …)]
	    public static void StaticMethod()
	    {
		    /* … */
	    }
	}

To enable use of C#8 default interface member generation, the
following two MSBuild properties must be set:

  * `$(LangVersion)`, which controls the C# compiler
    [language syntax version][2], must be set to `preview`.

  * `$(_EnableInterfaceMembers)` must be set to `True`.

For example:

	<PropertyGroup>
	  <LangVersion>preview</LangVersion>
	  <_EnableInterfaceMembers>True</_EnableInterfaceMembers>
	</PropertyGroup>

In the future, this will key entirely off of the `$(LangVersion)`
property and the `$(_EnableInterfaceMembers)` property will be removed.

[0]: https://github.com/dotnet/csharplang/blob/f7952cdddf85316a4beec493a0ecc14fcb3241c8/proposals/csharp-8.0/default-interface-methods.md
[1]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
[2]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-options/langversion-compiler-option
jonpryor pushed a commit to dotnet/android that referenced this issue Sep 9, 2019
…3533)

Context: dotnet/java-interop#25

Use [C#8 Default Interface Members][0] to improve bindings of Java 8
interface constructs, such as [default interface methods][1] and
static methods.

This allows the following Java type:

	// Java
	publc interface Example {
	    void requiredMethod();

	    default void optionalMethod() {
	    }

	    public static void staticMethod() {
	    }
	}

to be bound as:

	// C#
	public partial interface IExample : IJavaPeerable {
	    [Register("requiredMethod", "()V", …)]
	    void RequiredMethod ();
	    
	    [Register("optionalMethod", "()V", …)]
	    public void OptionalMethod()
	    {
		    /* … */
	    }
	    
	    [Register("staticMethod", "()V", …)]
	    public static void StaticMethod()
	    {
		    /* … */
	    }
	}

To enable use of C#8 default interface member generation, the
following two MSBuild properties must be set:

  * `$(LangVersion)`, which controls the C# compiler
    [language syntax version][2], must be set to `preview`.

  * `$(_EnableInterfaceMembers)` must be set to `True`.

For example:

	<PropertyGroup>
	  <LangVersion>preview</LangVersion>
	  <_EnableInterfaceMembers>True</_EnableInterfaceMembers>
	</PropertyGroup>

In the future, this will key entirely off of the `$(LangVersion)`
property and the `$(_EnableInterfaceMembers)` property will be removed.

[0]: https://github.com/dotnet/csharplang/blob/f7952cdddf85316a4beec493a0ecc14fcb3241c8/proposals/csharp-8.0/default-interface-methods.md
[1]: https://docs.oracle.com/javase/tutorial/java/IandI/defaultmethods.html
[2]: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/compiler-options/langversion-compiler-option
@brendanzagaeski
Copy link
Contributor

Release status update

A new Preview version of Xamarin.Android has now been published on Windows that includes the new capabilities related to this item. See the release notes for additional details.

The new functionality is not yet available on macOS. I will update this item again when a Preview version with the new functionality is available on macOS.

Feature included in Xamarin.Android 10.0.99.100.

Feature included on Windows in Visual Studio 2019 version 16.4 Preview 1. To try the Preview version that includes the fix, check for the latest updates in Visual Studio Preview.

Feature not yet available on macOS.

@brendanzagaeski
Copy link
Contributor

brendanzagaeski commented Dec 4, 2019

Release status update

A new Release version of Xamarin.Android has now been published on Windows that includes the new capabilities related to this item. See the release notes for additional details.

The new functionality is not yet published in a Release version on macOS. I will update this item again when a Release version with the new functionality is available on macOS.

Feature included in Xamarin.Android 10.1.0.30.

Feature included on Windows in Visual Studio 2019 version 16.4. To get the new version that includes the feature, check for the latest updates or install the latest version from https://visualstudio.microsoft.com/downloads/.

(Feature also included on macOS in Visual Studio 2019 for Mac version 8.4 Preview 2.1 and higher. To try the Preview version that includes the feature, check for the latest updates on the Preview updater channel.)

@brendanzagaeski
Copy link
Contributor

Release status update

A new Release version of Xamarin.Android has now been published on macOS that includes the new capabilities related to this item. See the release notes for additional details.

Feature included in Xamarin.Android 10.1.1.0.

Feature included on macOS in Visual Studio 2019 for Mac version 8.4. To get the new version that includes the feature, check for the latest updates on the Stable updater channel.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 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

No branches or pull requests

5 participants