remove needsResolution hack

lostmsu · lostmsu · commit e42ec3b8896a · 2021-08-25T12:33:07.000-07:00
fixes #1523
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -52,6 +52,8 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru
 -   `PythonException.Restore` no longer clears `PythonException` instance.
 -   Replaced the old `__import__` hook hack with a PEP302-style Meta Path Loader
 -   BREAKING: Names of .NET types (e.g. `str(__class__)`) changed to better support generic types
+-   BREAKING: overload resolution will no longer prefer basic types. Instead, first matching overload will
+be chosen.
 
 ### Fixed
 
diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs
@@ -1,7 +1,9 @@
 using NUnit.Framework;
 
 using Python.Runtime;
+using Python.Runtime.Codecs;
 
+using System;
 using System.Linq;
 using System.Reflection;
 
@@ -212,21 +214,19 @@ public OperableObject(int num)
                 return (a.Num >= b);
             }
 
-            public static bool operator >=(OperableObject a, PyObject b)
+            public static bool operator >=(OperableObject a, (int, int) b)
             {
                 using (Py.GIL())
                 {
-                    // Assuming b is a tuple, take the first element.
-                    int bNum = b[0].As<int>();
+                    int bNum = b.Item1;
                     return a.Num >= bNum;
                 }
             }
-            public static bool operator <=(OperableObject a, PyObject b)
+            public static bool operator <=(OperableObject a, (int, int) b)
             {
                 using (Py.GIL())
                 {
-                    // Assuming b is a tuple, take the first element.
-                    int bNum = b[0].As<int>();
+                    int bNum = b.Item1;
                     return a.Num <= bNum;
                 }
             }
@@ -421,6 +421,7 @@ public void ForwardOperatorOverloads()
         [Test]
         public void TupleComparisonOperatorOverloads()
         {
+                TupleCodec<ValueTuple>.Register();
                 string name = string.Format("{0}.{1}",
                 typeof(OperableObject).DeclaringType.Name,
                 typeof(OperableObject).Name);
diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs
@@ -433,9 +433,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth
                     pi = pi.Take(1).ToArray();
                 }
                 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);
+                var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, outs: out outs);
                 if (margs == null)
                 {
                     var mismatchCause = PythonException.FetchCurrent();
@@ -612,7 +610,6 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
             IntPtr args, int pyArgCount,
             Dictionary<string, IntPtr> kwargDict,
             ArrayList defaultArgList,
-            bool needsResolution,
             out int outs)
         {
             outs = 0;
@@ -653,7 +650,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, out margs[paramIndex], out isOut))
                 {
                     return null;
                 }
@@ -681,16 +678,15 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray,
         /// </summary>
         /// <param name="op">Pointer to the Python argument object.</param>
         /// <param name="parameterType">That parameter's managed type.</param>
-        /// <param name="needsResolution">If true, there are multiple overloading methods that need resolution.</param>
         /// <param name="arg">Converted argument.</param>
         /// <param name="isOut">Whether the CLR type is passed by reference.</param>
         /// <returns>true on success</returns>
-        static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResolution,
+        static bool TryConvertArgument(IntPtr op, Type parameterType,
                                        out object arg, out bool isOut)
         {
             arg = null;
             isOut = false;
-            var clrtype = TryComputeClrArgumentType(parameterType, op, needsResolution: needsResolution);
+            var clrtype = TryComputeClrArgumentType(parameterType, op);
             if (clrtype == null)
             {
                 return false;
@@ -710,25 +706,14 @@ static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResoluti
         /// </summary>
         /// <param name="parameterType">The parameter's managed type.</param>
         /// <param name="argument">Pointer to the Python argument object.</param>
-        /// <param name="needsResolution">If true, there are multiple overloading methods that need resolution.</param>
         /// <returns>null if conversion is not possible</returns>
-        static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool needsResolution)
+        static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument)
         {
             // this logic below handles cases when multiple overloading methods
             // are ambiguous, hence comparison between Python and CLR types
             // is necessary
             Type clrtype = null;
             IntPtr pyoptype;
-            if (needsResolution)
-            {
-                // HACK: each overload should be weighted in some way instead
-                pyoptype = Runtime.PyObject_Type(argument);
-                if (pyoptype != IntPtr.Zero)
-                {
-                    clrtype = Converter.GetTypeByAlias(pyoptype);
-                }
-                Runtime.XDecref(pyoptype);
-            }
 
             if (clrtype != null)
             {

Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,9 @@`
`1`	`1`	`using NUnit.Framework;`
`2`	`2`
`3`	`3`	`using Python.Runtime;`
	`4`	`+using Python.Runtime.Codecs;`
`4`	`5`
	`6`	`+using System;`
`5`	`7`	`using System.Linq;`
`6`	`8`	`using System.Reflection;`
`7`	`9`
`@@ -212,21 +214,19 @@ public OperableObject(int num)`
`212`	`214`	`return (a.Num >= b);`
`213`	`215`	`}`
`214`	`216`
`215`		`- public static bool operator >=(OperableObject a, PyObject b)`
	`217`	`+ public static bool operator >=(OperableObject a, (int, int) b)`
`216`	`218`	`{`
`217`	`219`	`using (Py.GIL())`
`218`	`220`	`{`
`219`		`- // Assuming b is a tuple, take the first element.`
`220`		`- int bNum = b[0].As<int>();`
	`221`	`+ int bNum = b.Item1;`
`221`	`222`	`return a.Num >= bNum;`
`222`	`223`	`}`
`223`	`224`	`}`
`224`		`- public static bool operator <=(OperableObject a, PyObject b)`
	`225`	`+ public static bool operator <=(OperableObject a, (int, int) b)`
`225`	`226`	`{`
`226`	`227`	`using (Py.GIL())`
`227`	`228`	`{`
`228`		`- // Assuming b is a tuple, take the first element.`
`229`		`- int bNum = b[0].As<int>();`
	`229`	`+ int bNum = b.Item1;`
`230`	`230`	`return a.Num <= bNum;`
`231`	`231`	`}`
`232`	`232`	`}`
`@@ -421,6 +421,7 @@ public void ForwardOperatorOverloads()`
`421`	`421`	`[Test]`
`422`	`422`	`public void TupleComparisonOperatorOverloads()`
`423`	`423`	`{`
	`424`	`+ TupleCodec<ValueTuple>.Register();`
`424`	`425`	`string name = string.Format("{0}.{1}",`
`425`	`426`	`typeof(OperableObject).DeclaringType.Name,`
`426`	`427`	`typeof(OperableObject).Name);`