[bgen] Implement support for using default interface members to bind protocols.

docs/objective-c-protocols.md

@@ -15,13 +15,13 @@ been possible to represent required members in a C# interface (any interface
 member would be required), but optional members were not possible until C#
 added support for default interface members in C# 8.
 
-We represent optional members in two ways:
+In the past (before .NET 9) we represented optional members in two ways:
 
 * As an extension method on the interface (useful when calling the optional member).
 * As an IDE feature that would show any optional members from an interface by
   typing 'override ...' in the text editor (useful when implementing an optional member).
 
-This has a few drawbacks:
+This had a few drawbacks:
 
 * There are no extension properties, so optional properties would have to be
   bound as a pair of GetProperty/SetProperty methods.
@@ -40,8 +40,9 @@ It's entirely possible to change a member from being required to being optional
 in Objective-C. Technically it's also a breaking change to do the opposite (make
 an optional member required), but Apple does it all the time.
 
-We've handled this by just not updating the binding until we're able to do
-breaking changes (which happens very rarely).
+Before .NET 9 we had no way of changing requiredness in the corresponding C#
+bindings, because it would be a breaking change. We would just not update the
+binding until we're able to do breaking changes (which happens very rarely).
 
 ### Static members
 
@@ -62,6 +63,53 @@ ignored them.
 
 ## Binding in C#
 
+### Optional/required members and changing requiredness
+
+Given the following API definition:
+
+```cs
+[Protocol]
+public interface Protocol {
+    [Abstract]
+    [Export ("requiredMethod")]
+    void RequiredMethod ();
+
+    [Export ("optionalMethod")]
+    void OptionalMethod ();
+}
+```
+
+we're 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 only difference between them is that the required method has a `[RequiredMember]`
+attribute, and the optional method as an `[OptionalMember]` attribute.
+
+This way it won't be a breaking change to make a required member optional, and
+vice versa.
+
+The downside is that the C# compiler won't enforce that required members are
+implemented (note that in many cases it's possible to not implement a required
+member in Objective-C - you'll get a compiler warning, but you may get away with
+it at runtime, depending on the code that uses your protocol implementation).
+
+In the future we plan to emit a warning at build time from our own build tools
+(linker steps), that lets the developer know about required members that
+haven't been implemented. It will be possible to either ignore these warnings,
+or make them errors.
+
 ### Static members
 
 Given the following API definition:
@@ -94,25 +142,31 @@ we're binding it like this:
 ```cs
 [Protocol ("Protocol")]
 public interface IProtocol : INativeObject {
+    [RequiredMember]
     [Export ("requiredStaticMethod")]
     public static void RequiredStaticMethod<T> () where T: NSObject, IProtocol { /* implementation */ }
 
+    [OptionalMember]
     [Export ("optionalStaticMethod")]
-    public static void OptionalStaticMethod<T> () where T: NSObject, IProtocol { /*  implementation */ }
+    public static void OptionalStaticMethod<T> () where T: NSObject, IProtocol { /* implementation */ }
 
     [Property ("RequiredStaticProperty")]
+    [RequiredMember]
     [Export ("requiredStaticProperty")]
     public static IntPtr GetRequiredStaticProperty<T> () where T: NSObject, IProtocol { /* implementation */ }
 
     [Property ("RequiredStaticProperty")]
+    [RequiredMember]
     [Export ("setRequiredStaticProperty:")]
     public static void SetRequiredStaticProperty<T> (IntPtr value) where T: NSObject, IProtocol { /* implementation */ }
 
     [Property ("OptionalStaticProperty")]
+    [OptionalMember]
     [Export ("optionalStaticProperty")]
     public static IntPtr GetOptionalStaticProperty<T> () where T: NSObject, IProtocol { /* implementation */ }
 
     [Property ("OptionalStaticProperty")]
+    [OptionalMember]
     [Export ("setOptionalStaticProperty:")]
     public static void SetOptionalStaticProperty<T> (IntPtr value) where T: NSObject, IProtocol { /* implementation */ }
 }
@@ -124,7 +178,7 @@ There are three points of interest here:
    member should be called.
 2. Properties have been turned into a pair of Get/Set methods - this is because
    properties can't have type arguments the way methods can.
-3. There's no difference between optional and required members.
+3. There are no differences between optional and required members.
 
 Example consuming code:
 
@@ -160,12 +214,15 @@ we're binding it like this:
 ```cs
 [Protocol ("Protocol")]
 public interface IProtocol : INativeObject {
+    [RequiredMember]
     [Export ("init")]
     public static T CreateInstance<T> () where T: NSObject, IProtocol { /* default implementation */ }
 
+    [OptionalMember]
     [Export ("initWithValue:")]
     public static T CreateInstance<T> () where T: NSObject, IProtocol { /* default implementation */ }
 
+    [OptionalMember]
     [Export ("initWithPlanet:")]
     public static T Create<T> () where T: NSObject, IProtocol { /* default implementation */ }
 }
