Improve numeric implicit conversion handling

Author: Martin-Molinero

src/embed_tests/TestMethodBinder.cs

@@ -20,7 +20,9 @@ class PythonModel(TestMethodBinder.CSharpModel):
     def TestA(self):
         return self.OnlyString(TestMethodBinder.TestImplicitConversion())
     def TestB(self):
-        return self.OnlyClass('input string')
+        return self.OnlyClass('input string', 1)
+    def TestBB(self):
+        return self.OnlyClass(TestMethodBinder.TestImplicitConversion(), 1)
     def TestC(self):
         return self.InvokeModel('input string')
     def TestD(self):
@@ -154,6 +156,17 @@ public void ImplicitConversionToClass()
             {
                 var data = (string)module.TestB();
                 // we assert implicit conversion took place
+                Assert.AreEqual("OnlyClass str", data);
+            }
+        }
+
+        [Test]
+        public void NoImplicitConversionToClass()
+        {
+            using (Py.GIL())
+            {
+                var data = (string)module.TestBB();
+                // we assert implicit conversion took place
                 Assert.AreEqual("OnlyClass impl", data);
             }
         }
@@ -1448,11 +1461,16 @@ public bool SomeMethod()
                 return true;
             }
 
-            public virtual string OnlyClass(TestImplicitConversion data)
+            public virtual string OnlyClass(TestImplicitConversion data, double anotherArgument = 0)
             {
                 return "OnlyClass impl";
             }
 
+            public virtual string OnlyClass(string data, double anotherArgument = 0)
+            {
+                return "OnlyClass str";
+            }
+
             public virtual string OnlyString(string data)
             {
                 return "OnlyString impl: " + data;

src/runtime/MethodBinder.cs

@@ -546,7 +546,7 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
                     var margs = new object[clrArgCount];
 
                     int paramsArrayIndex = paramsArray ? pi.Length - 1 : -1; // -1 indicates no paramsArray
-                    var usedImplicitConversion = false;
+                    int implicitConversions = 0;
                     var kwargsMatched = 0;
 
                     // Conversion loop for each parameter
@@ -658,18 +658,26 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
                                     else if (matches.Count == 0)
                                     {
                                         // accepts non-decimal numbers in decimal parameters
-                                        if (underlyingType == typeof(decimal))
+                                        if (underlyingType == typeof(decimal) || underlyingType == typeof(double))
                                         {
                                             clrtype = parameter.ParameterType;
-                                            usedImplicitConversion |= typematch = Converter.ToManaged(op, clrtype, out arg, false);
+                                            typematch = Converter.ToManaged(op, clrtype, out arg, false);
+                                            if (typematch)
+                                            {
+                                                implicitConversions++;
+                                            }
                                         }
                                         if (!typematch)
                                         {
                                             // this takes care of implicit conversions
                                             var opImplicit = parameter.ParameterType.GetMethod("op_Implicit", new[] { clrtype });
                                             if (opImplicit != null)
                                             {
-                                                usedImplicitConversion |= typematch = opImplicit.ReturnType == parameter.ParameterType;
+                                                typematch = opImplicit.ReturnType == parameter.ParameterType;
+                                                if (typematch)
+                                                {
+                                                    implicitConversions++;
+                                                }
                                                 clrtype = parameter.ParameterType;
                                             }
                                         }
@@ -739,13 +747,10 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
                         }
                     }
 
-                    var match = new MatchedMethod(kwargsMatched, margs, outs, methodInformation);
-                    if (usedImplicitConversion)
+                    var match = new MatchedMethod(kwargsMatched, margs, outs, methodInformation, implicitConversions);
+                    if (implicitConversions > 0)
                     {
-                        if (matchesUsingImplicitConversion == null)
-                        {
-                            matchesUsingImplicitConversion = new List<MatchedMethod>();
-                        }
+                        matchesUsingImplicitConversion ??= new List<MatchedMethod>();
                         matchesUsingImplicitConversion.Add(match);
                     }
                     else
@@ -767,11 +772,22 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
                 // we solve the ambiguity by taking the one with the highest precedence but only
                 // considering the actual arguments passed, ignoring the optional arguments for
                 // which the default values were used
-                var bestMatch = matchesTouse
-                    .GroupBy(x => x.KwargsMatched)
-                    .OrderByDescending(x => x.Key)
-                    .First()
-                    .MinBy(x => GetMatchedArgumentsPrecedence(x.MethodInformation, pyArgCount, kwArgDict?.Keys));
+                MatchedMethod bestMatch;
+                if (matchesTouse.Count == 1)
+                {
+                    bestMatch = matchesTouse[0];
+                }
+                else
+                {
+                    bestMatch = matchesTouse
+                        .GroupBy(x => x.KwargsMatched)
+                        .OrderByDescending(x => x.Key)
+                        .First()
+                        .GroupBy(x => x.ImplicitOperations)
+                        .OrderBy(x => x.Key)
+                        .First()
+                        .MinBy(x => GetMatchedArgumentsPrecedence(x.MethodInformation, pyArgCount, kwArgDict?.Keys));
+                }
 
                 var margs = bestMatch.ManagedArgs;
                 var outs = bestMatch.Outs;
@@ -1135,15 +1151,17 @@ private readonly struct MatchedMethod
             public int KwargsMatched { get; }
             public object?[] ManagedArgs { get; }
             public int Outs { get; }
+            public int ImplicitOperations { get; }
             public MethodInformation MethodInformation { get; }
             public MethodBase Method => MethodInformation.MethodBase;
 
-            public MatchedMethod(int kwargsMatched, object?[] margs, int outs, MethodInformation methodInformation)
+            public MatchedMethod(int kwargsMatched, object?[] margs, int outs, MethodInformation methodInformation, int implicitOperations)
             {
                 KwargsMatched = kwargsMatched;
                 ManagedArgs = margs;
                 Outs = outs;
                 MethodInformation = methodInformation;
+                ImplicitOperations = implicitOperations;
             }
         }