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

default interface methods support #1939

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d20453b
[msbuild] Add support for C# 8 default interface methods (DIMs) step 1.
atsushieno Jul 9, 2018
c5618e8
Add tests for default interface method bindings.
atsushieno Jul 10, 2018
7918653
Some cosmetic fix in default interface methods test - use latest for …
atsushieno Jul 11, 2018
d688d2b
bump xamarin-android-buildenhancer package. Its jar build was awkward.
atsushieno Jul 11, 2018
4109ae5
bump xamarin-android-build-enhancer to support build inputs/outputs, …
atsushieno Jul 12, 2018
4d29c5a
[linker] Do not emit AbstractMethodError thrower when target method i…
atsushieno Jul 17, 2018
31ec999
[tests] default interface methods run-time test: make it working.
atsushieno Jul 17, 2018
5f0ff78
[Mono.Android] enable default interface method generation.
atsushieno Jul 18, 2018
0e4a32e
[Mono.Android] new metadata fixup required for Animator event methods.
atsushieno Jul 18, 2018
091fb51
[Mono.Android] disable XYZStream.Spliterator() generation.
atsushieno Jul 18, 2018
938c293
[Mono.Android] minimum workaround to make it build with DIMs generated.
atsushieno Jul 19, 2018
f4084d1
[Mono.Android] bring back ConcurrentSkipListMap, Fixed all the ABI br…
atsushieno Jul 19, 2018
f180241
bump java.interop.
atsushieno Jul 20, 2018
6521086
xamarin.android.csc.dim needs to be nuget-restored at `make prepare`.
atsushieno Jul 26, 2018
01152f6
[Mono.Android] bring back some DIM related types and methods for Spli…
atsushieno Jul 26, 2018
14ab4a7
[Mono.Android] enable java.time package.
atsushieno Jul 26, 2018
6dd52c9
[Mono.Android] bring back java.nio.NetworkChannel and co.
atsushieno Jul 27, 2018
e0e42e2
[docs] add documentation about default interface methods.
atsushieno Jul 30, 2018
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
144 changes: 144 additions & 0 deletions Documentation/project-docs/DefaultInterfaceMethods.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
# Xamarin.Android and C# 8.0 default interface methods

We have been trying to implement support for C# 8.0 default interface methods (DIMs) in Xamarin.Android, to reflect Java 8 default interface methods (DIMs). Here we are going to write some notes about it.

## Reference materials

