From 857ebd294a19f373ebed670f996a96b75a8090cc Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Wed, 6 Sep 2017 13:18:23 +0200 Subject: [PATCH 1/2] Support overloading Objective-C methods based on static/instance. Two Objective-C methods can be named identically as long as one is static and the other instance. We must support this since Apple did just this (in the NSItemProviderReading / NSItemProviderWriting protocols). We solve it by prepending a '+' or '-' to the selector when hashing it (to determine selector uniqueness, and to look the method up again at runtime). --- runtime/delegates.t4 | 4 +- runtime/trampolines-invoke.m | 4 +- runtime/xamarin/runtime.h | 4 +- src/ObjCRuntime/DynamicRegistrar.cs | 14 +++--- src/ObjCRuntime/Registrar.cs | 43 ++++++++++++++++--- src/ObjCRuntime/Runtime.cs | 8 ++-- .../ObjCRuntime/RegistrarTest.cs | 11 +++++ 7 files changed, 67 insertions(+), 21 deletions(-) diff --git a/runtime/delegates.t4 b/runtime/delegates.t4 index 8f311a3d0b40..f56fb3114174 100644 --- a/runtime/delegates.t4 +++ b/runtime/delegates.t4 @@ -71,7 +71,8 @@ new XDelegate ("MethodDescription", "UnmanagedMethodDescription", "xamarin_get_method_for_selector", "Class", "IntPtr", "cls", - "SEL", "IntPtr", "sel" + "SEL", "IntPtr", "sel", + "bool", "bool", "is_static" ) { WrappedManagedFunction = "GetMethodForSelector" }, new XDelegate ("MonoObject *", "IntPtr", "xamarin_get_nsobject", @@ -140,6 +141,7 @@ new XDelegate ("MethodDescription", "UnmanagedMethodDescription", "xamarin_get_method_and_object_for_selector", "Class", "IntPtr", "cls", "SEL", "IntPtr", "sel", + "bool", "bool", "is_static", "id", "IntPtr", "obj", "MonoObject **", "ref IntPtr", "mthis" ) { WrappedManagedFunction = "GetMethodAndObjectForSelector" }, diff --git a/runtime/trampolines-invoke.m b/runtime/trampolines-invoke.m index 5c52a83ccc28..28aa4db00fd6 100644 --- a/runtime/trampolines-invoke.m +++ b/runtime/trampolines-invoke.m @@ -88,9 +88,9 @@ int mofs = 0; if (is_ctor || is_static) { - desc = xamarin_get_method_for_selector ([self class], sel, &exception_gchandle); + desc = xamarin_get_method_for_selector ([self class], sel, is_static, &exception_gchandle); } else { - desc = xamarin_get_method_and_object_for_selector ([self class], sel, self, &mthis, &exception_gchandle); + desc = xamarin_get_method_and_object_for_selector ([self class], sel, is_static, self, &mthis, &exception_gchandle); } if (exception_gchandle != 0) goto exception_handling; diff --git a/runtime/xamarin/runtime.h b/runtime/xamarin/runtime.h index e79af184a479..32affb4bee21 100644 --- a/runtime/xamarin/runtime.h +++ b/runtime/xamarin/runtime.h @@ -225,7 +225,7 @@ MonoObject* xamarin_get_class (Class ptr, guint32 *exception_gchandle) MonoObject* xamarin_get_selector (SEL ptr, guint32 *exception_gchandle); Class xamarin_get_class_handle (MonoObject *obj, guint32 *exception_gchandle); SEL xamarin_get_selector_handle (MonoObject *obj, guint32 *exception_gchandle); -MethodDescription xamarin_get_method_for_selector (Class cls, SEL sel, guint32 *exception_gchandle); +MethodDescription xamarin_get_method_for_selector (Class cls, SEL sel, bool is_static, guint32 *exception_gchandle); bool xamarin_has_nsobject (id obj, guint32 *exception_gchandle); MonoObject* xamarin_get_nsobject (id obj, guint32 *exception_gchandle); id xamarin_get_handle_for_inativeobject (MonoObject *obj, guint32 *exception_gchandle); @@ -239,7 +239,7 @@ MonoObject* xamarin_get_nsobject_with_type (id obj, void *type, int32_t * void xamarin_dispose (MonoObject *mobj, guint32 *exception_gchandle); bool xamarin_is_parameter_transient (MonoReflectionMethod *method, int parameter /* 0-based */, guint32 *exception_gchandle); bool xamarin_is_parameter_out (MonoReflectionMethod *method, int parameter /* 0-based */, guint32 *exception_gchandle); -MethodDescription xamarin_get_method_and_object_for_selector (Class cls, SEL sel, id self, MonoObject **mthis, guint32 *exception_gchandle); +MethodDescription xamarin_get_method_and_object_for_selector (Class cls, SEL sel, bool is_static, id self, MonoObject **mthis, guint32 *exception_gchandle); guint32 xamarin_create_product_exception_for_error (int code, const char *message, guint32 *exception_gchandle); #ifdef __cplusplus diff --git a/src/ObjCRuntime/DynamicRegistrar.cs b/src/ObjCRuntime/DynamicRegistrar.cs index cfe0b8cbb46d..63e499afa4be 100644 --- a/src/ObjCRuntime/DynamicRegistrar.cs +++ b/src/ObjCRuntime/DynamicRegistrar.cs @@ -840,10 +840,10 @@ public void AddCustomType (Type type) custom_type_map [type] = null; } - public UnmanagedMethodDescription GetMethodDescriptionAndObject (Type type, IntPtr selector, IntPtr obj, ref IntPtr mthis) + public UnmanagedMethodDescription GetMethodDescriptionAndObject (Type type, IntPtr selector, bool is_static, IntPtr obj, ref IntPtr mthis) { var sel = new Selector (selector); - var res = GetMethodNoThrow (type, type, sel.Name); + var res = GetMethodNoThrow (type, type, sel.Name, is_static); if (res == null) throw ErrorHelper.CreateError (8006, "Failed to find the selector '{0}' on the type '{1}'", sel.Name, type.FullName); @@ -887,10 +887,10 @@ internal static MethodInfo FindClosedMethod (Type closed_type, MethodBase open_m throw ErrorHelper.CreateError (8003, "Failed to find the closed generic method '{0}' on the type '{1}'.", open_method.Name, closed_type.FullName); } - public UnmanagedMethodDescription GetMethodDescription (Type type, IntPtr selector) + public UnmanagedMethodDescription GetMethodDescription (Type type, IntPtr selector, bool is_static) { var sel = new Selector (selector); - var res = GetMethodNoThrow (type, type, sel.Name); + var res = GetMethodNoThrow (type, type, sel.Name, is_static); if (res == null) throw ErrorHelper.CreateError (8006, "Failed to find the selector '{0}' on the type '{1}'", sel.Name, type.FullName); if (type.IsGenericType && res.Method is ConstructorInfo) @@ -899,7 +899,7 @@ public UnmanagedMethodDescription GetMethodDescription (Type type, IntPtr select return res.MethodDescription.GetUnmanagedDescription (); } - ObjCMethod GetMethodNoThrow (Type original_type, Type type, string selector) + ObjCMethod GetMethodNoThrow (Type original_type, Type type, string selector, bool is_static) { var objcType = RegisterType (type); @@ -908,8 +908,8 @@ ObjCMethod GetMethodNoThrow (Type original_type, Type type, string selector) ObjCMember member = null; - if (type.BaseType != typeof (object) && !objcType.Map.TryGetValue (selector, out member)) - return GetMethodNoThrow (original_type, type.BaseType, selector); + if (type.BaseType != typeof (object) && !objcType.TryGetMember (selector, is_static, out member)) + return GetMethodNoThrow (original_type, type.BaseType, selector, is_static); var method = member as ObjCMethod; diff --git a/src/ObjCRuntime/Registrar.cs b/src/ObjCRuntime/Registrar.cs index 274b121ca7a1..6c02135f643e 100644 --- a/src/ObjCRuntime/Registrar.cs +++ b/src/ObjCRuntime/Registrar.cs @@ -148,7 +148,7 @@ internal class ObjCType { public List Methods; public List Properties; - public Dictionary Map = new Dictionary (); + Dictionary Map = new Dictionary (); ObjCType superType; @@ -235,8 +235,9 @@ public void Add (ObjCField field, ref List exceptions) { // Check if there are any base types with the same property. var base_type = BaseType; + var fieldNameInDictionary = (field.IsStatic ? "+" : "-") + field.Name; while (base_type != null) { - if (base_type.Fields != null && base_type.Fields.ContainsKey (field.Name)) { + if (base_type.Fields != null && base_type.Fields.ContainsKey (fieldNameInDictionary)) { // Maybe we should warn here? Not sure if this is something bad or not. return; } @@ -248,7 +249,7 @@ public void Add (ObjCField field, ref List exceptions) if (Fields == null) Fields = new Dictionary (); // Do fields and methods live in the same objc namespace? // AddToMap (field, errorIfExists); - Fields.Add (field.Name, field); + Fields.Add (fieldNameInDictionary, field); } public bool Add (ObjCMethod method, ref List exceptions) @@ -332,11 +333,20 @@ void VerifyIsNotKeyword (ref List exceptions, ObjCProperty property) AddException (ref exceptions, CreateException (4164, property, "Cannot export the property '{0}' because its selector '{1}' is an Objective-C keyword. Please use a different name.", property.Name, property.Selector)); } + public bool TryGetMember (string selector, bool is_static, out ObjCMember member) + { + if (is_static) + selector = "+" + selector; + else + selector = "-" + selector; + return Map.TryGetValue (selector, out member); + } + bool AddToMap (ObjCMember member, ref List exceptions) { ObjCMember existing; bool rv = true; - if (Map.TryGetValue (member.Selector, out existing)) { + if (TryGetMember (member.Selector, member.IsNativeStatic, out existing)) { if (existing.IsImplicit) { AddException (ref exceptions, CreateException (4141, member, "Cannot register the selector '{0}' on the member '{1}.{2}' because Xamarin.iOS implicitly registers this selector.", member.Selector, Registrar.GetTypeFullName (Type), Registrar.GetMemberName (member))); } else { @@ -345,7 +355,7 @@ bool AddToMap (ObjCMember member, ref List exceptions) rv = false; } - Map [member.Selector] = member; + Map [(member.IsNativeStatic ? "+" : "-") + member.Selector] = member; return rv; } @@ -458,6 +468,7 @@ public string Selector { } public abstract string FullName { get; } + public abstract bool IsNativeStatic { get; } public virtual bool IsImplicit { get { return false; } } protected string ToSignature (TType type, ref bool success) @@ -571,6 +582,12 @@ public bool IsStatic { set { is_static = value; } } + public override bool IsNativeStatic { + get { + return IsStatic && !IsCategoryInstance; + } + } + public bool IsCategoryInstance { get { return IsCategory && Registrar.HasThisAttribute (Method); @@ -751,6 +768,10 @@ public bool IsStatic { set { is_static = value; } } + public override bool IsNativeStatic { + get { return IsStatic; } + } + public ObjCProperty () : base () { } @@ -771,12 +792,22 @@ internal class ObjCField : ObjCMember { #endif public string FieldType; public bool IsProperty; + bool is_static; public override string FullName { get { return Registrar.GetTypeFullName (DeclaringType.Type) + "." + Name; } } + + public bool IsStatic { + get { return is_static; } + set { is_static = value; } + } + + public override bool IsNativeStatic { + get { return IsStatic; } + } } protected virtual void OnRegisterType (ObjCType type) {} @@ -1618,6 +1649,7 @@ ObjCType RegisterTypeUnsafe (TType type, ref List exceptions) FieldType = "XamarinObject",// "^v", // void* Name = "__monoObjectGCHandle", IsPrivate = SupportsModernObjectiveC, + IsStatic = false, }, ref exceptions); } #endif @@ -1727,6 +1759,7 @@ ObjCType RegisterTypeUnsafe (TType type, ref List exceptions) #endif FieldType = "@", IsProperty = true, + IsStatic = IsStatic (property), }, ref exceptions); } } diff --git a/src/ObjCRuntime/Runtime.cs b/src/ObjCRuntime/Runtime.cs index 504827c8f664..3679752bffde 100644 --- a/src/ObjCRuntime/Runtime.cs +++ b/src/ObjCRuntime/Runtime.cs @@ -556,10 +556,10 @@ static IntPtr GetSelectorHandle (IntPtr sel) return ((Selector) ObjectWrapper.Convert (sel)).Handle; } - static UnmanagedMethodDescription GetMethodForSelector (IntPtr cls, IntPtr sel) + static UnmanagedMethodDescription GetMethodForSelector (IntPtr cls, IntPtr sel, bool is_static) { // This is called by the old registrar code. - return Registrar.GetMethodDescription (Class.Lookup (cls), sel); + return Registrar.GetMethodDescription (Class.Lookup (cls), sel, is_static); } static IntPtr GetNSObjectWrapped (IntPtr ptr) @@ -675,9 +675,9 @@ static bool IsParameterOut (IntPtr info, int parameter) return parameters [parameter].IsOut; } - static UnmanagedMethodDescription GetMethodAndObjectForSelector (IntPtr klass, IntPtr sel, IntPtr obj, ref IntPtr mthis) + static UnmanagedMethodDescription GetMethodAndObjectForSelector (IntPtr klass, IntPtr sel, bool is_static, IntPtr obj, ref IntPtr mthis) { - return Registrar.GetMethodDescriptionAndObject (Class.Lookup (klass), sel, obj, ref mthis); + return Registrar.GetMethodDescriptionAndObject (Class.Lookup (klass), sel, is_static, obj, ref mthis); } static int CreateProductException (int code, string msg) diff --git a/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs b/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs index 21d8bb87cd4e..d0a3490b6407 100644 --- a/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs +++ b/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs @@ -2529,4 +2529,15 @@ public void WithUserDelegateTypeAttribute () block.CleanupBlock (); } } + + [Preserve] + class OverloadByStaticity : NSObject + { + // Two methods/properties can + [Export ("method")] + public void InstanceMethod () { } + + [Export ("method")] + public static void StaticMethod () { } + } } From 4f42e552f2ff76ab644e4ca0d9ef48862368d77a Mon Sep 17 00:00:00 2001 From: Rolf Bjarne Kvinge Date: Thu, 7 Sep 2017 10:18:15 +0200 Subject: [PATCH 2/2] [monotouch-test] Fix comment. --- tests/monotouch-test/ObjCRuntime/RegistrarTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs b/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs index d0a3490b6407..de5f2bbb7023 100644 --- a/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs +++ b/tests/monotouch-test/ObjCRuntime/RegistrarTest.cs @@ -2533,11 +2533,11 @@ public void WithUserDelegateTypeAttribute () [Preserve] class OverloadByStaticity : NSObject { - // Two methods/properties can + // Two Objective-C methods can have the same selector if one is static and the other instance. [Export ("method")] public void InstanceMethod () { } [Export ("method")] - public static void StaticMethod () { } + public static void StaticMethod () { } } }