From 58d5df0fe731e3fe27f71316869ef5aad932e96f Mon Sep 17 00:00:00 2001 From: Martin Molinero Date: Thu, 4 Feb 2021 20:36:45 -0300 Subject: [PATCH] MethodBinder implicit resolution --- src/embed_tests/TestMethodBinder.cs | 25 ++--- src/runtime/classobject.cs | 6 +- src/runtime/constructorbinding.cs | 5 +- src/runtime/converter.cs | 51 +++++++-- src/runtime/indexer.cs | 10 +- src/runtime/methodbinder.cs | 168 +++++++++++++--------------- src/runtime/methodobject.cs | 6 +- 7 files changed, 148 insertions(+), 123 deletions(-) diff --git a/src/embed_tests/TestMethodBinder.cs b/src/embed_tests/TestMethodBinder.cs index 010e3d2af..3feaea917 100644 --- a/src/embed_tests/TestMethodBinder.cs +++ b/src/embed_tests/TestMethodBinder.cs @@ -43,40 +43,39 @@ public void Dispose() [Test] public void ImplicitConversionToString() { - var data = (string)module.TestA(); - // we assert implicit conversion took place - Assert.AreEqual("OnlyString impl: implicit to string", data); - } + // we assert implicit conversion took place + Assert.AreEqual("OnlyString impl: implicit to string", data); + } [Test] public void ImplicitConversionToClass() { var data = (string)module.TestB(); - // we assert implicit conversion took place - Assert.AreEqual("OnlyClass impl", data); - } + // we assert implicit conversion took place + Assert.AreEqual("OnlyClass impl", data); + } [Test] public void WillAvoidUsingImplicitConversionIfPossible_String() { var data = (string)module.TestC(); - // we assert no implicit conversion took place - Assert.AreEqual("string impl: input string", data); - } + // we assert no implicit conversion took place + Assert.AreEqual("string impl: input string", data); + } [Test] public void WillAvoidUsingImplicitConversionIfPossible_Class() { var data = (string)module.TestD(); - // we assert no implicit conversion took place - Assert.AreEqual("TestImplicitConversion impl", data); + // we assert no implicit conversion took place + Assert.AreEqual("TestImplicitConversion impl", data); } [Test] public void ArrayLength() { - var array = new[] { "pepe", "pinocho" }; + var array = new[] { "pepe", "pinocho" }; var data = (bool)module.TestE(array); // Assert it is true diff --git a/src/runtime/classobject.cs b/src/runtime/classobject.cs index 4aa97f648..2f8da8a54 100644 --- a/src/runtime/classobject.cs +++ b/src/runtime/classobject.cs @@ -33,15 +33,15 @@ internal ClassObject(Type tp) : base(tp) /// internal NewReference GetDocString() { - MethodBase[] methods = binder.GetMethods(); + var methods = binder.GetMethods(); var str = ""; - foreach (MethodBase t in methods) + foreach (var t in methods) { if (str.Length > 0) { str += Environment.NewLine; } - str += t.ToString(); + str += t.MethodBase.ToString(); } return NewReference.DangerousFromPointer(Runtime.PyString_FromString(str)); } diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs index 803823e39..b3c6b655c 100644 --- a/src/runtime/constructorbinding.cs +++ b/src/runtime/constructorbinding.cs @@ -126,7 +126,7 @@ public static IntPtr tp_repr(IntPtr ob) Runtime.XIncref(self.repr); return self.repr; } - MethodBase[] methods = self.ctorBinder.GetMethods(); + var methods = self.ctorBinder.GetMethods(); if (!self.type.Valid) { @@ -134,8 +134,9 @@ public static IntPtr tp_repr(IntPtr ob) } string name = self.type.Value.FullName; var doc = ""; - foreach (MethodBase t in methods) + foreach (var methodInformation in methods) { + var t = methodInformation.MethodBase; if (doc.Length > 0) { doc += "\n"; diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 2420e6819..b1746ef66 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -390,6 +390,12 @@ internal static IntPtr ToPythonImplicit(object value) } + internal static bool ToManaged(IntPtr value, Type type, + out object result, bool setError) + { + var usedImplicit = false; + return ToManaged(value, type, out result, setError, out usedImplicit); + } /// /// Return a managed object for the given Python object, taking funny /// byref types into account. @@ -400,21 +406,26 @@ internal static IntPtr ToPythonImplicit(object value) /// If true, call Exceptions.SetError with the reason for failure. /// True on success internal static bool ToManaged(IntPtr value, Type type, - out object result, bool setError) + out object result, bool setError, out bool usedImplicit) { if (type.IsByRef) { type = type.GetElementType(); } - return Converter.ToManagedValue(value, type, out result, setError); + return Converter.ToManagedValue(value, type, out result, setError, out usedImplicit); } internal static bool ToManagedValue(BorrowedReference value, Type obType, out object result, bool setError) - => ToManagedValue(value.DangerousGetAddress(), obType, out result, setError); + { + var usedImplicit = false; + return ToManagedValue(value.DangerousGetAddress(), obType, out result, setError, out usedImplicit); + } + internal static bool ToManagedValue(IntPtr value, Type obType, - out object result, bool setError) + out object result, bool setError, out bool usedImplicit) { + usedImplicit = false; if (obType == typeof(PyObject)) { Runtime.XIncref(value); // PyObject() assumes ownership @@ -446,6 +457,18 @@ internal static bool ToManagedValue(IntPtr value, Type obType, result = tmp; return true; } + else + { + var type = tmp.GetType(); + // check implicit conversions that receive tmp type and return obType + var conversionMethod = type.GetMethod("op_Implicit", new[] { type }); + if (conversionMethod != null && conversionMethod.ReturnType == obType) + { + result = conversionMethod.Invoke(null, new[] { tmp }); + usedImplicit = true; + return true; + } + } if (setError) { string typeString = tmp is null ? "null" : tmp.GetType().ToString(); @@ -599,7 +622,7 @@ internal static bool ToManagedValue(IntPtr value, Type obType, var underlyingType = Nullable.GetUnderlyingType(obType); if (underlyingType != null) { - return ToManagedValue(value, underlyingType, out result, setError); + return ToManagedValue(value, underlyingType, out result, setError, out usedImplicit); } TypeCode typeCode = Type.GetTypeCode(obType); @@ -612,6 +635,20 @@ internal static bool ToManagedValue(IntPtr value, Type obType, } } + var opImplicit = obType.GetMethod("op_Implicit", new[] { obType }); + if (opImplicit != null) + { + if (ToManagedValue(value, opImplicit.ReturnType, out result, setError, out usedImplicit)) + { + opImplicit = obType.GetMethod("op_Implicit", new[] { result.GetType() }); + if (opImplicit != null) + { + result = opImplicit.Invoke(null, new[] { result }); + } + return opImplicit != null; + } + } + return ToPrimitive(value, obType, out result, setError); } @@ -1046,12 +1083,12 @@ private static IList MakeList(IntPtr value, IntPtr IterObject, Type obType, Type } IntPtr item; - + var usedImplicit = false; while ((item = Runtime.PyIter_Next(IterObject)) != IntPtr.Zero) { object obj; - if (!Converter.ToManaged(item, elementType, out obj, setError)) + if (!Converter.ToManaged(item, elementType, out obj, setError, out usedImplicit)) { Runtime.XDecref(item); return null; diff --git a/src/runtime/indexer.cs b/src/runtime/indexer.cs index 0772b57c6..9a8853bd6 100644 --- a/src/runtime/indexer.cs +++ b/src/runtime/indexer.cs @@ -58,13 +58,13 @@ internal void SetItem(IntPtr inst, IntPtr args) internal bool NeedsDefaultArgs(IntPtr args) { var pynargs = Runtime.PyTuple_Size(args); - MethodBase[] methods = SetterBinder.GetMethods(); - if (methods.Length == 0) + var methods = SetterBinder.GetMethods(); + if (methods.Count == 0) { return false; } - MethodBase mi = methods[0]; + var mi = methods[0].MethodBase.UnsafeValue; ParameterInfo[] pi = mi.GetParameters(); // need to subtract one for the value int clrnargs = pi.Length - 1; @@ -99,8 +99,8 @@ internal IntPtr GetDefaultArgs(IntPtr args) var pynargs = Runtime.PyTuple_Size(args); // Get the default arg tuple - MethodBase[] methods = SetterBinder.GetMethods(); - MethodBase mi = methods[0]; + var methods = SetterBinder.GetMethods(); + var mi = methods[0].MethodBase.UnsafeValue; ParameterInfo[] pi = mi.GetParameters(); int clrnargs = pi.Length - 1; IntPtr defaultArgs = Runtime.PyTuple_New(clrnargs - pynargs); diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index f4d572026..528be2b2c 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -20,10 +20,7 @@ internal class MethodBinder /// /// The overloads of this method /// - public List list; - - [NonSerialized] - public MethodBase[] methods; + public List list; [NonSerialized] public bool init = false; @@ -32,12 +29,12 @@ internal class MethodBinder internal MethodBinder() { - list = new List(); + list = new List(); } internal MethodBinder(MethodInfo mi) { - list = new List { new MaybeMethodBase(mi) }; + list = new List { new MethodInformation(mi, mi.GetParameters()) }; } public int Count @@ -47,7 +44,9 @@ public int Count internal void AddMethod(MethodBase m) { - list.Add(m); + // we added a new method so we have to re sort the method list + init = false; + list.Add(new MethodInformation(m, m.GetParameters())); } /// @@ -173,22 +172,21 @@ internal static MethodInfo MatchSignatureAndParameters(MethodInfo[] mi, Type[] g return null; } - /// /// Return the array of MethodInfo for this method. The result array /// is arranged in order of precedence (done lazily to avoid doing it /// at all for methods that are never called). /// - internal MethodBase[] GetMethods() + internal List GetMethods() { if (!init) { + list = list.Where(information => information.MethodBase.Valid).ToList(); // I'm sure this could be made more efficient. list.Sort(new MethodSorter()); - methods = (from method in list where method.Valid select method.Value).ToArray(); init = true; } - return methods; + return list; } /// @@ -199,14 +197,10 @@ internal MethodBase[] GetMethods() /// Based from Jython `org.python.core.ReflectedArgs.precedence` /// See: https://github.com/jythontools/jython/blob/master/src/org/python/core/ReflectedArgs.java#L192 /// - internal static int GetPrecedence(MethodBase mi) + private static int GetPrecedence(MethodInformation methodInformation) { - if (mi == null) - { - return int.MaxValue; - } - - ParameterInfo[] pi = mi.GetParameters(); + ParameterInfo[] pi = methodInformation.ParameterInfo; + var mi = methodInformation.MethodBase.UnsafeValue; int val = mi.IsStatic ? 3000 : 0; int num = pi.Length; @@ -330,15 +324,16 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info) private readonly struct MatchedMethod { - public MatchedMethod(int kwargsMatched, int defaultsNeeded, object[] margs, int outs, MethodBase mb) + public MatchedMethod(int kwargsMatched, int defaultsNeeded, object[] margs, int outs, MethodBase mb, bool usedImplicitConversion) { KwargsMatched = kwargsMatched; DefaultsNeeded = defaultsNeeded; ManagedArgs = margs; Outs = outs; Method = mb; + UsedImplicitConversion = usedImplicitConversion; } - + public bool UsedImplicitConversion { get; } public int KwargsMatched { get; } public int DefaultsNeeded { get; } public object[] ManagedArgs { get; } @@ -372,9 +367,6 @@ public MismatchedMethod(PythonException exception, MethodBase mb) /// A Binding if successful. Otherwise null. internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, MethodInfo[] methodinfo) { - // loop to find match, return invoker w/ or w/o error - MethodBase[] _methods = null; - var kwargDict = new Dictionary(); if (kw != IntPtr.Zero) { @@ -392,27 +384,24 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth var pynargs = (int)Runtime.PyTuple_Size(args); var isGeneric = false; - if (info != null) - { - _methods = new MethodBase[1]; - _methods.SetValue(info, 0); - } - else - { - _methods = GetMethods(); - } + var methods = info == null ? GetMethods() + : new List(1) { new MethodInformation(info, info.GetParameters()) }; - var argMatchedMethods = new List(_methods.Length); + var argMatchedMethods = new List(methods.Count); var mismatchedMethods = new List(); + var anyUsedImplicitConversion = false; + // TODO: Clean up - foreach (MethodBase mi in _methods) + foreach (var methodInformation in methods) { + var mi = methodInformation.MethodBase.UnsafeValue; + var pi = methodInformation.ParameterInfo; + var usedImplicitConversion = false; if (mi.IsGenericMethod) { isGeneric = true; } - ParameterInfo[] pi = mi.GetParameters(); ArrayList defaultArgList; bool paramsArray; int kwargsMatched; @@ -441,8 +430,8 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth } int outs; var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, - needsResolution: _methods.Length > 1, // If there's more than one possible match. - outs: out outs); + needsResolution: methods.Count > 1, // If there's more than one possible match. + outs: out outs, out usedImplicitConversion); if (margs == null) { mismatchedMethods.Add(new MismatchedMethod(new PythonException(), mi)); @@ -474,11 +463,16 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth } - var matchedMethod = new MatchedMethod(kwargsMatched, defaultsNeeded, margs, outs, mi); + var matchedMethod = new MatchedMethod(kwargsMatched, defaultsNeeded, margs, outs, mi, usedImplicitConversion); argMatchedMethods.Add(matchedMethod); + anyUsedImplicitConversion |= usedImplicitConversion; } if (argMatchedMethods.Count > 0) { + if (anyUsedImplicitConversion) + { + argMatchedMethods.Sort((method, matchedMethod) => matchedMethod.UsedImplicitConversion ? 0 : 1); + } var bestKwargMatchCount = argMatchedMethods.Max(x => x.KwargsMatched); var fewestDefaultsRequired = argMatchedMethods.Where(x => x.KwargsMatched == bestKwargMatchCount).Min(x => x.DefaultsNeeded); @@ -620,8 +614,10 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, Dictionary kwargDict, ArrayList defaultArgList, bool needsResolution, - out int outs) + out int outs, + out bool usedImplicitConversion) { + usedImplicitConversion = false; outs = 0; var margs = new object[pi.Length]; int arrayStart = paramsArray ? pi.Length - 1 : -1; @@ -660,7 +656,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, } bool isOut; - if (!TryConvertArgument(op, parameter.ParameterType, needsResolution, out margs[paramIndex], out isOut)) + if (!TryConvertArgument(op, parameter.ParameterType, needsResolution, out margs[paramIndex], out isOut, out usedImplicitConversion)) { return null; } @@ -693,11 +689,11 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, /// Whether the CLR type is passed by reference. /// true on success static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResolution, - out object arg, out bool isOut) + out object arg, out bool isOut, out bool usedImplicitConversion) { arg = null; isOut = false; - var clrtype = TryComputeClrArgumentType(parameterType, op, needsResolution: needsResolution); + var clrtype = TryComputeClrArgumentType(parameterType, op, needsResolution: needsResolution, out usedImplicitConversion); if (clrtype == null) { return false; @@ -719,8 +715,9 @@ static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResoluti /// Pointer to the Python argument object. /// If true, there are multiple overloading methods that need resolution. /// null if conversion is not possible - static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool needsResolution) + static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool needsResolution, out bool usedImplicitConversion) { + usedImplicitConversion = false; // this logic below handles cases when multiple overloading methods // are ambiguous, hence comparison between Python and CLR types // is necessary @@ -743,6 +740,7 @@ static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool { IntPtr pytype = Converter.GetPythonTypeByAlias(parameterType); pyoptype = Runtime.PyObject_Type(argument); + Exceptions.Clear(); var typematch = false; if (pyoptype != IntPtr.Zero) { @@ -772,10 +770,6 @@ static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool typematch = true; clrtype = parameterType; } - else - { - Exceptions.RaiseTypeError($"Expected {parameterTypeCode}, got {clrTypeCode}"); - } // accepts non-decimal numbers in decimal parameters if (parameterType == typeof(decimal)) @@ -788,7 +782,7 @@ static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool var opImplicit = parameterType.GetMethod("op_Implicit", new[] { clrtype }); if (opImplicit != null) { - typematch = opImplicit.ReturnType == parameterType; + usedImplicitConversion = typematch = opImplicit.ReturnType == parameterType; clrtype = parameterType; } } @@ -927,10 +921,10 @@ protected static void AppendArgumentTypes(StringBuilder to, IntPtr args) internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, MethodInfo[] methodinfo) { // No valid methods, nothing to bind. - if (GetMethods().Length == 0) + if (GetMethods().Count == 0) { var msg = new StringBuilder("The underlying C# method(s) have been deleted"); - if (list.Count > 0 && list[0].Name != null) + if (list.Count > 0 && list[0].MethodBase.Name != null) { msg.Append($": {list[0]}"); } @@ -948,9 +942,9 @@ internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw, MethodBase i { value.Append($" for {methodinfo[0].Name}"); } - else if (list.Count > 0 && list[0].Valid) + else if (list.Count > 0 && list[0].MethodBase.Valid) { - value.Append($" for {list[0].Value.Name}"); + value.Append($" for {list[0].MethodBase.Name}"); } value.Append(": "); @@ -1035,57 +1029,51 @@ internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw, MethodBase i return Converter.ToPython(result, mi.ReturnType); } - } - - /// - /// Utility class to sort method info by parameter type precedence. - /// - internal class MethodSorter : IComparer - { - int IComparer.Compare(MaybeMethodBase m1, MaybeMethodBase m2) + /// + /// Utility class to store the information about a + /// + [Serializable] + internal class MethodInformation { - MethodBase me1 = m1.UnsafeValue; - MethodBase me2 = m2.UnsafeValue; - if (me1 == null && me2 == null) - { - return 0; - } - else if (me1 == null) - { - return -1; - } - else if (me2 == null) - { - return 1; - } + public MaybeMethodBase MethodBase { get; } - if (me1.DeclaringType != me2.DeclaringType) - { - // m2's type derives from m1's type, favor m2 - if (me1.DeclaringType.IsAssignableFrom(me2.DeclaringType)) - return 1; + public ParameterInfo[] ParameterInfo { get; } - // m1's type derives from m2's type, favor m1 - if (me2.DeclaringType.IsAssignableFrom(me1.DeclaringType)) - return -1; + public MethodInformation(MethodBase methodBase, ParameterInfo[] parameterInfo) + { + MethodBase = methodBase; + ParameterInfo = parameterInfo; } - int p1 = MethodBinder.GetPrecedence(me1); - int p2 = MethodBinder.GetPrecedence(me2); - if (p1 < p2) + public override string ToString() { - return -1; + return MethodBase.ToString(); } - if (p1 > p2) + } + + /// + /// Utility class to sort method info by parameter type precedence. + /// + internal class MethodSorter : IComparer + { + public int Compare(MethodInformation x, MethodInformation y) { - return 1; + int p1 = GetPrecedence(x); + int p2 = GetPrecedence(y); + if (p1 < p2) + { + return -1; + } + if (p1 > p2) + { + return 1; + } + return 0; } - return 0; } } - /// /// A Binding is a utility instance that bundles together a MethodInfo /// representing a method to call, a (possibly null) target instance for diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs index 37c01f5c5..7711eabde 100644 --- a/src/runtime/methodobject.cs +++ b/src/runtime/methodobject.cs @@ -79,14 +79,14 @@ internal IntPtr GetDocString() } var str = ""; Type marker = typeof(DocStringAttribute); - MethodBase[] methods = binder.GetMethods(); - foreach (MethodBase method in methods) + var methods = binder.GetMethods(); + foreach (var method in methods) { if (str.Length > 0) { str += Environment.NewLine; } - var attrs = (Attribute[])method.GetCustomAttributes(marker, false); + var attrs = (Attribute[])method.MethodBase.UnsafeValue.GetCustomAttributes(marker, false); if (attrs.Length == 0) { str += method.ToString();