From 9fb753a789e52188061dc5a3194ba92d8e45ca18 Mon Sep 17 00:00:00 2001 From: jmlidbetter <53430310+jmlidbetter@users.noreply.github.com> Date: Tue, 28 Apr 2020 17:28:42 +0100 Subject: [PATCH 1/6] Fix function resolution when calling overloads with keyword arguments --- src/runtime/methodbinder.cs | 69 +++++++++++++++++++++++++++++++++++-- src/testing/methodtest.cs | 20 ++++++++++- src/tests/test_method.py | 30 ++++++++++++++++ 3 files changed, 116 insertions(+), 3 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index b3186a3f2..4d7175010 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -310,6 +310,11 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth _methods = GetMethods(); } + // List of tuples containing methods that have matched based on arguments. + // Format of tuple is: Number of kwargs matched, defaults needed, margs, outs, method base for matched method + List> argMatchedMethods = + new List>(_methods.Length); + // TODO: Clean up foreach (MethodBase mi in _methods) { @@ -320,8 +325,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; } @@ -335,6 +342,57 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth continue; } + var matchedMethodTuple = Tuple.Create(kwargsMatched, defaultsNeeded, margs, outs, mi); + argMatchedMethods.Add(matchedMethodTuple); + } + if (argMatchedMethods.Count() > 0) + { + // Order matched methods by number of kwargs matched and get the max possible number + // of kwargs matched + var bestKwargMatchCount = argMatchedMethods.OrderBy((x) => x.Item1).Reverse().ToArray()[0].Item1; + + List> bestKwargMatches = + new List>(argMatchedMethods.Count()); + foreach (Tuple argMatchedTuple in argMatchedMethods) + { + if (argMatchedTuple.Item1 == bestKwargMatchCount) + { + bestKwargMatches.Add(argMatchedTuple); + } + } + + // Order by the number of defaults required and find the smallest + var fewestDefaultsRequired = bestKwargMatches.OrderBy((x) => x.Item2).ToArray()[0].Item2; + + List> bestDefaultsMatches = + new List>(bestKwargMatches.Count()); + foreach (Tuple testTuple in bestKwargMatches) + { + if (testTuple.Item2 == fewestDefaultsRequired) + { + bestDefaultsMatches.Add(testTuple); + } + } + + if (bestDefaultsMatches.Count() > 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 + Tuple bestMatch = bestDefaultsMatches.ToArray()[0]; + var margs = bestMatch.Item3; + var outs = bestMatch.Item4; + var mi = bestMatch.Item5; + object target = null; if (!mi.IsStatic && inst != IntPtr.Zero) { @@ -574,11 +632,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) { @@ -598,6 +661,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) { @@ -606,6 +670,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 91836b727..084b73439 100644 --- a/src/testing/methodtest.cs +++ b/src/testing/methodtest.cs @@ -683,7 +683,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 d9c033802..ad02992ce 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -1146,3 +1146,33 @@ 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() \ No newline at end of file From fee4373419f640500dcf3210cc018f1ef6bb3abc Mon Sep 17 00:00:00 2001 From: jmlidbetter <53430310+jmlidbetter@users.noreply.github.com> Date: Thu, 7 May 2020 10:02:34 +0100 Subject: [PATCH 2/6] Updates CHANGELOG.md --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e82f40e3..d344766a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,12 @@ This version improves performance on benchmarks significantly compared to 2.3. - Fill `__classcell__` correctly for Python subclasses of .NET types - Fixed issue with params methods that are not passed an array. - Use UTF8 to encode strings passed to `PyRun_String` on Python 3 +- Fixed runtime that fails loading when using pythonnet in an environment + together with Nuitka +- Fixes bug where delegates get casts (dotnetcore) +- Determine size of interpreter longs at runtime +- Handling exceptions ocurred in ModuleObject's getattribute +- Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097]) ## [2.4.0][] - 2019-05-15 From 8581715173766f3f1d9a5396665c430d45f8eef9 Mon Sep 17 00:00:00 2001 From: jmlidbetter <53430310+jmlidbetter@users.noreply.github.com> Date: Mon, 11 May 2020 10:57:28 +0100 Subject: [PATCH 3/6] Switches Tuple for struct --- src/runtime/methodbinder.cs | 56 ++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 4d7175010..0c1e2beb2 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -278,6 +278,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 @@ -310,10 +327,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth _methods = GetMethods(); } - // List of tuples containing methods that have matched based on arguments. - // Format of tuple is: Number of kwargs matched, defaults needed, margs, outs, method base for matched method - List> argMatchedMethods = - new List>(_methods.Length); + List argMatchedMethods = new List(_methods.Length); // TODO: Clean up foreach (MethodBase mi in _methods) @@ -342,35 +356,33 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth continue; } - var matchedMethodTuple = Tuple.Create(kwargsMatched, defaultsNeeded, margs, outs, mi); - argMatchedMethods.Add(matchedMethodTuple); + var matchedMethod = new MatchedMethod(kwargsMatched, defaultsNeeded, margs, outs, mi); + argMatchedMethods.Add(matchedMethod); } if (argMatchedMethods.Count() > 0) { // Order matched methods by number of kwargs matched and get the max possible number // of kwargs matched - var bestKwargMatchCount = argMatchedMethods.OrderBy((x) => x.Item1).Reverse().ToArray()[0].Item1; + var bestKwargMatchCount = argMatchedMethods.OrderBy((x) => x.KwargsMatched).Reverse().ToArray()[0].KwargsMatched; - List> bestKwargMatches = - new List>(argMatchedMethods.Count()); - foreach (Tuple argMatchedTuple in argMatchedMethods) + List bestKwargMatches = new List(argMatchedMethods.Count()); + foreach (MatchedMethod testMatch in argMatchedMethods) { - if (argMatchedTuple.Item1 == bestKwargMatchCount) + if (testMatch.KwargsMatched == bestKwargMatchCount) { - bestKwargMatches.Add(argMatchedTuple); + bestKwargMatches.Add(testMatch); } } // Order by the number of defaults required and find the smallest - var fewestDefaultsRequired = bestKwargMatches.OrderBy((x) => x.Item2).ToArray()[0].Item2; + var fewestDefaultsRequired = bestKwargMatches.OrderBy((x) => x.DefaultsNeeded).ToArray()[0].DefaultsNeeded; - List> bestDefaultsMatches = - new List>(bestKwargMatches.Count()); - foreach (Tuple testTuple in bestKwargMatches) + List bestDefaultsMatches = new List(bestKwargMatches.Count()); + foreach (MatchedMethod testMatch in bestKwargMatches) { - if (testTuple.Item2 == fewestDefaultsRequired) + if (testMatch.DefaultsNeeded == fewestDefaultsRequired) { - bestDefaultsMatches.Add(testTuple); + bestDefaultsMatches.Add(testMatch); } } @@ -388,10 +400,10 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth // 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 - Tuple bestMatch = bestDefaultsMatches.ToArray()[0]; - var margs = bestMatch.Item3; - var outs = bestMatch.Item4; - var mi = bestMatch.Item5; + MatchedMethod bestMatch = bestDefaultsMatches.ToArray()[0]; + var margs = bestMatch.ManagedArgs; + var outs = bestMatch.Outs; + var mi = bestMatch.Method; object target = null; if (!mi.IsStatic && inst != IntPtr.Zero) From 86d21f1696f79cf1f6b8eedd0a287ad1d7fdccbb Mon Sep 17 00:00:00 2001 From: jmlidbetter <53430310+jmlidbetter@users.noreply.github.com> Date: Tue, 12 May 2020 09:11:32 +0100 Subject: [PATCH 4/6] Use LINQ Min and Max methods; don't create second list for methods best matched on defaults --- src/runtime/methodbinder.cs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 0c1e2beb2..c02664705 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -363,7 +363,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth { // Order matched methods by number of kwargs matched and get the max possible number // of kwargs matched - var bestKwargMatchCount = argMatchedMethods.OrderBy((x) => x.KwargsMatched).Reverse().ToArray()[0].KwargsMatched; + var bestKwargMatchCount = argMatchedMethods.Max(x => x.KwargsMatched); List bestKwargMatches = new List(argMatchedMethods.Count()); foreach (MatchedMethod testMatch in argMatchedMethods) @@ -375,18 +375,21 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth } // Order by the number of defaults required and find the smallest - var fewestDefaultsRequired = bestKwargMatches.OrderBy((x) => x.DefaultsNeeded).ToArray()[0].DefaultsNeeded; + var fewestDefaultsRequired = bestKwargMatches.Min(x => x.DefaultsNeeded); + int bestCount = 0; + int bestMatchIndex = -1; - List bestDefaultsMatches = new List(bestKwargMatches.Count()); foreach (MatchedMethod testMatch in bestKwargMatches) { if (testMatch.DefaultsNeeded == fewestDefaultsRequired) { - bestDefaultsMatches.Add(testMatch); + bestCount++; + if (bestMatchIndex == -1) + bestMatchIndex = bestKwargMatches.IndexOf(testMatch); } } - if (bestDefaultsMatches.Count() > 1 && fewestDefaultsRequired > 0) + 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 @@ -400,7 +403,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth // 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 = bestDefaultsMatches.ToArray()[0]; + MatchedMethod bestMatch = bestKwargMatches.ElementAt(bestMatchIndex); var margs = bestMatch.ManagedArgs; var outs = bestMatch.Outs; var mi = bestMatch.Method; From d45315969d1a5d65481ae34fa17fda3c155ac984 Mon Sep 17 00:00:00 2001 From: jmlidbetter <53430310+jmlidbetter@users.noreply.github.com> Date: Tue, 12 May 2020 14:24:42 +0100 Subject: [PATCH 5/6] Removes all secondary lists --- src/runtime/methodbinder.cs | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index c02664705..681304e9c 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -327,7 +327,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth _methods = GetMethods(); } - List argMatchedMethods = new List(_methods.Length); + var argMatchedMethods = new List(_methods.Length); // TODO: Clean up foreach (MethodBase mi in _methods) @@ -359,33 +359,22 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth var matchedMethod = new MatchedMethod(kwargsMatched, defaultsNeeded, margs, outs, mi); argMatchedMethods.Add(matchedMethod); } - if (argMatchedMethods.Count() > 0) + if (argMatchedMethods.Count > 0) { - // Order matched methods by number of kwargs matched and get the max possible number - // of kwargs matched var bestKwargMatchCount = argMatchedMethods.Max(x => x.KwargsMatched); + var fewestDefaultsRequired = argMatchedMethods.Where(x => x.KwargsMatched == bestKwargMatchCount).Min(x => x.DefaultsNeeded); - List bestKwargMatches = new List(argMatchedMethods.Count()); - foreach (MatchedMethod testMatch in argMatchedMethods) - { - if (testMatch.KwargsMatched == bestKwargMatchCount) - { - bestKwargMatches.Add(testMatch); - } - } - - // Order by the number of defaults required and find the smallest - var fewestDefaultsRequired = bestKwargMatches.Min(x => x.DefaultsNeeded); int bestCount = 0; int bestMatchIndex = -1; - foreach (MatchedMethod testMatch in bestKwargMatches) + for (int index = 0; index < argMatchedMethods.Count; index++) { - if (testMatch.DefaultsNeeded == fewestDefaultsRequired) + var testMatch = argMatchedMethods[index]; + if (testMatch.DefaultsNeeded == fewestDefaultsRequired && testMatch.KwargsMatched == bestKwargMatchCount) { bestCount++; if (bestMatchIndex == -1) - bestMatchIndex = bestKwargMatches.IndexOf(testMatch); + bestMatchIndex = index; } } @@ -403,7 +392,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth // 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 = bestKwargMatches.ElementAt(bestMatchIndex); + MatchedMethod bestMatch = argMatchedMethods[bestMatchIndex]; var margs = bestMatch.ManagedArgs; var outs = bestMatch.Outs; var mi = bestMatch.Method; @@ -654,7 +643,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa defaultArgList = null; var match = false; paramsArray = parameters.Length > 0 ? Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute)) : false; - var kwargCount = kwargDict.Count(); + var kwargCount = kwargDict.Count; kwargsMatched = 0; defaultsNeeded = 0; From a1b6c76bfb45b445add2d9942b651057b5852610 Mon Sep 17 00:00:00 2001 From: jmlidbetter <53430310+jmlidbetter@users.noreply.github.com> Date: Mon, 22 Jun 2020 16:23:58 +0100 Subject: [PATCH 6/6] Updates CHANGELOG --- CHANGELOG.md | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d344766a3..2d353fe01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. - 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]) ## [2.5.0][] - 2020-06-14 @@ -68,12 +69,6 @@ This version improves performance on benchmarks significantly compared to 2.3. - Fill `__classcell__` correctly for Python subclasses of .NET types - Fixed issue with params methods that are not passed an array. - Use UTF8 to encode strings passed to `PyRun_String` on Python 3 -- Fixed runtime that fails loading when using pythonnet in an environment - together with Nuitka -- Fixes bug where delegates get casts (dotnetcore) -- Determine size of interpreter longs at runtime -- Handling exceptions ocurred in ModuleObject's getattribute -- Fixes issue with function resolution when calling overloaded function with keyword arguments from python ([#1097][i1097]) ## [2.4.0][] - 2019-05-15