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.

Sign up for GitHub

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

Support overloading Objective-C methods based on static/instance. #2607

Merged
merged 2 commits into from
Sep 7, 2017
Merged

Support overloading Objective-C methods based on static/instance. #2607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
857ebd2
Support overloading Objective-C methods based on static/instance.
rolfbjarne Sep 6, 2017
4f42e55
[monotouch-test] Fix comment.
rolfbjarne Sep 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion runtime/delegates.t4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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" },
Expand Down
4 changes: 2 additions & 2 deletions runtime/trampolines-invoke.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions runtime/xamarin/runtime.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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
Expand Down
14 changes: 7 additions & 7 deletions src/ObjCRuntime/DynamicRegistrar.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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)
Expand All @@ -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);

Expand All @@ -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;

Expand Down
43 changes: 38 additions & 5 deletions src/ObjCRuntime/Registrar.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ internal class ObjCType {
public List<ObjCMethod> Methods;
public List<ObjCProperty> Properties;

public Dictionary<string, ObjCMember> Map = new Dictionary<string, ObjCMember> ();
Dictionary<string, ObjCMember> Map = new Dictionary<string, ObjCMember> ();

ObjCType superType;

Expand Down Expand Up @@ -235,8 +235,9 @@ public void Add (ObjCField field, ref List<Exception> exceptions)
{
// Check if there are any base types with the same property.
var base_type = BaseType;
var fieldNameInDictionary = (field.IsStatic ? "+" : "-") + field.Name;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to have the potential to create a lot of strings. Could we (later) try to:

  • lookup different static/instance collections ? or
  • only prepend "+" for static (and leave the instance without a sign) ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose the implementation that I thought would have the lowest minimum memory requirements.

  • Having two different dictionaries, one for static and one for instance, would use more memory since there would be two dictionaries instead of one.
  • The strings are transient, the GC will collect them, and as such they don't contribute to the minimum memory requirements.
  • I could use a different key type (Dictionary<Tuple<string, bool>, ObjCMember for instance), but this means AOT-compiling a Dictionary with a struct as a key, which would increase code size (unless we already AOT-compile such a dictionary for something else).

I considered prepending + for static methods only, but then I thought of this (admittedly pathological) case:

[Export ("+foo")]
public void Foo () {}

which won't work with the static registrar, but works with the dynamic registrar (which is a use-case for Xamarin.Mac).

Finally I came to the conclusion I might be able to remove the whole problem when I make the static registrar do more at build time (see bug #56022).

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;
}
Expand All @@ -248,7 +249,7 @@ public void Add (ObjCField field, ref List<Exception> exceptions)
if (Fields == null)
Fields = new Dictionary<string, ObjCField> ();
// 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<Exception> exceptions)
Expand Down Expand Up @@ -332,11 +333,20 @@ void VerifyIsNotKeyword (ref List<Exception> 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<Exception> 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 {
Expand All @@ -345,7 +355,7 @@ bool AddToMap (ObjCMember member, ref List<Exception> exceptions)
rv = false;
}

Map [member.Selector] = member;
Map [(member.IsNativeStatic ? "+" : "-") + member.Selector] = member;
return rv;
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -751,6 +768,10 @@ public bool IsStatic {
set { is_static = value; }
}

public override bool IsNativeStatic {
get { return IsStatic; }
}

public ObjCProperty () : base ()
{
}
Expand All @@ -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) {}
Expand Down Expand Up @@ -1618,6 +1649,7 @@ ObjCType RegisterTypeUnsafe (TType type, ref List<Exception> exceptions)
FieldType = "XamarinObject",// "^v", // void*
Name = "__monoObjectGCHandle",
IsPrivate = SupportsModernObjectiveC,
IsStatic = false,
}, ref exceptions);
}
#endif
Expand Down Expand Up @@ -1727,6 +1759,7 @@ ObjCType RegisterTypeUnsafe (TType type, ref List<Exception> exceptions)
#endif
FieldType = "@",
IsProperty = true,
IsStatic = IsStatic (property),
}, ref exceptions);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/ObjCRuntime/Runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions tests/monotouch-test/ObjCRuntime/RegistrarTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2529,4 +2529,15 @@ public void WithUserDelegateTypeAttribute ()
block.CleanupBlock ();
}
}

[Preserve]
class OverloadByStaticity : NSObject
{
// 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 () { }
}
}