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

[generator] Add support for skipInterfaceMethods. #1265

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

jpobst
Copy link
Contributor

@jpobst jpobst commented Oct 11, 2024

Context: #1264

The scenario described in #1264 seems very complex to fix, as we would have to determine if each interface method has a compatible DIM on any other interface in scope.

As a workaround, we can mimic skipInvokerMethods with a new skipInterfaceMethods metadata that allows us to suppress generation of an abstract method in an abstract class that implements the interface.

The example in #1264 would be fixed with:

<attr
  path="/api/package[@name='kotlin.collections']/class[@name='AbstractList']"
  name="skipInterfaceMethods">
    java/util/SequencedCollection.reversed()Ljava/util/SequencedCollection;
</attr>

The format is the same as skipInvokerMethods.

@jpobst jpobst marked this pull request as ready for review October 11, 2024 19:49
@jpobst jpobst requested a review from jonpryor October 11, 2024 19:49
@jonpryor
Copy link
Member

Context: https://github.com/dotnet/java-interop/issues/1264
Context: 73ebad2496a0d6df251fd24e216248da70e3fea9
Context: a7e09b7be4dc9dbc1c1a65b5f2c1e8fcf028b209
Context: https://github.com/dotnet/android/commit/00256b6912f80e36c2a5d31046eb8c32691ed221

Consider [`java.util.List<E>`][0], which implements
[`java.util.SequencedCollection<E>`][1] and provides an interface
default method to implement `SequencedCollection<E>.reversed()`:

	// Java
	package java.util;
	public /* partial */ interface SequencedCollection<E> implements Collection<T> {
	  SequencedCollection<E> reverse ();
	}
	public /* partial */ interface List<E> implements SequencedCollection<E> {
	  default List<E> reversed() { … }
	}

We then *bind* the above, along with some manual fixups in
dotnet/android@00256b69:

	// C# bindings
	public partial interface ISequencedCollection {
	  [Register ("reversed", "()Ljava/util/SequencedCollection;", "GetReversedHandler:Java.Util.ISequencedCollectionInvoker, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 35)]
	  ISequencedCollection? Reversed ();
	}

	public partial interface IList : ISequencedCollection {
	  [Register ("reversed", "()Ljava/util/List;", "GetReversedHandler:Java.Util.IList, Mono.Android, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", ApiSince = 35)]
	  unsafe Java.Util.ISequencedCollection Java.Util.ISequencedCollection.Reversed () => …
	}

Next, we try to bind [androidx.paging:paging-common-android][2].
The [`androidx.paging.ItemSnapshotList`][3] inherits from
[`kotlin.collections.AbstractList`][4]:

	// Kotlin
	package kotlin.collections;
	public abstract /* partial */ class AbstractList<out E> protected constructor() :
	  kotlin.collections.AbstractCollection<E>(), java.util.List<E> {
	}

In a scenario that rhymes with 73ebad24, `generator` with
`Mono.Android.dll` (bindings for `Java.Util.IList`) cannot determine
that the `Kotlin.Collections.AbstractList` type has a `Reversed()`
method (via `Java.Util.IList`), and so re-declares it:

	// C#
	public partial class AbstractList : Kotlin.Collections.AbstractCollection, Java.Util.IList {
	  public abstract ISequencedCollection Reversed ();
	}

We fear that a complete fix for this may be complicated, as we would
have to determine if each interface method has a compatible default
interface method on any other interface in scope.

Introduce a new `skipInterfaceMethods` metadata, with identical
semantics to `skipInvokerMethods` (73ebad24), to allow specifying
`interface`-derived methods which should be *skipped* when generating
bindings:

	<attr
	    path="/api/package[@name='kotlin.collections']/class[@name='AbstractList']"
	    name="skipInterfaceMethods">
	  java/util/SequencedCollection.reversed()Ljava/util/SequencedCollection;
	</attr>

The above fragment would prevent `Reversed()` from being declared in
the binding for `Kotlin.Collections.AbstractList`.

[0]: https://developer.android.com/reference/java/util/List
[1]: https://developer.android.com/reference/java/util/SequencedCollection
[2]: https://maven.google.com/web/index.html?q=paging#androidx.paging:paging-common-android
[3]: https://developer.android.com/reference/androidx/paging/ItemSnapshotList
[4]: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.collections/-abstract-list/

@jonpryor jonpryor merged commit b656f7f into main Oct 17, 2024
4 checks passed
@jonpryor jonpryor deleted the skip-interface-methods branch October 17, 2024 19:25
jonpryor added a commit to dotnet/android that referenced this pull request Oct 22, 2024
Changes: dotnet/java-interop@9d99723...2a1e180

  * dotnet/java-interop@2a1e1800: [generator] Fix StackOverflow when copying DIM via private interfaces (dotnet/java-interop#1261)
  * dotnet/java-interop@f863351e: [generator] Only use `[JniTypeSignatureAttribute]` for `JavaInterop1` (dotnet/java-interop#1266)
  * dotnet/java-interop@56cfab93: [generator] Fix exception caused by incorrect nested type name. (dotnet/java-interop#1267)
  * dotnet/java-interop@b656f7f5: [generator] Add support for `skipInterfaceMethods` (dotnet/java-interop#1265)
jonpryor added a commit to dotnet/android that referenced this pull request Oct 22, 2024
Changes: dotnet/java-interop@9d99723...2a1e180

  * dotnet/java-interop@2a1e1800: [generator] Fix StackOverflow when copying DIM via private interfaces (dotnet/java-interop#1261)
  * dotnet/java-interop@f863351e: [generator] Only use `[JniTypeSignatureAttribute]` for `JavaInterop1` (dotnet/java-interop#1266)
  * dotnet/java-interop@56cfab93: [generator] Fix exception caused by incorrect nested type name. (dotnet/java-interop#1267)
  * dotnet/java-interop@b656f7f5: [generator] Add support for `skipInterfaceMethods` (dotnet/java-interop#1265)
@github-actions github-actions bot locked and limited conversation to collaborators Nov 17, 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.

2 participants