From 677d98004c3920e392334b261bf7a29cb59f1677 Mon Sep 17 00:00:00 2001 From: Joe Savage Date: Thu, 12 Sep 2019 03:26:06 -0500 Subject: [PATCH 1/5] Feature/named arg support (#953) * Add support for named arguments (#849) * Remove kwarg check since it breaks the python-derived CLR class use-case * Add named parameter test cases * Update changelog and authors * Add default params tests --- AUTHORS.md | 2 + CHANGELOG.md | 22 ++++ src/runtime/methodbinder.cs | 199 +++++++++++++++++++++++++++++++++++- src/testing/methodtest.cs | 33 ++++++ src/tests/test_method.py | 162 +++++++++++++++++++++++++++++ 5 files changed, 416 insertions(+), 2 deletions(-) diff --git a/AUTHORS.md b/AUTHORS.md index fc0fdf86b..66c21ee93 100644 --- a/AUTHORS.md +++ b/AUTHORS.md @@ -28,6 +28,8 @@ -   Ivan Cronyn ([@cronan](https://github.com/cronan)) -   Jeff Reback ([@jreback](https://github.com/jreback)) - Joe Frayne ([@jfrayne](https://github.com/jfrayne)) +- Joe Lidbetter ([@jmlidbetter](https://github.com/jmlidbetter)) +- Joe Savage ([@s4v4g3](https://github.com/s4v4g3)) - John Burnett ([@johnburnett](https://github.com/johnburnett)) - John Wilkes ([@jbw3](https://github.com/jbw3)) - Luke Stratman ([@lstratman](https://github.com/lstratman)) diff --git a/CHANGELOG.md b/CHANGELOG.md index b92703d43..50573b15e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,28 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ## [unreleased][] ### Added + +- Added automatic NuGet package generation in appveyor and local builds + +### Changed + +- Added argument types information to "No method matches given arguments" message +- Moved wheel import in setup.py inside of a try/except to prevent pip collection failures +- Removes PyLong_GetMax and PyClass_New when targetting Python3 +- Added support for converting python iterators to C# arrays +- Changed usage of obselete function GetDelegateForFunctionPointer(IntPtr, Type) to GetDelegateForFunctionPointer(IntPtr) +- Added support for kwarg parameters when calling .NET methods from Python + +### Fixed + +- Fixed runtime that fails loading when using pythonnet in an environment + together with Nuitka +- Fixes bug where delegates get casts (dotnetcore) + +## [2.4.0][] + +### Added + - Added support for embedding python into dotnet core 2.0 (NetStandard 2.0) - Added new build system (pythonnet.15.sln) based on dotnetcore-sdk/xplat(crossplatform msbuild). Currently there two side-by-side build systems that produces the same output (net40) from the same sources. diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 854f386ab..199a6412c 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -2,6 +2,9 @@ using System.Collections; using System.Collections.Generic; using System.Reflection; +using System.Text; +using System.Collections.Generic; +using System.Linq; namespace Python.Runtime { @@ -292,8 +295,24 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info) internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, MethodInfo[] methodinfo) { // loop to find match, return invoker w/ or /wo error - int pynargs = Runtime.PyTuple_Size(args); - object arg; + MethodBase[] _methods = null; + + var kwargDict = new Dictionary(); + if (kw != IntPtr.Zero) + { + var pynkwargs = (int)Runtime.PyDict_Size(kw); + IntPtr keylist = Runtime.PyDict_Keys(kw); + IntPtr valueList = Runtime.PyDict_Values(kw); + for (int i = 0; i < pynkwargs; ++i) + { + var keyStr = Runtime.GetManagedString(Runtime.PyList_GetItem(keylist, i)); + kwargDict[keyStr] = Runtime.PyList_GetItem(valueList, i); + } + Runtime.XDecref(keylist); + Runtime.XDecref(valueList); + } + + var pynargs = (int)Runtime.PyTuple_Size(args); var isGeneric = false; ArrayList defaultArgList; Type clrtype; @@ -324,7 +343,120 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth var outs = 0; var margs = new object[clrnargs]; var usedImplicitConversion = false; + if (!MatchesArgumentCount(pynargs, pi, kwargDict, out paramsArray, out defaultArgList)) + { + continue; + } + var outs = 0; + var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, + needsResolution: _methods.Length > 1, + outs: out outs); + + if (margs == null) + { + continue; + } + + object target = null; + if (!mi.IsStatic && inst != IntPtr.Zero) + { + //CLRObject co = (CLRObject)ManagedType.GetManagedObject(inst); + // InvalidCastException: Unable to cast object of type + // 'Python.Runtime.ClassObject' to type 'Python.Runtime.CLRObject' + var co = ManagedType.GetManagedObject(inst) as CLRObject; + + // Sanity check: this ensures a graceful exit if someone does + // something intentionally wrong like call a non-static method + // on the class rather than on an instance of the class. + // XXX maybe better to do this before all the other rigmarole. + if (co == null) + { + return null; + } + target = co.inst; + } + + return new Binding(mi, target, margs, outs); + } + // We weren't able to find a matching method but at least one + // is a generic method and info is null. That happens when a generic + // method was not called using the [] syntax. Let's introspect the + // type of the arguments and use it to construct the correct method. + if (isGeneric && info == null && methodinfo != null) + { + Type[] types = Runtime.PythonArgsToTypeArray(args, true); + MethodInfo mi = MatchParameters(methodinfo, types); + return Bind(inst, args, kw, mi, null); + } + return null; + } + + /// + /// Attempts to convert Python positional argument tuple and keyword argument table + /// into an array of managed objects, that can be passed to a method. + /// + /// Information about expected parameters + /// true, if the last parameter is a params array. + /// A pointer to the Python argument tuple + /// Number of arguments, passed by Python + /// Dictionary of keyword argument name to python object pointer + /// A list of default values for omitted parameters + /// true, if overloading resolution is required + /// Returns number of output parameters + /// An array of .NET arguments, that can be passed to a method. + static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, + IntPtr args, int pyArgCount, + Dictionary kwargDict, + ArrayList defaultArgList, + bool needsResolution, + out int outs) + { + outs = 0; + var margs = new object[pi.Length]; + int arrayStart = paramsArray ? pi.Length - 1 : -1; + for (int paramIndex = 0; paramIndex < pi.Length; paramIndex++) + { + var parameter = pi[paramIndex]; + bool hasNamedParam = kwargDict.ContainsKey(parameter.Name); + + if (paramIndex >= pyArgCount && !hasNamedParam) + { + if (defaultArgList != null) + { + margs[paramIndex] = defaultArgList[paramIndex - pyArgCount]; + } + + continue; + } + + IntPtr op; + if (hasNamedParam) + { + op = kwargDict[parameter.Name]; + } + else + { + op = (arrayStart == paramIndex) + // map remaining Python arguments to a tuple since + // the managed function accepts it - hopefully :] + ? Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount) + : Runtime.PyTuple_GetItem(args, paramIndex); + } + + bool isOut; + if (!TryConvertArgument(op, parameter.ParameterType, needsResolution, out margs[paramIndex], out isOut)) + { + return null; + } + + if (arrayStart == paramIndex) + { + // TODO: is this a bug? Should this happen even if the conversion fails? + // GetSlice() creates a new reference but GetItem() + // returns only a borrow reference. + Runtime.XDecref(op); + } for (var n = 0; n < clrnargs; n++) { IntPtr op; @@ -525,26 +657,60 @@ private bool CheckMethodArgumentsMatch(int clrnargs, int pynargs, ParameterInfo[] parameterInfo, out int arrayStart, + static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] parameters, + Dictionary kwargDict, + out bool paramsArray, out ArrayList defaultArgList) { arrayStart = -1; defaultArgList = null; var match = false; + paramsArray = false; + + if (positionalArgumentCount == parameters.Length) if (pynargs == clrnargs) { match = true; } + else if (positionalArgumentCount < parameters.Length) + } else if (pynargs < clrnargs) { + // every parameter past 'positionalArgumentCount' must have either + // a corresponding keyword argument or a default parameter match = true; defaultArgList = new ArrayList(); + for (var v = positionalArgumentCount; v < parameters.Length; v++) + { + if (kwargDict.ContainsKey(parameters[v].Name)) + { + // we have a keyword argument for this parameter, + // no need to check for a default parameter, but put a null + // placeholder in defaultArgList + defaultArgList.Add(null); + } + else if (parameters[v].IsOptional) + { + // IsOptional will be true if the parameter has a default value, + // or if the parameter has the [Optional] attribute specified. + // The GetDefaultValue() extension method will return the value + // to be passed in as the parameter value + defaultArgList.Add(parameters[v].GetDefaultValue()); + } + else + { for (var v = pynargs; v < clrnargs && match; v++) { if (parameterInfo[v].DefaultValue == DBNull.Value) { match = false; } + } + } + else if (positionalArgumentCount > parameters.Length && parameters.Length > 0 && + Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute))) + } else { defaultArgList.Add(parameterInfo[v].DefaultValue); @@ -729,4 +895,33 @@ internal Binding(MethodBase info, object inst, object[] args, int outs) this.outs = outs; } } + + + static internal class ParameterInfoExtensions + { + public static object GetDefaultValue(this ParameterInfo parameterInfo) + { + // parameterInfo.HasDefaultValue is preferable but doesn't exist in .NET 4.0 + bool hasDefaultValue = (parameterInfo.Attributes & ParameterAttributes.HasDefault) == + ParameterAttributes.HasDefault; + + if (hasDefaultValue) + { + return parameterInfo.DefaultValue; + } + else + { + // [OptionalAttribute] was specified for the parameter. + // See https://stackoverflow.com/questions/3416216/optionalattribute-parameters-default-value + // for rules on determining the value to pass to the parameter + var type = parameterInfo.ParameterType; + if (type == typeof(object)) + return Type.Missing; + else if (type.IsValueType) + return Activator.CreateInstance(type); + else + return null; + } + } + } } diff --git a/src/testing/methodtest.cs b/src/testing/methodtest.cs index cf653f9f9..91836b727 100644 --- a/src/testing/methodtest.cs +++ b/src/testing/methodtest.cs @@ -1,5 +1,6 @@ using System; using System.IO; +using System.Runtime.InteropServices; namespace Python.Test { @@ -651,6 +652,38 @@ public static string Casesensitive() { return "Casesensitive"; } + + public static string DefaultParams(int a=0, int b=0, int c=0, int d=0) + { + return string.Format("{0}{1}{2}{3}", a, b, c, d); + } + + public static string OptionalParams([Optional]int a, [Optional]int b, [Optional]int c, [Optional] int d) + { + return string.Format("{0}{1}{2}{3}", a, b, c, d); + } + + public static bool OptionalParams_TestMissing([Optional]object a) + { + return a == Type.Missing; + } + + public static bool OptionalParams_TestReferenceType([Optional]string a) + { + return a == null; + } + + public static string OptionalAndDefaultParams([Optional]int a, [Optional]int b, int c=0, int d=0) + { + return string.Format("{0}{1}{2}{3}", a, b, c, d); + } + + public static string OptionalAndDefaultParams2([Optional]int a, [Optional]int b, [Optional, DefaultParameterValue(1)]int c, int d = 2) + { + return string.Format("{0}{1}{2}{3}", a, b, c, d); + } + + } diff --git a/src/tests/test_method.py b/src/tests/test_method.py index ad678611b..34f460d59 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -776,6 +776,9 @@ def test_no_object_in_param(): res = MethodTest.TestOverloadedNoObject(5) assert res == "Got int" + + res = MethodTest.TestOverloadedNoObject(i=7) + assert res == "Got int" with pytest.raises(TypeError): MethodTest.TestOverloadedNoObject("test") @@ -787,9 +790,15 @@ def test_object_in_param(): res = MethodTest.TestOverloadedObject(5) assert res == "Got int" + + res = MethodTest.TestOverloadedObject(i=7) + assert res == "Got int" res = MethodTest.TestOverloadedObject("test") assert res == "Got object" + + res = MethodTest.TestOverloadedObject(o="test") + assert res == "Got object" def test_object_in_multiparam(): @@ -813,6 +822,42 @@ def test_object_in_multiparam(): res = MethodTest.TestOverloadedObjectTwo(7.24, 7.24) assert res == "Got object-object" + res = MethodTest.TestOverloadedObjectTwo(a=5, b=5) + assert res == "Got int-int" + + res = MethodTest.TestOverloadedObjectTwo(5, b=5) + assert res == "Got int-int" + + res = MethodTest.TestOverloadedObjectTwo(a=5, b="foo") + assert res == "Got int-string" + + res = MethodTest.TestOverloadedObjectTwo(5, b="foo") + assert res == "Got int-string" + + res = MethodTest.TestOverloadedObjectTwo(a="foo", b=7.24) + assert res == "Got string-object" + + res = MethodTest.TestOverloadedObjectTwo("foo", b=7.24) + assert res == "Got string-object" + + res = MethodTest.TestOverloadedObjectTwo(a="foo", b="bar") + assert res == "Got string-string" + + res = MethodTest.TestOverloadedObjectTwo("foo", b="bar") + assert res == "Got string-string" + + res = MethodTest.TestOverloadedObjectTwo(a="foo", b=5) + assert res == "Got string-int" + + res = MethodTest.TestOverloadedObjectTwo("foo", b=5) + assert res == "Got string-int" + + res = MethodTest.TestOverloadedObjectTwo(a=7.24, b=7.24) + assert res == "Got object-object" + + res = MethodTest.TestOverloadedObjectTwo(7.24, b=7.24) + assert res == "Got object-object" + def test_object_in_multiparam_exception(): """Test method with object multiparams behaves""" @@ -966,3 +1011,120 @@ def test_getting_overloaded_constructor_binding_does_not_leak_ref_count(): # simple test refCount = sys.getrefcount(PlainOldClass.Overloads[int]) assert refCount == 1 + + +def test_default_params(): + # all positional parameters + res = MethodTest.DefaultParams(1,2,3,4) + assert res == "1234" + + res = MethodTest.DefaultParams(1, 2, 3) + assert res == "1230" + + res = MethodTest.DefaultParams(1, 2) + assert res == "1200" + + res = MethodTest.DefaultParams(1) + assert res == "1000" + + res = MethodTest.DefaultParams(a=2) + assert res == "2000" + + res = MethodTest.DefaultParams(b=3) + assert res == "0300" + + res = MethodTest.DefaultParams(c=4) + assert res == "0040" + + res = MethodTest.DefaultParams(d=7) + assert res == "0007" + + res = MethodTest.DefaultParams(a=2, c=5) + assert res == "2050" + + res = MethodTest.DefaultParams(1, d=7, c=3) + assert res == "1037" + + with pytest.raises(TypeError): + MethodTest.DefaultParams(1,2,3,4,5) + +def test_optional_params(): + res = MethodTest.OptionalParams(1, 2, 3, 4) + assert res == "1234" + + res = MethodTest.OptionalParams(1, 2, 3) + assert res == "1230" + + res = MethodTest.OptionalParams(1, 2) + assert res == "1200" + + res = MethodTest.OptionalParams(1) + assert res == "1000" + + res = MethodTest.OptionalParams(a=2) + assert res == "2000" + + res = MethodTest.OptionalParams(b=3) + assert res == "0300" + + res = MethodTest.OptionalParams(c=4) + assert res == "0040" + + res = MethodTest.OptionalParams(d=7) + assert res == "0007" + + res = MethodTest.OptionalParams(a=2, c=5) + assert res == "2050" + + res = MethodTest.OptionalParams(1, d=7, c=3) + assert res == "1037" + + res = MethodTest.OptionalParams_TestMissing() + assert res == True + + res = MethodTest.OptionalParams_TestMissing(None) + assert res == False + + res = MethodTest.OptionalParams_TestMissing(a = None) + assert res == False + + res = MethodTest.OptionalParams_TestMissing(a='hi') + assert res == False + + res = MethodTest.OptionalParams_TestReferenceType() + assert res == True + + res = MethodTest.OptionalParams_TestReferenceType(None) + assert res == True + + res = MethodTest.OptionalParams_TestReferenceType(a=None) + assert res == True + + res = MethodTest.OptionalParams_TestReferenceType('hi') + assert res == False + + res = MethodTest.OptionalParams_TestReferenceType(a='hi') + assert res == False + +def test_optional_and_default_params(): + + res = MethodTest.OptionalAndDefaultParams() + assert res == "0000" + + res = MethodTest.OptionalAndDefaultParams(1) + assert res == "1000" + + res = MethodTest.OptionalAndDefaultParams(1, c=4) + assert res == "1040" + + res = MethodTest.OptionalAndDefaultParams(b=4, c=7) + assert res == "0470" + + res = MethodTest.OptionalAndDefaultParams2() + assert res == "0012" + + res = MethodTest.OptionalAndDefaultParams2(a=1,b=2,c=3,d=4) + assert res == "1234" + + res = MethodTest.OptionalAndDefaultParams2(b=2, c=3) + assert res == "0232" From d76bc603b23f91864694fcc580a2342977e2eec7 Mon Sep 17 00:00:00 2001 From: AlexCatarino Date: Thu, 19 Dec 2019 21:46:09 +0000 Subject: [PATCH 2/5] Adds Support to Named Arguments Pick-cherry from pythonnet master and resolve conflicts. --- src/runtime/methodbinder.cs | 446 ++++++++++++++++-------------------- 1 file changed, 202 insertions(+), 244 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 199a6412c..424cb85cf 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -3,8 +3,6 @@ using System.Collections.Generic; using System.Reflection; using System.Text; -using System.Collections.Generic; -using System.Linq; namespace Python.Runtime { @@ -294,9 +292,6 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info) internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, MethodInfo[] methodinfo) { - // loop to find match, return invoker w/ or /wo error - MethodBase[] _methods = null; - var kwargDict = new Dictionary(); if (kw != IntPtr.Zero) { @@ -314,10 +309,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth var pynargs = (int)Runtime.PyTuple_Size(args); var isGeneric = false; - ArrayList defaultArgList; - Type clrtype; Binding bindingUsingImplicitConversion = null; - var methods = info == null ? GetMethods() : new List(1) { new MethodInformation(info, info.GetParameters()) }; @@ -331,26 +323,18 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth { isGeneric = true; } - int clrnargs = pi.Length; - int arrayStart; - - if (CheckMethodArgumentsMatch(clrnargs, - pynargs, - pi, - out arrayStart, - out defaultArgList)) - { - var outs = 0; - var margs = new object[clrnargs]; - var usedImplicitConversion = false; + ArrayList defaultArgList; + bool paramsArray; if (!MatchesArgumentCount(pynargs, pi, kwargDict, out paramsArray, out defaultArgList)) { continue; } var outs = 0; + var usedImplicitConversion = false; var margs = TryConvertArguments(pi, paramsArray, args, pynargs, kwargDict, defaultArgList, - needsResolution: _methods.Length > 1, - outs: out outs); + needsResolution: methods.Count > 1, + outs: out outs, + usedImplicitConversion: out usedImplicitConversion); if (margs == null) { @@ -376,8 +360,30 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth target = co.inst; } - return new Binding(mi, target, margs, outs); + var binding = new Binding(mi, target, margs, outs); + if (usedImplicitConversion) + { + // lets just keep the first binding using implicit conversion + // this is to respect method order/precedence + if (bindingUsingImplicitConversion == null) + { + // in this case we will not return the binding yet in case there is a match + // which does not use implicit conversions, which will return directly + bindingUsingImplicitConversion = binding; + } + } + else + { + return binding; + } + } + + // if we generated a binding using implicit conversion return it + if (bindingUsingImplicitConversion != null) + { + return bindingUsingImplicitConversion; } + // We weren't able to find a matching method but at least one // is a generic method and info is null. That happens when a generic // method was not called using the [] syntax. Let's introspect the @@ -409,9 +415,11 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, Dictionary kwargDict, ArrayList defaultArgList, bool needsResolution, - out int outs) + out int outs, + out bool usedImplicitConversion) { outs = 0; + usedImplicitConversion = false; var margs = new object[pi.Length]; int arrayStart = paramsArray ? pi.Length - 1 : -1; @@ -445,11 +453,14 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, } bool isOut; - if (!TryConvertArgument(op, parameter.ParameterType, needsResolution, out margs[paramIndex], out isOut)) + bool usedImplicitConversionForArgument; + if (!TryConvertArgument(op, parameter.ParameterType, needsResolution, out margs[paramIndex], out isOut, out usedImplicitConversionForArgument)) { return null; } + usedImplicitConversion |= usedImplicitConversionForArgument; + if (arrayStart == paramIndex) { // TODO: is this a bug? Should this happen even if the conversion fails? @@ -457,225 +468,145 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, // returns only a borrow reference. Runtime.XDecref(op); } - for (var n = 0; n < clrnargs; n++) - { - IntPtr op; - if (n < pynargs) - { - if (arrayStart == n) - { - // map remaining Python arguments to a tuple since - // the managed function accepts it - hopefully :] - op = Runtime.PyTuple_GetSlice(args, arrayStart, pynargs); - } - else - { - op = Runtime.PyTuple_GetItem(args, n); - } + if (parameter.IsOut || isOut) + { + outs++; + } + } - // this logic below handles cases when multiple overloading methods - // are ambiguous, hence comparison between Python and CLR types - // is necessary - clrtype = null; - IntPtr pyoptype; - if (methods.Count > 1) - { - pyoptype = IntPtr.Zero; - pyoptype = Runtime.PyObject_Type(op); - Exceptions.Clear(); - if (pyoptype != IntPtr.Zero) - { - clrtype = Converter.GetTypeByAlias(pyoptype); - } - Runtime.XDecref(pyoptype); - } + return margs; + } + static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResolution, + out object arg, out bool isOut, out bool usedImplicitConversion) + { + arg = null; + isOut = false; + usedImplicitConversion = false; + var clrtype = TryComputeClrArgumentType(parameterType, op, needsResolution: needsResolution, + usedImplicitConversion: out usedImplicitConversion); + if (clrtype == null) + { + return false; + } - if (clrtype != null) - { - var typematch = false; - if ((pi[n].ParameterType != typeof(object)) && (pi[n].ParameterType != clrtype)) - { - IntPtr pytype = Converter.GetPythonTypeByAlias(pi[n].ParameterType); - pyoptype = Runtime.PyObject_Type(op); - Exceptions.Clear(); - if (pyoptype != IntPtr.Zero) - { - if (pytype != pyoptype) - { - typematch = false; - } - else - { - typematch = true; - clrtype = pi[n].ParameterType; - } - } - if (!typematch) - { - // this takes care of nullables - var underlyingType = Nullable.GetUnderlyingType(pi[n].ParameterType); - if (underlyingType == null) - { - underlyingType = pi[n].ParameterType; - } - // this takes care of enum values - TypeCode argtypecode = Type.GetTypeCode(underlyingType); - TypeCode paramtypecode = Type.GetTypeCode(clrtype); - if (argtypecode == paramtypecode) - { - typematch = true; - clrtype = pi[n].ParameterType; - } - // accepts non-decimal numbers in decimal parameters - if (underlyingType == typeof(decimal)) - { - clrtype = pi[n].ParameterType; - typematch = Converter.ToManaged(op, clrtype, out arg, false); - } - // this takes care of implicit conversions - var opImplicit = pi[n].ParameterType.GetMethod("op_Implicit", new[] { clrtype }); - if (opImplicit != null) - { - usedImplicitConversion = typematch = opImplicit.ReturnType == pi[n].ParameterType; - clrtype = pi[n].ParameterType; - } - } - Runtime.XDecref(pyoptype); - if (!typematch) - { - margs = null; - break; - } - } - else - { - typematch = true; - clrtype = pi[n].ParameterType; - } - } - else - { - clrtype = pi[n].ParameterType; - } + if (!Converter.ToManaged(op, clrtype, out arg, false)) + { + Exceptions.Clear(); + return false; + } - if (pi[n].IsOut || clrtype.IsByRef) - { - outs++; - } + isOut = clrtype.IsByRef; + return true; + } - if (!Converter.ToManaged(op, clrtype, out arg, false)) - { - Exceptions.Clear(); - margs = null; - break; - } - if (arrayStart == n) - { - // GetSlice() creates a new reference but GetItem() - // returns only a borrow reference. - Runtime.XDecref(op); - } - margs[n] = arg; + static Type TryComputeClrArgumentType(Type parameterType, IntPtr argument, bool needsResolution, + out bool usedImplicitConversion) + { + // 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; + usedImplicitConversion = false; + if (needsResolution) + { + // HACK: each overload should be weighted in some way instead + pyoptype = Runtime.PyObject_Type(argument); + Exceptions.Clear(); + if (pyoptype != IntPtr.Zero) + { + clrtype = Converter.GetTypeByAlias(pyoptype); + } + Runtime.XDecref(pyoptype); + } + + + if (clrtype != null) + { + var typematch = false; + if ((parameterType != typeof(object)) && (parameterType != clrtype)) + { + IntPtr pytype = Converter.GetPythonTypeByAlias(parameterType); + pyoptype = Runtime.PyObject_Type(argument); + Exceptions.Clear(); + if (pyoptype != IntPtr.Zero) + { + if (pytype != pyoptype) + { + typematch = false; } else { - if (defaultArgList != null) - { - margs[n] = defaultArgList[n - pynargs]; - } + typematch = true; + clrtype = parameterType; } } - - if (margs == null) - { - continue; - } - - object target = null; - if (!mi.IsStatic && inst != IntPtr.Zero) + if (!typematch) { - //CLRObject co = (CLRObject)ManagedType.GetManagedObject(inst); - // InvalidCastException: Unable to cast object of type - // 'Python.Runtime.ClassObject' to type 'Python.Runtime.CLRObject' - var co = ManagedType.GetManagedObject(inst) as CLRObject; - - // Sanity check: this ensures a graceful exit if someone does - // something intentionally wrong like call a non-static method - // on the class rather than on an instance of the class. - // XXX maybe better to do this before all the other rigmarole. - if (co == null) + // this takes care of nullables + var underlyingType = Nullable.GetUnderlyingType(parameterType); + if (underlyingType == null) { - return null; + underlyingType = parameterType; } - target = co.inst; - } - - var binding = new Binding(mi, target, margs, outs); - if (usedImplicitConversion) - { - // lets just keep the first binding using implicit conversion - // this is to respect method order/precedence - if (bindingUsingImplicitConversion == null) + // this takes care of enum values + TypeCode argtypecode = Type.GetTypeCode(underlyingType); + TypeCode paramtypecode = Type.GetTypeCode(clrtype); + if (argtypecode == paramtypecode) { - // in this case we will not return the binding yet in case there is a match - // which does not use implicit conversions, which will return directly - bindingUsingImplicitConversion = binding; + typematch = true; + clrtype = parameterType; + } + // accepts non-decimal numbers in decimal parameters + if (underlyingType == typeof(decimal)) + { + object arg; + clrtype = parameterType; + typematch = Converter.ToManaged(argument, clrtype, out arg, false); + } + // this takes care of implicit conversions + var opImplicit = parameterType.GetMethod("op_Implicit", new[] {clrtype}); + if (opImplicit != null) + { + usedImplicitConversion = typematch = opImplicit.ReturnType == parameterType; + clrtype = parameterType; } } - else + Runtime.XDecref(pyoptype); + if (!typematch) { - return binding; + return null; } } + else + { + typematch = true; + clrtype = parameterType; + } } - - // if we generated a binding using implicit conversion return it - if (bindingUsingImplicitConversion != null) + else { - return bindingUsingImplicitConversion; + clrtype = parameterType; } - // We weren't able to find a matching method but at least one - // is a generic method and info is null. That happens when a generic - // method was not called using the [] syntax. Let's introspect the - // type of the arguments and use it to construct the correct method. - if (isGeneric && info == null && methodinfo != null) - { - Type[] types = Runtime.PythonArgsToTypeArray(args, true); - MethodInfo mi = MatchParameters(methodinfo, types); - return Bind(inst, args, kw, mi, null); - } - return null; + return clrtype; } - /// - /// This helper method will perform an initial check to determine if we found a matching - /// method based on its parameters count and type - /// - private bool CheckMethodArgumentsMatch(int clrnargs, - int pynargs, - ParameterInfo[] parameterInfo, - out int arrayStart, static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] parameters, Dictionary kwargDict, out bool paramsArray, out ArrayList defaultArgList) { - arrayStart = -1; defaultArgList = null; - var match = false; paramsArray = false; if (positionalArgumentCount == parameters.Length) - if (pynargs == clrnargs) { match = true; } else if (positionalArgumentCount < parameters.Length) - } - else if (pynargs < clrnargs) { // every parameter past 'positionalArgumentCount' must have either // a corresponding keyword argument or a default parameter @@ -699,30 +630,17 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa defaultArgList.Add(parameters[v].GetDefaultValue()); } else - { - for (var v = pynargs; v < clrnargs && match; v++) - { - if (parameterInfo[v].DefaultValue == DBNull.Value) { match = false; } } } else if (positionalArgumentCount > parameters.Length && parameters.Length > 0 && - Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute))) - } - else - { - defaultArgList.Add(parameterInfo[v].DefaultValue); - } - } - } - else if (pynargs > clrnargs && clrnargs > 0 && - Attribute.IsDefined(parameterInfo[clrnargs - 1], typeof(ParamArrayAttribute))) + Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute))) { // This is a `foo(params object[] bar)` style method match = true; - arrayStart = clrnargs - 1; + paramsArray = true; } return match; @@ -746,12 +664,44 @@ internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw, MethodBase i if (binding == null) { - var value = "No method matches given arguments"; + var value = new StringBuilder("No method matches given arguments"); if (methodinfo != null && methodinfo.Length > 0) { - value += $" for {methodinfo[0].Name}"; + value.Append($" for {methodinfo[0].Name}"); + } + + long argCount = Runtime.PyTuple_Size(args); + value.Append(": ("); + for (var argIndex = 0; argIndex < argCount; argIndex++) + { + var arg = Runtime.PyTuple_GetItem(args, argIndex); + if (arg != IntPtr.Zero) + { + var type = Runtime.PyObject_Type(arg); + if (type != IntPtr.Zero) + { + try + { + var description = Runtime.PyObject_Unicode(type); + if (description != IntPtr.Zero) + { + value.Append(Runtime.GetManagedString(description)); + Runtime.XDecref(description); + } + } + finally + { + Runtime.XDecref(type); + } + } + } + + if (argIndex + 1 < argCount) + value.Append(", "); } - Exceptions.SetError(Exceptions.TypeError, value); + + value.Append(')'); + Exceptions.SetError(Exceptions.TypeError, value.ToString()); return IntPtr.Zero; } @@ -872,38 +822,46 @@ public int Compare(MethodInformation x, MethodInformation y) return 0; } } - } - - /// - /// A Binding is a utility instance that bundles together a MethodInfo - /// representing a method to call, a (possibly null) target instance for - /// the call, and the arguments for the call (all as managed values). - /// - internal class Binding - { - public MethodBase info; - public object[] args; - public object inst; - public int outs; - internal Binding(MethodBase info, object inst, object[] args, int outs) + /// + /// A Binding is a utility instance that bundles together a MethodInfo + /// representing a method to call, a (possibly null) target instance for + /// the call, and the arguments for the call (all as managed values). + /// + internal class Binding { - this.info = info; - this.inst = inst; - this.args = args; - this.outs = outs; + public MethodBase info; + public object[] args; + public object inst; + public int outs; + + internal Binding(MethodBase info, object inst, object[] args, int outs) + { + this.info = info; + this.inst = inst; + this.args = args; + this.outs = outs; + } } } - static internal class ParameterInfoExtensions { public static object GetDefaultValue(this ParameterInfo parameterInfo) { + bool hasDefaultValue; + // parameterInfo.HasDefaultValue is preferable but doesn't exist in .NET 4.0 - bool hasDefaultValue = (parameterInfo.Attributes & ParameterAttributes.HasDefault) == - ParameterAttributes.HasDefault; + try + { + hasDefaultValue = parameterInfo.HasDefaultValue; + } + catch + { + hasDefaultValue = (parameterInfo.Attributes & ParameterAttributes.HasDefault) == + ParameterAttributes.HasDefault; + } if (hasDefaultValue) { From 912bbb73bcd2d2c2a735c78d239761dde7fa9aa5 Mon Sep 17 00:00:00 2001 From: AlexCatarino Date: Thu, 19 Dec 2019 21:46:42 +0000 Subject: [PATCH 3/5] Version bump 1.0.5.30 --- .bumpversion.cfg | 2 +- conda.recipe/meta.yaml | 2 +- setup.py | 2 +- src/SharedAssemblyInfo.cs | 2 +- src/clrmodule/ClrModule.cs | 2 +- src/runtime/assemblymanager.cs | 1 + src/runtime/resources/clr.py | 2 +- 7 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index f0a2abed2..01b9ae991 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.0.5.29 +current_version = 1.0.5.30 parse = (?P\d+)\.(?P\d+)\.(?P\d+)(\.(?P[a-z]+)(?P\d+))? serialize = {major}.{minor}.{patch}.{release}{dev} diff --git a/conda.recipe/meta.yaml b/conda.recipe/meta.yaml index 61f336080..e2c407435 100644 --- a/conda.recipe/meta.yaml +++ b/conda.recipe/meta.yaml @@ -1,6 +1,6 @@ package: name: pythonnet - version: "1.0.5.29" + version: "1.0.5.30" build: skip: True # [not win] diff --git a/setup.py b/setup.py index 512771fa9..8e72acc96 100644 --- a/setup.py +++ b/setup.py @@ -485,7 +485,7 @@ def run(self): setup( name="pythonnet", - version="1.0.5.29", + version="1.0.5.30", description=".Net and Mono integration for Python", url='https://pythonnet.github.io/', license='MIT', diff --git a/src/SharedAssemblyInfo.cs b/src/SharedAssemblyInfo.cs index a8f3149df..3d2192f56 100644 --- a/src/SharedAssemblyInfo.cs +++ b/src/SharedAssemblyInfo.cs @@ -25,4 +25,4 @@ // Version Information. Keeping it simple. May need to revisit for Nuget // See: https://codingforsmarties.wordpress.com/2016/01/21/how-to-version-assemblies-destined-for-nuget/ // AssemblyVersion can only be numeric -[assembly: AssemblyVersion("1.0.5.29")] +[assembly: AssemblyVersion("1.0.5.30")] diff --git a/src/clrmodule/ClrModule.cs b/src/clrmodule/ClrModule.cs index 6387df7e5..62ec1ad42 100644 --- a/src/clrmodule/ClrModule.cs +++ b/src/clrmodule/ClrModule.cs @@ -53,7 +53,7 @@ public static void initclr() { #if USE_PYTHON_RUNTIME_VERSION // Has no effect until SNK works. Keep updated anyways. - Version = new Version("1.0.5.29"), + Version = new Version("1.0.5.30"), #endif CultureInfo = CultureInfo.InvariantCulture }; diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index a22db843d..069950676 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -69,6 +69,7 @@ internal static void Initialize() // lets wait until all assemblies are loaded do { + break; if (safeCount++ > 200) { throw new TimeoutException("Timeout while waiting for assemblies to load"); diff --git a/src/runtime/resources/clr.py b/src/runtime/resources/clr.py index 01d0d409b..3e35b13ab 100644 --- a/src/runtime/resources/clr.py +++ b/src/runtime/resources/clr.py @@ -2,7 +2,7 @@ Code in this module gets loaded into the main clr module. """ -__version__ = "1.0.5.29" +__version__ = "1.0.5.30" class clrproperty(object): From 518c414edd50138bac2002a1cf1ea3c3e75d3f30 Mon Sep 17 00:00:00 2001 From: AlexCatarino Date: Thu, 19 Dec 2019 23:35:14 +0000 Subject: [PATCH 4/5] Addresses Peer-Review Remoes `break` at `AssemblyManager.Initialize` added for debugging/testing. --- src/runtime/assemblymanager.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs index 069950676..a22db843d 100644 --- a/src/runtime/assemblymanager.cs +++ b/src/runtime/assemblymanager.cs @@ -69,7 +69,6 @@ internal static void Initialize() // lets wait until all assemblies are loaded do { - break; if (safeCount++ > 200) { throw new TimeoutException("Timeout while waiting for assemblies to load"); From 04e6927d1721bf5e0a25403da0a64314697690e6 Mon Sep 17 00:00:00 2001 From: AlexCatarino Date: Mon, 23 Dec 2019 20:40:11 +0000 Subject: [PATCH 5/5] Addresses Peer-Review --- src/runtime/methodbinder.cs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 424cb85cf..dcd3f1313 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -292,10 +292,12 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info) internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, MethodInfo[] methodinfo) { - var kwargDict = new Dictionary(); + Dictionary kwargDict = null; + if (kw != IntPtr.Zero) { var pynkwargs = (int)Runtime.PyDict_Size(kw); + kwargDict = new Dictionary(pynkwargs); IntPtr keylist = Runtime.PyDict_Keys(kw); IntPtr valueList = Runtime.PyDict_Values(kw); for (int i = 0; i < pynkwargs; ++i) @@ -409,6 +411,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth /// A list of default values for omitted parameters /// true, if overloading resolution is required /// Returns number of output parameters + /// Returns true if the type of one of the parameters can be implicitly converted into another type /// An array of .NET arguments, that can be passed to a method. static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, IntPtr args, int pyArgCount, @@ -426,7 +429,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, for (int paramIndex = 0; paramIndex < pi.Length; paramIndex++) { var parameter = pi[paramIndex]; - bool hasNamedParam = kwargDict.ContainsKey(parameter.Name); + bool hasNamedParam = kwargDict != null && kwargDict.ContainsKey(parameter.Name); if (paramIndex >= pyArgCount && !hasNamedParam) { @@ -482,7 +485,6 @@ static bool TryConvertArgument(IntPtr op, Type parameterType, bool needsResoluti { arg = null; isOut = false; - usedImplicitConversion = false; var clrtype = TryComputeClrArgumentType(parameterType, op, needsResolution: needsResolution, usedImplicitConversion: out usedImplicitConversion); if (clrtype == null) @@ -599,22 +601,20 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa out ArrayList defaultArgList) { defaultArgList = null; - var match = false; paramsArray = false; if (positionalArgumentCount == parameters.Length) { - match = true; + return true; } - else if (positionalArgumentCount < parameters.Length) + if (positionalArgumentCount < parameters.Length) { // every parameter past 'positionalArgumentCount' must have either // a corresponding keyword argument or a default parameter - match = true; defaultArgList = new ArrayList(); for (var v = positionalArgumentCount; v < parameters.Length; v++) { - if (kwargDict.ContainsKey(parameters[v].Name)) + if (kwargDict != null && kwargDict.ContainsKey(parameters[v].Name)) { // we have a keyword argument for this parameter, // no need to check for a default parameter, but put a null @@ -631,19 +631,20 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa } else { - match = false; + return false; } } + return true; } - else if (positionalArgumentCount > parameters.Length && parameters.Length > 0 && - Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute))) + if (positionalArgumentCount > parameters.Length && parameters.Length > 0 && + Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute))) { // This is a `foo(params object[] bar)` style method - match = true; paramsArray = true; + return true; } - return match; + return false; } internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw)