From 6beaf82268b8020bef31044e93cc5afaed67be07 Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Tue, 7 Apr 2020 14:42:14 -0700 Subject: [PATCH 1/7] Add check for ParamArrayAttribute --- src/runtime/methodbinder.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 4e8698da1..78931e424 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -398,7 +398,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, var parameter = pi[paramIndex]; bool hasNamedParam = kwargDict.ContainsKey(parameter.Name); - if (paramIndex >= pyArgCount && !hasNamedParam) + if (paramIndex >= pyArgCount && !(hasNamedParam || (paramsArray && paramIndex == arrayStart))) { if (defaultArgList != null) { @@ -572,6 +572,10 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa // to be passed in as the parameter value defaultArgList.Add(parameters[v].GetDefaultValue()); } + else if(v == parameters.Length - 1 && parameters[v].IsDefined(typeof(ParamArrayAttribute), false)) + { + paramsArray = true; + } else { match = false; From 0392fdc1091f7baa42bfe47d0bcc6ec07e4582a5 Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Tue, 7 Apr 2020 18:29:56 -0700 Subject: [PATCH 2/7] Fix #943 Add a check to see if the last parameter is a params parameter. Set the correct flag to show that it is a paramsArray function. --- src/runtime/methodbinder.cs | 2 +- src/tests/test_method.py | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 78931e424..91e590130 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -572,7 +572,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa // to be passed in as the parameter value defaultArgList.Add(parameters[v].GetDefaultValue()); } - else if(v == parameters.Length - 1 && parameters[v].IsDefined(typeof(ParamArrayAttribute), false)) + else if((v == (parameters.Length - 1)) && Attribute.IsDefined(parameters[v], typeof(ParamArrayAttribute))) { paramsArray = true; } diff --git a/src/tests/test_method.py b/src/tests/test_method.py index 34f460d59..b48e301a5 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -257,6 +257,17 @@ def test_non_params_array_in_last_place(): result = MethodTest.TestNonParamsArrayInLastPlace(1, 2, 3) assert result +def test_params_methods_with_no_params(): + """Tests that passing no arguments to a params method + passes an empty array""" + result = MethodTest.TestValueParamsArg() + assert len(result) == 0 + + result = MethodTest.TestOneArgWithParams('Some String') + assert len(result) == 0 + + result = MethodTest.TestTwoArgWithParams('Some String', 'Some Other String') + assert len(result) == 0 def test_string_out_params(): """Test use of string out-parameters.""" From 5a762738c6ad673f43e08dc1a2e5ec94dfec4ac5 Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Tue, 7 Apr 2020 18:32:16 -0700 Subject: [PATCH 3/7] Add CHANGELOG entry --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db126bd1c..ea90c62f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,7 +31,8 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. together with Nuitka - Fixes bug where delegates get casts (dotnetcore) - Determine size of interpreter longs at runtime -- Handling exceptions ocurred in ModuleObject's getattribute +- Handling exceptions ocurred in ModuleObject's getattribute +- Fixed issue with params methods that are not passed an array. ## [2.4.0][] From e73ddc867f5adb4e44d71a79609192a14b6e570b Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Wed, 8 Apr 2020 08:07:09 -0700 Subject: [PATCH 4/7] Also fix #331 --- src/runtime/methodbinder.cs | 42 ++++++++++++++++++++++++++----------- src/tests/test_method.py | 15 +++++++++++++ 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 91e590130..2ab9e1aa3 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -397,6 +397,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, { var parameter = pi[paramIndex]; bool hasNamedParam = kwargDict.ContainsKey(parameter.Name); + bool doDecref = false; if (paramIndex >= pyArgCount && !(hasNamedParam || (paramsArray && paramIndex == arrayStart))) { @@ -415,11 +416,32 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, } 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); + if(arrayStart == paramIndex) + { + // for a params method, we may have a sequence or single/multiple items + // here we look to see if the item at the paramIndex is there or not + // and then if it is a sequence itself. + IntPtr item = Runtime.PyTuple_GetItem(args, paramIndex); + Runtime.PyErr_Clear(); + if(item != IntPtr.Zero && !Runtime.PyString_Check(item) && Runtime.PySequence_Check(item)) + { + op = item; + } + else + { + // we only need to decref in the case where we get a slice + doDecref = true; + op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); + if (item != IntPtr.Zero) + { + Runtime.XDecref(item); + } + } + } + else + { + op = Runtime.PyTuple_GetItem(args, paramIndex); + } } bool isOut; @@ -428,7 +450,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, return null; } - if (arrayStart == paramIndex) + if (doDecref) { // TODO: is this a bug? Should this happen even if the conversion fails? // GetSlice() creates a new reference but GetItem() @@ -543,7 +565,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa { defaultArgList = null; var match = false; - paramsArray = false; + paramsArray = parameters.Length > 0 ? Attribute.IsDefined(parameters[parameters.Length - 1], typeof(ParamArrayAttribute)) : false; if (positionalArgumentCount == parameters.Length) { @@ -572,11 +594,7 @@ static bool MatchesArgumentCount(int positionalArgumentCount, ParameterInfo[] pa // to be passed in as the parameter value defaultArgList.Add(parameters[v].GetDefaultValue()); } - else if((v == (parameters.Length - 1)) && Attribute.IsDefined(parameters[v], typeof(ParamArrayAttribute))) - { - paramsArray = true; - } - else + else if(!paramsArray) { match = false; } diff --git a/src/tests/test_method.py b/src/tests/test_method.py index b48e301a5..12fca6b16 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -269,6 +269,21 @@ def test_params_methods_with_no_params(): result = MethodTest.TestTwoArgWithParams('Some String', 'Some Other String') assert len(result) == 0 +def test_params_methods_with_non_lists(): + """Tests that passing single parameters to a params + method will convert into an array on the .NET side""" + result = MethodTest.TestOneArgWithParams('Some String', [1, 2, 3, 4]) + assert len(result) == 4 + + result = MethodTest.TestOneArgWithParams('Some String', 1, 2, 3, 4) + assert len(result) == 4 + + result = MethodTest.TestOneArgWithParams('Some String', [5]) + assert len(result) == 1 + + result = MethodTest.TestOneArgWithParams('Some String', 5) + assert len(result) == 1 + def test_string_out_params(): """Test use of string out-parameters.""" result = MethodTest.TestStringOutParams("hi", "there") From 4a8928eecb6b8e7db186d190423041e8a1eaf34a Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Thu, 9 Apr 2020 06:08:34 -0700 Subject: [PATCH 5/7] Fix-up based on comments in PR. --- src/runtime/methodbinder.cs | 30 ++++++++++++++++++++---------- src/tests/test_method.py | 5 +++++ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 2ab9e1aa3..163d81b66 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -421,25 +421,35 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, // for a params method, we may have a sequence or single/multiple items // here we look to see if the item at the paramIndex is there or not // and then if it is a sequence itself. - IntPtr item = Runtime.PyTuple_GetItem(args, paramIndex); - Runtime.PyErr_Clear(); - if(item != IntPtr.Zero && !Runtime.PyString_Check(item) && Runtime.PySequence_Check(item)) + if((pyArgCount - paramIndex) == 1) { - op = item; + // we only have one argument left, so we need to check it + // to see if it is a sequence or a single item + IntPtr item = Runtime.PyTuple_GetItem(args, paramIndex); + if(!Runtime.PyString_Check(item) && Runtime.PySequence_Check(item)) + { + // it's a sequence (and not a string), so we use it as the op + op = item; + } + else + { + doDecref = true; + op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); + if (item != IntPtr.Zero) + { + Runtime.XDecref(item); + } + } } else { - // we only need to decref in the case where we get a slice doDecref = true; op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); - if (item != IntPtr.Zero) - { - Runtime.XDecref(item); - } - } + } } else { + // not at a params argument op = Runtime.PyTuple_GetItem(args, paramIndex); } } diff --git a/src/tests/test_method.py b/src/tests/test_method.py index 12fca6b16..69f1b5e72 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -284,6 +284,11 @@ def test_params_methods_with_non_lists(): result = MethodTest.TestOneArgWithParams('Some String', 5) assert len(result) == 1 +def test_params_method_with_lists(): + """Tests passing multiple lists to a params object[] method""" + result = MethodTest.TestObjectParamsArg([],[]) + assert len(result) == 2 + def test_string_out_params(): """Test use of string out-parameters.""" result = MethodTest.TestStringOutParams("hi", "there") From 04f016e5e146187fe63cd3d0c69095c43c5870e3 Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Thu, 9 Apr 2020 09:46:15 -0700 Subject: [PATCH 6/7] Move params array handling into function --- src/runtime/methodbinder.cs | 65 ++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 163d81b66..2499ce5cd 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -369,6 +369,41 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth return null; } + static IntPtr HandleParamsArray(IntPtr args, int arrayStart, int pyArgCount, out bool doDecref) + { + IntPtr op = IntPtr.Zero; + doDecref = false; + // for a params method, we may have a sequence or single/multiple items + // here we look to see if the item at the paramIndex is there or not + // and then if it is a sequence itself. + if ((pyArgCount - arrayStart) == 1) + { + // we only have one argument left, so we need to check it + // to see if it is a sequence or a single item + IntPtr item = Runtime.PyTuple_GetItem(args, arrayStart); + if (!Runtime.PyString_Check(item) && Runtime.PySequence_Check(item)) + { + // it's a sequence (and not a string), so we use it as the op + op = item; + } + else + { + doDecref = true; + op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); + if (item != IntPtr.Zero) + { + Runtime.XDecref(item); + } + } + } + else + { + doDecref = true; + op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); + } + return op; + } + /// /// Attempts to convert Python positional argument tuple and keyword argument table /// into an array of managed objects, that can be passed to a method. @@ -418,38 +453,10 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, { if(arrayStart == paramIndex) { - // for a params method, we may have a sequence or single/multiple items - // here we look to see if the item at the paramIndex is there or not - // and then if it is a sequence itself. - if((pyArgCount - paramIndex) == 1) - { - // we only have one argument left, so we need to check it - // to see if it is a sequence or a single item - IntPtr item = Runtime.PyTuple_GetItem(args, paramIndex); - if(!Runtime.PyString_Check(item) && Runtime.PySequence_Check(item)) - { - // it's a sequence (and not a string), so we use it as the op - op = item; - } - else - { - doDecref = true; - op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); - if (item != IntPtr.Zero) - { - Runtime.XDecref(item); - } - } - } - else - { - doDecref = true; - op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); - } + op = HandleParamsArray(args, arrayStart, pyArgCount, out doDecref); } else { - // not at a params argument op = Runtime.PyTuple_GetItem(args, paramIndex); } } From 62c71abd1b21dc082cbd38a9ae2e27d8e92a6e07 Mon Sep 17 00:00:00 2001 From: Alex Earl Date: Thu, 9 Apr 2020 10:45:29 -0700 Subject: [PATCH 7/7] Fix name of variable to match Python docs --- src/runtime/methodbinder.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 2499ce5cd..af5e33be4 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -369,10 +369,10 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth return null; } - static IntPtr HandleParamsArray(IntPtr args, int arrayStart, int pyArgCount, out bool doDecref) + static IntPtr HandleParamsArray(IntPtr args, int arrayStart, int pyArgCount, out bool isNewReference) { - IntPtr op = IntPtr.Zero; - doDecref = false; + isNewReference = false; + IntPtr op; // for a params method, we may have a sequence or single/multiple items // here we look to see if the item at the paramIndex is there or not // and then if it is a sequence itself. @@ -388,7 +388,7 @@ static IntPtr HandleParamsArray(IntPtr args, int arrayStart, int pyArgCount, out } else { - doDecref = true; + isNewReference = true; op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); if (item != IntPtr.Zero) { @@ -398,7 +398,7 @@ static IntPtr HandleParamsArray(IntPtr args, int arrayStart, int pyArgCount, out } else { - doDecref = true; + isNewReference = true; op = Runtime.PyTuple_GetSlice(args, arrayStart, pyArgCount); } return op; @@ -432,7 +432,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, { var parameter = pi[paramIndex]; bool hasNamedParam = kwargDict.ContainsKey(parameter.Name); - bool doDecref = false; + bool isNewReference = false; if (paramIndex >= pyArgCount && !(hasNamedParam || (paramsArray && paramIndex == arrayStart))) { @@ -453,7 +453,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, { if(arrayStart == paramIndex) { - op = HandleParamsArray(args, arrayStart, pyArgCount, out doDecref); + op = HandleParamsArray(args, arrayStart, pyArgCount, out isNewReference); } else { @@ -467,7 +467,7 @@ static object[] TryConvertArguments(ParameterInfo[] pi, bool paramsArray, return null; } - if (doDecref) + if (isNewReference) { // TODO: is this a bug? Should this happen even if the conversion fails? // GetSlice() creates a new reference but GetItem()