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

.NET: create a spec for how to add support for default interface members #13294

Closed
rolfbjarne opened this issue Nov 5, 2021 · 2 comments · Fixed by #20681
Closed

.NET: create a spec for how to add support for default interface members #13294

rolfbjarne opened this issue Nov 5, 2021 · 2 comments · Fixed by #20681
Labels
dotnet An issue or pull request related to .NET (6) enhancement The issue or pull request is an enhancement
Projects
Milestone

Comments

@rolfbjarne
Copy link
Member

rolfbjarne commented Nov 5, 2021

We've had support for default interface methods for a while, and the plan is to use them to improve our support for Objective-C protocols.

However, we haven't spec'ed out exactly how we're going to do that.

We should do this before .NET 6, because we can make breaking changes now: #13087 - this doesn't mean that adding support for default interface methods will include breaking changes, but before that train leaves we should spec it out to determine if we might want to do breaking changes (and implement them).

One major goal we should strive for would be for a method or property to go back and forth between being optional and required, and it shouldn't be a breaking API change either way.

This probably means that the optional/required state of a protocol member should be stored in an attribute on the method/property.


Need to look into #20265 as well as part of this work.

@rolfbjarne rolfbjarne added dotnet An issue or pull request related to .NET (6) dotnet-pri1 .NET 6: important for stable release labels Nov 5, 2021
@rolfbjarne rolfbjarne added this to the .NET 6 milestone Nov 5, 2021
@rolfbjarne rolfbjarne added this to Pri 1 - Not Started in .NET 6 Nov 5, 2021
@rolfbjarne rolfbjarne changed the title .NET: create a spec for how to add support for default interface methods .NET: create a spec for how to add support for default interface members Nov 5, 2021
@chamons
Copy link
Contributor

chamons commented Jan 25, 2022

If we go down this route, this is an old (multiple years) branch with some work I did originally before we decided DIM wasn't helpful - https://github.com/chamons/xamarin-macios/commits/default_impl_prototype

@rolfbjarne rolfbjarne modified the milestones: .NET 6, .NET 7 Feb 15, 2022
@rolfbjarne rolfbjarne removed this from Pri 1 - Not Started in .NET 6 Feb 15, 2022
@rolfbjarne rolfbjarne added enhancement The issue or pull request is an enhancement and removed dotnet-pri1 .NET 6: important for stable release labels Feb 15, 2022
@rolfbjarne rolfbjarne modified the milestones: .NET 7, .NET 8 Aug 26, 2022
@rolfbjarne rolfbjarne added this to Default interface members in .NET 8 - Themes Aug 26, 2022
@rolfbjarne rolfbjarne removed this from Default interface members in .NET 8 - Themes Sep 5, 2023
@rolfbjarne rolfbjarne added this to Features in .NET 9 Sep 5, 2023
@rolfbjarne rolfbjarne modified the milestones: .NET 8, .NET 9 Nov 9, 2023
@rolfbjarne
Copy link
Member Author

This turned out a bit annoying to implement, because there's no way to call the default implementation from an implementing class.

This feature was cut from C# 8, supposedly to return in C# 9, but that never happened.

rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Jun 4, 2024
…protocols.

Given the following API definition:

```cs
[Protocol]
public interface Protocol {
    [Abstract]
    [Export ("requiredMethod")]
    void RequiredMethod ();

    [Export ("optionalMethod")]
    void OptionalMethod ();
}
```

we're now binding it like this:

```cs
[Protocol ("Protocol")]
public interface IProtocol : INativeObject {
    [RequiredMember]
    [Export ("requiredMethod")]
    public void RequiredMethod () { /* default implementation */ }

    [OptionalMember]
    [Export ("optionalMethod")]
    public void OptionalMethod () { /* default implementation */ }
}
```

The main difference from before is that the only difference between required
and optional members is the [RequiredMember]/[OptionalMember] attributes.

This has one major advantage: it's now possible to switch a member from being
required to being optional, or vice versa, without breaking neither source nor
binary compatibility.

It also improves intellisense for optional members. In the past optional
members were implemented using extension methods, which were not very
discoverable when you were supposed to implement a protocol in your own class.

The main downside is that the C# compiler won't enforce developers to
implement required protocol members (which is a necessary side effect of the
fact that we want to be able to switch members between being required and
optional without breaking compatibility). If this turns out to be a problem,
we can implement a custom source analyzer and/or linker step that detects
missing implementations and issue warnings/errors.

Also:

* Add numerous tests.
* Add documentation.
* Handle numerous corner cases, which are documented in code and docs.

Fixes xamarin#13294.
rolfbjarne added a commit that referenced this issue Jun 7, 2024
…protocols. (#20681)

Given the following API definition:

```cs
[Protocol]
public interface Protocol {
    [Abstract]
    [Export ("requiredMethod")]
    void RequiredMethod ();

    [Export ("optionalMethod")]
    void OptionalMethod ();
}
```

we're now binding it like this:

```cs
[Protocol ("Protocol")]
public interface IProtocol : INativeObject {
    [RequiredMember]
    [Export ("requiredMethod")]
    public void RequiredMethod () { /* default implementation */ }

    [OptionalMember]
    [Export ("optionalMethod")]
    public void OptionalMethod () { /* default implementation */ }
}
```

The main difference from before is that the only difference between required
and optional members is the [RequiredMember]/[OptionalMember] attributes.

This has one major advantage: it's now possible to switch a member from being
required to being optional, or vice versa, without breaking neither source nor
binary compatibility.

It also improves intellisense for optional members. In the past optional
members were implemented using extension methods, which were not very
discoverable when you were supposed to implement a protocol in your own class.

The main downside is that the C# compiler won't enforce developers to
implement required protocol members (which is a necessary side effect of the
fact that we want to be able to switch members between being required and
optional without breaking compatibility). If this turns out to be a problem,
we can implement a custom source analyzer and/or linker step that detects
missing implementations and issue warnings/errors.

This PR also:

* Adds numerous tests.
* Updates the requiredness of a few members in Metal to test that it works as
  expected.
* Adds documentation.
* Handles numerous corner cases, which are documented in code and docs.

This PR is probably best reviewed commit-by-commit.

Fixes #13294.
.NET 9 automation moved this from Features to Done Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotnet An issue or pull request related to .NET (6) enhancement The issue or pull request is an enhancement
Projects
2 participants