diff --git a/CHANGELOG.md b/CHANGELOG.md index 0da350cef..63ba315e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ details about the cause of the failure - Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash - Fix incorrect dereference in params array handling +- Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097]) - Fix `object[]` parameters taking precedence when should not in overload resolution - Fixed a bug where all .NET class instances were considered Iterable - Fix incorrect choice of method to invoke when using keyword arguments. diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index c0cc4c75c..bd6eb32ba 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -279,6 +279,23 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info) return Bind(inst, args, kw, info, null); } + private readonly struct MatchedMethod { + public MatchedMethod(int kwargsMatched, int defaultsNeeded, object[] margs, int outs, MethodBase mb) + { + KwargsMatched = kwargsMatched; + DefaultsNeeded = defaultsNeeded; + ManagedArgs = margs; + Outs = outs; + Method = mb; + } + + public int KwargsMatched { get; } + public int DefaultsNeeded { get; } + public object[] ManagedArgs { get; } + public int Outs { get; } + public MethodBase Method { get; } + } + internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, MethodInfo[] methodinfo) { // loop to find match, return invoker w/ or /wo error @@ -311,6 +328,8 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth _methods = GetMethods(); } + var argMatchedMethods = new List(_methods.Length); + // TODO: Clean up foreach (MethodBase mi in _methods) { @@ -321,8 +340,10 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth ParameterInfo[] pi = mi.GetParameters(); ArrayList defaultArgList; bool paramsArray; + int kwargsMatched; + int defaultsNeeded; - if (!MatchesArgumentCount(pynargs, pi, kwargDict, out paramsArray, out defaultArgList)) + if (!MatchesArgumentCount(pynargs, pi, kwargDict, out paramsArray, out defaultArgList, out kwargsMatched, out defaultsNeeded)) { continue; } @@ -336,6 +357,47 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth continue; } + var matchedMethod = new MatchedMethod(kwargsMatched, defaultsNeeded, margs, outs, mi); + argMatchedMethods.Add(matchedMethod); + } + if (argMatchedMethods.Count > 0) + { + var bestKwargMatchCount = argMatchedMethods.Max(x => x.KwargsMatched); + var fewestDefaultsRequired = argMatchedMethods.Where(x => x.KwargsMatched == bestKwargMatchCount).Min(x => x.DefaultsNeeded); + + int bestCount = 0; + int bestMatchIndex = -1; + + for (int index = 0; index < argMatchedMethods.Count; index++) + { + var testMatch = argMatchedMethods[index]; + if (testMatch.DefaultsNeeded == fewestDefaultsRequired && testMatch.KwargsMatched == bestKwargMatchCount) + { + bestCount++; + if (bestMatchIndex == -1) + bestMatchIndex = index; + } + } + + if (bestCount > 1 && fewestDefaultsRequired > 0) + { + // Best effort for determining method to match on gives multiple possible + // matches and we need at least one default argument - bail from this point + return null; + } + + // If we're here either: + // (a) There is only one best match + // (b) There are multiple best matches but none of them require + // default arguments + // in the case of (a) we're done by default. For (b) regardless of which + // method we choose, all arguments are specified _and_ can be converted + // from python to C# so picking any will suffice + MatchedMethod bestMatch = argMatchedMethods[bestMatchIndex]; + var margs = bestMatch.ManagedArgs; + var outs = bestMatch.Outs; + var mi = bestMatch.Method; + object target = null; if (!mi.IsStatic && inst != IntPtr.Zero) { @@ -575,11 +637,16 @@ static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] parameters, Dictionary kwargDict, out bool paramsArray, - out ArrayList defaultArgList) + out ArrayList defaultArgList, + out int kwargsMatched, + out int defaultsNeeded) { defaultArgList = null; var match = false; paramsArray = parameters.Length > 0 ? Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute)) : false; + var kwargCount = kwargDict.Count; + kwargsMatched = 0; + defaultsNeeded = 0; if (positionalArgumentCount == parameters.Length && kwargDict.Count == 0) { @@ -599,6 +666,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa // no need to check for a default parameter, but put a null // placeholder in defaultArgList defaultArgList.Add(null); + kwargsMatched++; } else if (parameters[v].IsOptional) { @@ -607,6 +675,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa // The GetDefaultValue() extension method will return the value // to be passed in as the parameter value defaultArgList.Add(parameters[v].GetDefaultValue()); + defaultsNeeded++; } else if(!paramsArray) { diff --git a/src/testing/methodtest.cs b/src/testing/methodtest.cs index 30e42ac9f..62d154539 100644 --- a/src/testing/methodtest.cs +++ b/src/testing/methodtest.cs @@ -684,7 +684,25 @@ public static string OptionalAndDefaultParams2([Optional]int a, [Optional]int b, return string.Format("{0}{1}{2}{3}", a, b, c, d); } - + public static string DefaultParamsWithOverloading(int a = 2, int b = 1) + { + return $"{a}{b}"; + } + + public static string DefaultParamsWithOverloading(string a = "a", string b = "b") + { + return $"{a}{b}X"; + } + + public static string DefaultParamsWithOverloading(int a = 0, int b = 1, int c = 2) + { + return $"{a}{b}{c}XX"; + } + + public static string DefaultParamsWithOverloading(int a = 5, int b = 6, int c = 7, int d = 8) + { + return $"{a}{b}{c}{d}XXX"; + } } diff --git a/src/tests/test_method.py b/src/tests/test_method.py index 15327cb3d..f6522e49e 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -1153,6 +1153,35 @@ def test_optional_and_default_params(): res = MethodTest.OptionalAndDefaultParams2(b=2, c=3) assert res == "0232" +def test_default_params_overloads(): + res = MethodTest.DefaultParamsWithOverloading(1, 2) + assert res == "12" + + res = MethodTest.DefaultParamsWithOverloading(b=5) + assert res == "25" + + res = MethodTest.DefaultParamsWithOverloading("d") + assert res == "dbX" + + res = MethodTest.DefaultParamsWithOverloading(b="c") + assert res == "acX" + + res = MethodTest.DefaultParamsWithOverloading(c=3) + assert res == "013XX" + + res = MethodTest.DefaultParamsWithOverloading(5, c=2) + assert res == "512XX" + + res = MethodTest.DefaultParamsWithOverloading(c=0, d=1) + assert res == "5601XXX" + + res = MethodTest.DefaultParamsWithOverloading(1, d=1) + assert res == "1671XXX" + +def test_default_params_overloads_ambiguous_call(): + with pytest.raises(TypeError): + MethodTest.DefaultParamsWithOverloading() + def test_keyword_arg_method_resolution(): from Python.Test import MethodArityTest