There are [couple](https://github.com/dotnet/csharplang/issues/52) [of](https://github.com/dotnet/csharplang/issues/288) [relevant](https://github.com/dotnet/csharplang/blob/master/proposals/default-interface-methods.md) [issues](https://github.com/dotnet/roslyn/issues/17952) (and docs), but not everything is consistent. This post is written based on the implementation (mono's `csc-dim` version of csc) I have been using and packaging.

## What are different from Java

The feature is similar, but there are actually some caucious differences.

This is how C# 8.0 DIMs work:

```
~/Desktop$ cat dim.cs
using System;

public interface IFoo
{
int X () { return 1; }
int Y => 2;
}

public class Bar : IFoo
{
}

public class Baz
{
public static void Main ()
{
var bar = new Bar ();
var foo = (IFoo) bar;
Console.WriteLine (foo.X ());
Console.WriteLine (bar.X ());
}
}
~/Desktop$ csc-dim dim.cs -langversion:latest
Microsoft (R) Visual C# Compiler version 42.42.42.42424 (<developer build>)
Copyright (C) Microsoft Corporation. All rights reserved.

dim.cs(20,26): error CS1061: 'Bar' does not contain a definition for 'X' and no extension method 'X' accepting a first argument of type 'Bar' could be found (are you missing a using directive or an assembly reference?)
```

(`csc-dim` is a special C# compiler distributed in the latest mono, which is anohter roslyn csc build which enabled C# 8 DIMs. We need `-langversion:latest` to actually use DIMs even with it.)

You cannot call `new Bar ().X ()` because `X()` does not exist in `Bar`. In Java 8, `Bar.X()` is publicly accessible. In current C# 8.0 language specification, `Bar.X()` is **explicitly implemented** and **has no public API**. It's like existing non-DIMs that are not implicitly declared in a class.

(It is explained at https://github.com/dotnet/csharplang/issues/288#issue-215243291 too. See "Concrete methods in interfaces".)

It is actually argurable design, but that is another story. we're explaining only the fact part.

It should be noted that we are under different circumstances than others:

- We have no control over the API. We have no position to define API. Even Googlers don't especially in java.\* API.
- Android API, or even Java 8 API, has grown up to convert non-DIMs to DIMs.
- We have existing customers who have implemented existing Xamarin.Android interfaces.

Therefore my investigation on C# 8.0 DIMs was in quite different direction from what others do/did.

Let's see what kinf of default interface methods Android API has:

- `java.lang.Iterable#spliterator()` is a DIM which was added at API Level 24 (Android N). Google had moved to OpenJDK class libraries at API Level 24 and there was a bunch of additions like this.
- Similarly, `java.lang.Iterable#remove()` is a DIM, but it had existed before API Level 24 as non-DIM. Since this method was public, there would be customers who have used this method.


Would there be any behavioral difference result in that Xamarin.Android cannot support DIMs? Not anything so far.

What happens if, some customer has code that uses this non-default version of the method, like:

```
public class MyIterator : Java.Lang.IIterator
{
// (there must be customer implementation of Remove() because it was not default before)
public void Remove () { ... }
}

public void ReduceAndSkipSome (MyIterator iterator)
{
IIterator i = iterator; // RValue can be anything. This is a PoC so just let it as is, but with explicit interface type.
while (i.HasNext ()) {
var current = i.Next ();
if (MatchesSomeCondition (current)
i.Remove (); // therefore it should still compile.
}
}
```

This will work.

What if once we replace non-DIM IIterator.Remove() with a DIM, and customer rebuilt project without touching any of their source code, will it invoke `MyIterator.Remove()` ? The answer is **yes**. It is not `IIterator.Remove()` and it should still work just like it used to do.

There won't be ABI breakage, but there is a problem that Java developers won't face. If a Xamarin.Android interface provides some DIMs and a third party library class `C` implements it without implicitly overriding it, the class is not as useful as one in Java, because of the reason I explained at first. There is no callable `new Bar().X()` where `X` is a DIM in `Foo` which `Bar` implements.

To avoid that uselessness, the `C` developer needs to override ALL those DIMs. That's what current specification supporters claim developers to do. The current specification is likely the most consistent with the existing interface specification indeed.

Another case Java developers would get stuck:

```
public interface IComparer
{
int Compare (Object o1, Object o2);
}
public interface IList
{
void Sort (IComparer comparer) { ... } // DIM
}
public class Vector : IList { ... } // implements everything in IList
public class MyVector : Vector
{
void IList.Sort (IComparer comparer) { ...} // ERROR!!
}
```

This doesn't compile because "`MyVector` does not implement `IList`"... to make it compile, you will have to explicitly add `IList` as an implemented interface to `MyVector` declaration:

```
public class MyVector : Vector, IList { ... }
```

Then it will compile (and you don't have to implement non-DIMs in `IList` because `Vector` implements them). It is weird but that's how C# works.


### What Xamarin.Android currently does

In the stable releases as of now, we don't bind any "new" default interface methods because generating them as non-default interface methods means the interface will be messy to implement (for example, what if you are told "you are supposed to implement `forEachRemaining()` and `remove()` in all of your `Iterator` implementation classes" ?). It is safe to ignore them because they don't need implementation.

On the other hand, we generate managed methods for default interface methods IF they had existed as non-DIM in the earlier API Levels. There is a supplemental reason for that: `Mono.Android.dll` is offered per API Level, but it is not only from the corresponding API Level. Each `Mono.Android.dll` is a result of "merging" APIs from the earlier API. It is to bring API consistency across API Levels (if you look for `api-merge` in `xamarin-android/src/Mono.Android/Mono.Android.targets` you'll see how it is done). With this API model, it kind of makes sense to keep non-DIM as is, regardless of whether it is DIM now or not.

For example, `javax.security.auth.Destroyable#destroy()` is a DIM in API Level 24 or later, but we mark it as non-DIM because it had existed since API Level 1.


### What Xamarin.Android will change

We keep "already existed as non-DIMs" as is i.e. `Javax.Security.Auth.Destroyable.Destroy()` will remain DIM. I'm not sure if we will ever change that. This means Xamarin.Android application and library developers will be suffered from "extraneous work to implement some interface methods" to some extent.

On the other hand, there will be totally new interface methods as DIMs such as `Java.Util.IIterator.ForEachRemaining()`.

For non-abstract overrides such as `Java.Util.IPrimitiveIteratorOfDouble.ForEachRemaining()`, it will override the DIM as non-abstract, explicitly.

For abstract overrides such as `Java.Util.IPrimitiveIterator.ForEachRemaining()`, it will override the DIM as abstract, explicitly.


14 changes: 14 additions & 0 deletions Xamarin.Android-Tests.sln
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Performance-Tests", "Perfor
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "timing", "build-tools\timing\timing.csproj", "{37CAA28C-40BE-4253-BA68-CC5D7316A617}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Xamarin.Android.DefaultInterfaceMethods-Tests", "tests\CodeGen-Binding\Xamarin.Android.DefaultInterfaceMethods-Tests\Xamarin.Android.DefaultInterfaceMethods-Tests.csproj", "{80E5171A-56DF-48AC-8AD9-65028319C1B8}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Xamarin.Android.DefaultInterfaceMethods-Lib", "tests\CodeGen-Binding\Xamarin.Android.DefaultInterfaceMethods-Lib\Xamarin.Android.DefaultInterfaceMethods-Lib.csproj", "{E2F0F78B-6088-4282-A49C-122EE28611FF}"
EndProject
Global
GlobalSection(SharedMSBuildProjectFiles) = preSolution
tests\Xamarin.Forms-Performance-Integration\Xamarin.Forms.Performance.Integration.projitems*{195be9c2-1f91-40dc-bd6d-de860bf083fb}*SharedItemsImports = 13
Expand Down Expand Up @@ -194,6 +198,14 @@ Global
{37CAA28C-40BE-4253-BA68-CC5D7316A617}.Debug|Any CPU.Build.0 = Debug|Any CPU
{37CAA28C-40BE-4253-BA68-CC5D7316A617}.Release|Any CPU.ActiveCfg = Release|Any CPU
{37CAA28C-40BE-4253-BA68-CC5D7316A617}.Release|Any CPU.Build.0 = Release|Any CPU
{80E5171A-56DF-48AC-8AD9-65028319C1B8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{80E5171A-56DF-48AC-8AD9-65028319C1B8}.Debug|Any CPU.Build.0 = Debug|Any CPU
{80E5171A-56DF-48AC-8AD9-65028319C1B8}.Release|Any CPU.ActiveCfg = Release|Any CPU
{80E5171A-56DF-48AC-8AD9-65028319C1B8}.Release|Any CPU.Build.0 = Release|Any CPU
{E2F0F78B-6088-4282-A49C-122EE28611FF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{E2F0F78B-6088-4282-A49C-122EE28611FF}.Debug|Any CPU.Build.0 = Debug|Any CPU
{E2F0F78B-6088-4282-A49C-122EE28611FF}.Release|Any CPU.ActiveCfg = Release|Any CPU
{E2F0F78B-6088-4282-A49C-122EE28611FF}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -225,6 +237,8 @@ Global
{F4DAFD78-BE76-46C9-A1AD-85D8C91CD77B} = {9B63992C-2201-4BB0-BD00-D637B481A995}
{2DD1EE75-6D8D-4653-A800-0A24367F7F38} = {9B63992C-2201-4BB0-BD00-D637B481A995}
{37CAA28C-40BE-4253-BA68-CC5D7316A617} = {68B8E272-5B12-47AA-8923-550B9CE535C7}
{80E5171A-56DF-48AC-8AD9-65028319C1B8} = {2EFFECF5-1CCA-4005-AE62-1D6F01C88DF4}
{E2F0F78B-6088-4282-A49C-122EE28611FF} = {2EFFECF5-1CCA-4005-AE62-1D6F01C88DF4}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {8643CD20-B195-4919-8135-27549488237E}
Expand Down
2 changes: 1 addition & 1 deletion external/Java.Interop
Submodule Java.Interop updated 26 files
+12 −1 Java.Interop.sln
+1 −1 src/Java.Interop/Java.Interop/JniPeerMembers.JniInstanceMethods_Invoke.tt
+292 −0 tests/binding-integrated-Tests/BindingBuilder.cs
+65 −0 tests/binding-integrated-Tests/BindingProject.cs
+26 −0 tests/binding-integrated-Tests/README.md
+134 −0 tests/binding-integrated-Tests/SampleTest.cs
+20 −0 tests/binding-integrated-Tests/binding-integration-Tests.csproj
+9 −23 tools/generator/ClassGen.cs
+36 −10 tools/generator/CodeGenerator.cs
+38 −8 tools/generator/GenBase.cs
+27 −9 tools/generator/InterfaceGen.cs
+20 −8 tools/generator/JavaInteropCodeGenerator.cs
+2 −2 tools/generator/Method.cs
+4 −0 tools/generator/MethodBase.cs
+3 −3 tools/generator/Property.cs
+1 −1 tools/generator/Tests/Integration-Tests/BaseGeneratorTest.cs
+42 −0 tools/generator/Tests/Integration-Tests/Compiler.cs
+29 −0 tools/generator/Tests/Integration-Tests/DefaultInterfaceMethods.cs
+1 −1 tools/generator/Tests/Unit-Tests/JavaInteropCodeGeneratorTests.cs
+1 −1 tools/generator/Tests/Unit-Tests/XamarinAndroidCodeGeneratorTests.cs
+31 −0 tools/generator/Tests/expected.ji/DefaultInterfaceMethods/DefaultInterfaceMethods.xml
+268 −0 tools/generator/Tests/expected.ji/DefaultInterfaceMethods/Xamarin.Test.ITheInterface.cs
+52 −0 tools/generator/Tests/expected.ji/DefaultInterfaceMethods/Xamarin.Test.TheImplementor.cs
+15 −0 tools/generator/Tests/generator-Tests.csproj
+1 −0 tools/generator/Tests/packages.config
+1 −1 tools/generator/XamarinAndroidCodeGenerator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#if ANDROID_26

using System;
using Java.Net;

namespace Java.Nio.Channels
{
partial class AsynchronousServerSocketChannel
{
INetworkChannel INetworkChannel.Bind (SocketAddress address)
{
return Bind (address);
}
INetworkChannel INetworkChannel.SetOption (ISocketOption option, Java.Lang.Object value)
{
return SetOption (option, value);
}
}
}

#endif
21 changes: 21 additions & 0 deletions src/Mono.Android/Java.Nio.Channels/AsynchronousSocketChannel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#if ANDROID_26

using System;
using Java.Net;

namespace Java.Nio.Channels
{
partial class AsynchronousSocketChannel
{
INetworkChannel INetworkChannel.Bind (SocketAddress address)
{
return Bind (address);
}
INetworkChannel INetworkChannel.SetOption (ISocketOption option, Java.Lang.Object value)
{
return SetOption (option, value);
}
}
}

#endif
17 changes: 17 additions & 0 deletions src/Mono.Android/Java.Nio.Channels/DatagramChannel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;
using Java.Net;

namespace Java.Nio.Channels
{
partial class DatagramChannel
{
INetworkChannel INetworkChannel.Bind (SocketAddress address)
{
return Bind (address);
}
INetworkChannel INetworkChannel.SetOption (ISocketOption option, Java.Lang.Object value)
{
return SetOption (option, value);
}
}
}
17 changes: 17 additions & 0 deletions src/Mono.Android/Java.Nio.Channels/ServerSocketChannel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;
using Java.Net;

namespace Java.Nio.Channels
{
partial class ServerSocketChannel
{
INetworkChannel INetworkChannel.Bind (SocketAddress address)
{
return Bind (address);
}
INetworkChannel INetworkChannel.SetOption (ISocketOption option, Java.Lang.Object value)
{
return SetOption (option, value);
}
}
}
17 changes: 17 additions & 0 deletions src/Mono.Android/Java.Nio.Channels/SocketChannel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
using System;
using Java.Net;

namespace Java.Nio.Channels
{
partial class SocketChannel
{
INetworkChannel INetworkChannel.Bind (SocketAddress address)
{
return Bind (address);
}
INetworkChannel INetworkChannel.SetOption (ISocketOption option, Java.Lang.Object value)
{
return SetOption (option, value);
}
}
}
3 changes: 1 addition & 2 deletions src/Mono.Android/Java.Util/Spliterators.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
#if ANDROID_24
#if false//ANDROID_24
using System;
using Android.Runtime;
using Java.Interop;
using Java.Util.Functions;

namespace Java.Util
{
// FIXME: this should not be required to be hand-bound.
public partial class Spliterators
{
public partial class AbstractSpliterator
Expand Down
1 change: 1 addition & 0 deletions src/Mono.Android/Mono.Android.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@
<Compile Include="Java.Nio\Buffer.cs" />
<Compile Include="Java.Nio\CharBuffer.cs" />
<Compile Include="Java.Nio\FileChannel.cs" />
<Compile Include="Java.Nio.Channels\*.cs" />
<Compile Include="Java.Security\IdentityScope.cs" />
<Compile Include="Java.Util\Spliterators.cs" />
<Compile Include="Java.Util.Concurrent.Atomic\AtomicInteger.cs" />
Expand Down
7 changes: 6 additions & 1 deletion src/Mono.Android/Mono.Android.targets
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
<UsingTask AssemblyFile="..\..\bin\Build$(Configuration)\xa-prep-tasks.dll" TaskName="Xamarin.Android.BuildTools.PrepTasks.ReplaceFileContents" />
<Import Project="..\..\build-tools\scripts\XAVersionInfo.targets" />
<Import Project="Mono.Android.projitems" />
<PropertyGroup>
<CscToolPath>..\..\packages\xamarin.android.csc.dim.0.1.2\tools\</CscToolPath>
<LangVersion>latest</LangVersion>
</PropertyGroup>
<ItemGroup>
<Compile Include="$(IntermediateOutputPath)AssemblyInfo.cs" />
</ItemGroup>
Expand Down Expand Up @@ -78,6 +82,7 @@
<PropertyGroup>
<Generator>"$(XAInstallPrefix)xbuild\Xamarin\Android\generator.exe"</Generator>
<_GenFlags>--public --product-version=7</_GenFlags>
<_DefaultInterfaceMethods>--default-interface-methods</_DefaultInterfaceMethods>
<_ApiLevel>--api-level=$(AndroidApiLevel)</_ApiLevel>
<_Out>-o "$(IntermediateOutputPath)mcw"</_Out>
<_Codegen>--codegen-target=XAJavaInterop1</_Codegen>
Expand All @@ -92,7 +97,7 @@
<_Dirs>--enumdir=$(IntermediateOutputPath)mcw</_Dirs>
</PropertyGroup>
<Exec
Command="$(ManagedRuntime) $(Generator) $(_GenFlags) $(_ApiLevel) $(_Out) $(_Codegen) $(_Fixup) $(_Enums1) $(_Enums2) $(_Versions) $(_Annotations) $(_Assembly) $(_TypeMap) $(_Dirs) $(_Api)"
Command="$(ManagedRuntime) $(Generator) $(_GenFlags) $(_ApiLevel) $(_Out) $(_Codegen) $(_DefaultInterfaceMethods) $(_Fixup) $(_Enums1) $(_Enums2) $(_Versions) $(_Annotations) $(_Assembly) $(_TypeMap) $(_Dirs) $(_Api)"
/>
<ItemGroup>
<Compile Include="$(IntermediateOutputPath)mcw\**\*.cs" />
Expand Down