@@ -202,3 +259,145 @@ class MyClass : NSObject, IMyProtocol {
     }
 }
 ```
+
+### Coping with C# quirks
+
+An interface member is not accesible from a variable typed as a class that
+implements the interface. This means that a variable must be cast to the
+interface before calling any members on it.
+
+Example:
+
+```cs
+interface I {
+    public void DoSomething () {}
+}
+class C : I {
+}
+class Program {
+    static void Main ()
+    {
+        var c = new C ();
+        c.DoSomething (); // this doesn't work: CS1061: 'C' does not contain a definition for 'DoSomething' and no accessible extension method 'DoSomething' accepting a first argument of type 'C' could be found (are you missing a using directive or an assembly reference?)
+        ((I) c).DoSomething (); // this works
+    }
+}
+```
+
+We improve this by inlining all protocol members in any implementing class.
+
+One complication is that there's currently no way to call the default
+interface implementation from the implementing class (see
+https://github.com/dotnet/csharplang/issues/2337), so we have to go through a
+static method.
+
+Given the following API definition:
+
+```cs
+[Protocol]
+public interface Protocol {
+    [Abstract]
+    [Export ("requiredMethod")]
+    void RequiredMethod ();
+
+    [Export ("optionalMethod")]
+    void OptionalMethod ();
+}
+
+[BaseType (NSObject)]
+public interface MyObject : Protocol {
+}
+```
+
+we're binding it like this:
+
+```cs
+[Protocol ("Protocol")]
+public interface IProtocol : INativeObject {
+    [RequiredMember]
+    [Export ("requiredMethod")]
+    public void RequiredMethod () { _RequireMethodImpl (Handle); }
+
+    [OptionalMember]
+    [Export ("optionalMethod")]
+    public void OptionalMethod () { _OptionalMethodImpl (Handle); }
+
+    internal static void _RequireMethodImpl (IntPtr handle)
+    {
+        /* default implementation */
+    }
+
+    internal static void _OptionalMethodImpl (IntPtr handle)
+    {
+        /* default implementation */
+    }
+}
+
+public class MyObject : NSObject, IProtocol {
+    public virtual void RequiredMethod ()
+    {
+        IProtocol._RequireMethodImpl (Handle); // just forward to the default implementation
+    }
+
+    public virtual void OptionalMethod ()
+    {
+        IProtocol._OptionalMethodImpl (Handle); // just forward to the default implementation
+    }
+}
+```
+
+## Backwards compatibility
+
+### Pre-NET 9 extension class
+
+Before .NET 9, we generated an extension class for optional members. This is no
+longer needed, but we still need to do it for existing protocols (to not break
+backwards compatibility).
+
+The Protocol attribute used by the generator will have a new property to reflect
+that the extension class is not needed anymore:
+
+```cs
+class ProtocolAttribute : Attribute
+{
+#if !XAMCORE_5_0 && GENERATOR
+    public ProtocolAttribute (bool mustBeBackwardsCompatible = true)
+    {
+        this.MustBeBackwardsCompatible = mustBeBackwardsCompatible;
+    }
+
+    public bool MustBeBackwardsCompatible { get; set; }
+#endif
+}
+```
+
+This property will default to true (that way we don't have to change existing
+code), and then the next time we can do an API break (the `XAMCORE_5_0` define),
+we'll remove the property since we no longer need to be backwards compatible.
+
+### Pre-NET 9 attributes
+
+Before .NET 9, we generated a ProtocolMember attribute on the interface for all
+members on the protocol, with enough information for our runtime to be able to
+do the right thing.
+
+This is no longer necessary, since we have all the required information on the
+interface members.
+
+We'll keep generating these attributes for protocols defined with
+`MustBeBackwardsCompatible`.
+
+## Notable consequences
+
+Since every member in a C# interface binding a protocol will have a default
+implementation, the compiler won't enforce required members anymore.
+
+As a result, the IDE (at least Visual Studio for Mac) won't show a quick action
+to implement these interface members. However, Intellisense will show up if you
+do this:
+
+```cs
+public class MyObject : IProtocol {
+    void IProtocol.[list of members on IProtocol will show up]
+}
+```

src/Foundation/ProtocolAttribute.cs

@@ -49,6 +49,15 @@ public string? FormalSince {
 				informal_until = value;
 			}
 		}
+
+#if !XAMCORE_5_0
+		/// <summary>
+		///	  <para>This property indicates whether the binding generator will generate backwards-compatible code for the protocol in question.</para>
+		///	  <para>In particular, if this property is true, then the binding generator will generate extension methods for optional members and <see cref="ProtocolMemberAttribute" /> attributes on the protocol interface for all protocol members.</para>
+		/// </summary>
+		/// <remarks>This property is by default true.</remarks>
+		public bool BackwardsCompatibleCodeGeneration { get; set; } = true;
+#endif
 	}
 
 	[AttributeUsage (AttributeTargets.Interface, AllowMultiple = true)]
@@ -72,4 +81,17 @@ public ProtocolMemberAttribute () { }
 		public string? SetterSelector { get; set; }
 		public ArgumentSemantic ArgumentSemantic { get; set; }
 	}
+
+	/// <summary>This attribute is added by the binding generator to members that bind required protocol members.</summary>
+	[AttributeUsage (AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = false)]
+	public sealed class RequiredMemberAttribute : Attribute {
+	}
+
+	// There's already an OptionalAttribute in System.Runtime.InteropServices, so I went with
+	// "OptionalMemberAttribute" - and in that case it prefered "RequiredMemberAttribute" instead
+	// of "RequiredAttribute" just to keep the symmetry.
+	/// <summary>This attribute is added by the binding generator to members that bind optional protocol members.</summary>
+	[AttributeUsage (AttributeTargets.Method | AttributeTargets.Property, AllowMultiple = false)]
+	public sealed class OptionalMemberAttribute : Attribute {
+	}
 }

src/ObjCRuntime/Registrar.cs

@@ -346,8 +346,9 @@ public bool Add (ObjCMethod method, ref List<Exception> exceptions)
 				if (!method.IsPropertyAccessor && !method.DeclaringType.IsProtocol)
 					Registrar.VerifyInSdk (ref exceptions, method);
 
-				rv = AddToMap (method, ref exceptions);
-				Methods.Add (method);
+				rv = AddToMap (method, ref exceptions, out var alreadyAdded);
+				if (!alreadyAdded)
+					Methods.Add (method);
 				return rv;
 			}
 
@@ -427,11 +428,21 @@ public bool TryGetMember (string selector, bool is_static, out ObjCMember member
 				return Map.TryGetValue (selector, out member);
 			}
 
-			bool AddToMap (ObjCMember member, ref List<Exception> exceptions)
+			bool AddToMap (ObjCMember member, ref List<Exception> exceptions, out bool alreadyAdded)
 			{
 				ObjCMember existing;
 				bool rv = true;
+
+				alreadyAdded = false;
+
 				if (TryGetMember (member.Selector, member.IsNativeStatic, out existing)) {
+					if (existing is ObjCMethod existingMethod && member is ObjCMethod method && object.ReferenceEquals (method.Method, existingMethod.Method)) {
+						// This can happen if we try to register:
+						// * A property declared on a type (we register the getter + setter)
+						// * The getter or setter matches the signature of an exported method from a protocol interface, in which case we'll try to export again.
+						alreadyAdded = true;
+						return true;
+					}
 					if (existing.IsImplicit) {
 						AddException (ref exceptions, CreateException (4141, member, Errors.MT4141, member.Selector, Registrar.GetTypeFullName (Type), Registrar.GetMemberName (member)));
 					} else {

src/Resources.resx

@@ -616,4 +616,9 @@
 	<data name="BI1119" xml:space="preserve">
 		<value>Internal error: found the same type ({0}) in multiple assemblies ({1} and {2}). Please file a bug report (https://github.com/xamarin/xamarin-macios/issues/new) with a test case.</value>
 	</data>
+
+	<data name="BI1120" xml:space="preserve">
+		<value>The type '{0}' is trying to inline the methods binding the selector '{1}' from the protocols '{2}' and '{3}', using methods with different signatures ('{4}' vs '{5}'). These methods will be ignored.</value>
+	</data>
+
 </root>