From decc95fc7b420a03e09ed9d3071b36174c78bc97 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 6 Jan 2022 15:30:38 -0800 Subject: [PATCH] added support for byref parameters when overriding .NET methods from Python new code is emitted to 1. unpack the tuple returned from Python to extract new values for byref parameters and modify args array correspondingly 2. marshal those new values from the args array back into arguments in IL fixes https://github.com/pythonnet/pythonnet/issues/1481 --- CHANGELOG.md | 2 + src/runtime/classderived.cs | 109 ++++++++++++++++++++++++++++----- src/runtime/codegenerator.cs | 35 +++++++++++ src/runtime/delegatemanager.cs | 23 +------ src/testing/interfacetest.cs | 14 +++++ tests/test_interface.py | 14 +++++ 6 files changed, 160 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16489a9c9..cf64c3a64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. - Python operator method will call C# operator method for supported binary and unary operators ([#1324][p1324]). - Add GetPythonThreadID and Interrupt methods in PythonEngine - Ability to implement delegates with `ref` and `out` parameters in Python, by returning the modified parameter values in a tuple. ([#1355][i1355]) +- Ability to override .NET methods that have `out` or `ref` in Python by returning the modified parameter values in a tuple. ([#1481][i1481]) - `PyType` - a wrapper for Python type objects, that also permits creating new heap types from `TypeSpec` - Improved exception handling: * exceptions can now be converted with codecs @@ -879,3 +880,4 @@ This version improves performance on benchmarks significantly compared to 2.3. [i449]: https://github.com/pythonnet/pythonnet/issues/449 [i1342]: https://github.com/pythonnet/pythonnet/issues/1342 [i238]: https://github.com/pythonnet/pythonnet/issues/238 +[i1481]: https://github.com/pythonnet/pythonnet/issues/1481 diff --git a/src/runtime/classderived.cs b/src/runtime/classderived.cs index 47c9b4e0e..da1bf0f9a 100644 --- a/src/runtime/classderived.cs +++ b/src/runtime/classderived.cs @@ -429,24 +429,33 @@ private static void AddVirtualMethod(MethodInfo method, Type baseType, TypeBuild il.Emit(OpCodes.Ldloc_0); il.Emit(OpCodes.Ldc_I4, i); il.Emit(OpCodes.Ldarg, i + 1); - if (parameterTypes[i].IsValueType) + var type = parameterTypes[i]; + if (type.IsByRef) { - il.Emit(OpCodes.Box, parameterTypes[i]); + type = type.GetElementType(); + il.Emit(OpCodes.Ldobj, type); + } + if (type.IsValueType) + { + il.Emit(OpCodes.Box, type); } il.Emit(OpCodes.Stelem, typeof(object)); } il.Emit(OpCodes.Ldloc_0); + + il.Emit(OpCodes.Ldtoken, method); #pragma warning disable CS0618 // PythonDerivedType is for internal use only if (method.ReturnType == typeof(void)) { - il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeMethodVoid")); + il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod(nameof(InvokeMethodVoid))); } else { il.Emit(OpCodes.Call, - typeof(PythonDerivedType).GetMethod("InvokeMethod").MakeGenericMethod(method.ReturnType)); + typeof(PythonDerivedType).GetMethod(nameof(InvokeMethod)).MakeGenericMethod(method.ReturnType)); } #pragma warning restore CS0618 // PythonDerivedType is for internal use only + CodeGenerator.GenerateMarshalByRefsBack(il, parameterTypes); il.Emit(OpCodes.Ret); } @@ -500,35 +509,65 @@ private static void AddPythonMethod(string methodName, PyObject func, TypeBuilde argTypes.ToArray()); ILGenerator il = methodBuilder.GetILGenerator(); + il.DeclareLocal(typeof(object[])); + il.DeclareLocal(typeof(RuntimeMethodHandle)); + + // this il.Emit(OpCodes.Ldarg_0); + + // Python method to call il.Emit(OpCodes.Ldstr, methodName); + + // original method name il.Emit(OpCodes.Ldnull); // don't fall back to the base type's method + + // create args array il.Emit(OpCodes.Ldc_I4, argTypes.Count); il.Emit(OpCodes.Newarr, typeof(object)); il.Emit(OpCodes.Stloc_0); + + // fill args array for (var i = 0; i < argTypes.Count; ++i) { il.Emit(OpCodes.Ldloc_0); il.Emit(OpCodes.Ldc_I4, i); il.Emit(OpCodes.Ldarg, i + 1); - if (argTypes[i].IsValueType) + var type = argTypes[i]; + if (type.IsByRef) { - il.Emit(OpCodes.Box, argTypes[i]); + type = type.GetElementType(); + il.Emit(OpCodes.Ldobj, type); + } + if (type.IsValueType) + { + il.Emit(OpCodes.Box, type); } il.Emit(OpCodes.Stelem, typeof(object)); } + + // args array il.Emit(OpCodes.Ldloc_0); + + // method handle for the base method is null + il.Emit(OpCodes.Ldloca_S, 1); + il.Emit(OpCodes.Initobj, typeof(RuntimeMethodHandle)); + il.Emit(OpCodes.Ldloc_1); #pragma warning disable CS0618 // PythonDerivedType is for internal use only + + // invoke the method if (returnType == typeof(void)) { - il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod("InvokeMethodVoid")); + il.Emit(OpCodes.Call, typeof(PythonDerivedType).GetMethod(nameof(InvokeMethodVoid))); } else { il.Emit(OpCodes.Call, - typeof(PythonDerivedType).GetMethod("InvokeMethod").MakeGenericMethod(returnType)); + typeof(PythonDerivedType).GetMethod(nameof(InvokeMethod)).MakeGenericMethod(returnType)); } + + CodeGenerator.GenerateMarshalByRefsBack(il, argTypes); + #pragma warning restore CS0618 // PythonDerivedType is for internal use only il.Emit(OpCodes.Ret); } @@ -672,7 +711,8 @@ public class PythonDerivedType /// method binding (i.e. it has been overridden in the derived python /// class) it calls it, otherwise it calls the base method. /// - public static T? InvokeMethod(IPythonDerivedType obj, string methodName, string origMethodName, object[] args) + public static T? InvokeMethod(IPythonDerivedType obj, string methodName, string origMethodName, + object[] args, RuntimeMethodHandle methodHandle) { var self = GetPyObj(obj); @@ -698,8 +738,10 @@ public class PythonDerivedType } PyObject py_result = method.Invoke(pyargs); - disposeList.Add(py_result); - return py_result.As(); + PyTuple? result_tuple = MarshalByRefsBack(args, methodHandle, py_result, outsOffset: 1); + return result_tuple is not null + ? result_tuple[0].As() + : py_result.As(); } } } @@ -726,7 +768,7 @@ public class PythonDerivedType } public static void InvokeMethodVoid(IPythonDerivedType obj, string methodName, string origMethodName, - object[] args) + object?[] args, RuntimeMethodHandle methodHandle) { var self = GetPyObj(obj); if (null != self.Ref) @@ -736,8 +778,7 @@ public static void InvokeMethodVoid(IPythonDerivedType obj, string methodName, s try { using var pyself = new PyObject(self.CheckRun()); - PyObject method = pyself.GetAttr(methodName, Runtime.None); - disposeList.Add(method); + using PyObject method = pyself.GetAttr(methodName, Runtime.None); if (method.Reference != Runtime.None) { // if the method hasn't been overridden then it will be a managed object @@ -752,7 +793,7 @@ public static void InvokeMethodVoid(IPythonDerivedType obj, string methodName, s } PyObject py_result = method.Invoke(pyargs); - disposeList.Add(py_result); + MarshalByRefsBack(args, methodHandle, py_result, outsOffset: 0); return; } } @@ -779,6 +820,44 @@ public static void InvokeMethodVoid(IPythonDerivedType obj, string methodName, s args); } + /// + /// If the method has byref arguments, reinterprets Python return value + /// as a tuple of new values for those arguments, and updates corresponding + /// elements of array. + /// + private static PyTuple? MarshalByRefsBack(object?[] args, RuntimeMethodHandle methodHandle, PyObject pyResult, int outsOffset) + { + if (methodHandle == default) return null; + + var originalMethod = MethodBase.GetMethodFromHandle(methodHandle); + var parameters = originalMethod.GetParameters(); + PyTuple? outs = null; + int byrefIndex = 0; + for (int i = 0; i < parameters.Length; ++i) + { + Type type = parameters[i].ParameterType; + if (!type.IsByRef) + { + continue; + } + + type = type.GetElementType(); + + if (outs is null) + { + outs = new PyTuple(pyResult); + pyResult.Dispose(); + } + + args[i] = outs[byrefIndex + outsOffset].AsManagedObject(type); + byrefIndex++; + } + if (byrefIndex > 0 && outs!.Length() > byrefIndex + outsOffset) + throw new ArgumentException("Too many output parameters"); + + return outs; + } + public static T? InvokeGetProperty(IPythonDerivedType obj, string propertyName) { var self = GetPyObj(obj); diff --git a/src/runtime/codegenerator.cs b/src/runtime/codegenerator.cs index dc466bafb..d0079fabb 100644 --- a/src/runtime/codegenerator.cs +++ b/src/runtime/codegenerator.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Reflection; using System.Reflection.Emit; using System.Threading; @@ -42,5 +43,39 @@ internal TypeBuilder DefineType(string name, Type basetype) var attrs = TypeAttributes.Public; return mBuilder.DefineType(name, attrs, basetype); } + + /// + /// Generates code, that copies potentially modified objects in args array + /// back to the corresponding byref arguments + /// + internal static void GenerateMarshalByRefsBack(ILGenerator il, IReadOnlyList argTypes) + { + // assumes argument array is in loc_0 + for (int i = 0; i < argTypes.Count; ++i) + { + var type = argTypes[i]; + if (type.IsByRef) + { + type = type.GetElementType(); + + il.Emit(OpCodes.Ldarg, i + 1); // for stobj/stind later at the end + + il.Emit(OpCodes.Ldloc_0); + il.Emit(OpCodes.Ldc_I4, i); + il.Emit(OpCodes.Ldelem_Ref); + + if (type.IsValueType) + { + il.Emit(OpCodes.Unbox_Any, type); + il.Emit(OpCodes.Stobj, type); + } + else + { + il.Emit(OpCodes.Castclass, type); + il.Emit(OpCodes.Stind_Ref); + } + } + } + } } } diff --git a/src/runtime/delegatemanager.cs b/src/runtime/delegatemanager.cs index 24e9d5f0d..092c9be1d 100644 --- a/src/runtime/delegatemanager.cs +++ b/src/runtime/delegatemanager.cs @@ -135,28 +135,7 @@ private Type GetDispatcher(Type dtype) if (anyByRef) { // Dispatch() will have modified elements of the args list that correspond to out parameters. - for (var c = 0; c < signature.Length; c++) - { - Type t = signature[c]; - if (t.IsByRef) - { - t = t.GetElementType(); - // *arg = args[c] - il.Emit(OpCodes.Ldarg_S, (byte)(c + 1)); - il.Emit(OpCodes.Ldloc_0); - il.Emit(OpCodes.Ldc_I4, c); - il.Emit(OpCodes.Ldelem_Ref); - if (t.IsValueType) - { - il.Emit(OpCodes.Unbox_Any, t); - il.Emit(OpCodes.Stobj, t); - } - else - { - il.Emit(OpCodes.Stind_Ref); - } - } - } + CodeGenerator.GenerateMarshalByRefsBack(il, signature); } if (method.ReturnType == voidtype) diff --git a/src/testing/interfacetest.cs b/src/testing/interfacetest.cs index 0158d64da..7c5d937b9 100644 --- a/src/testing/interfacetest.cs +++ b/src/testing/interfacetest.cs @@ -79,4 +79,18 @@ private interface IPrivate { } } + + public interface IOutArg + { + string MyMethod_Out(string name, out int index); + } + + public class OutArgCaller + { + public static int CallMyMethod_Out(IOutArg myInterface) + { + myInterface.MyMethod_Out("myclient", out int index); + return index; + } + } } diff --git a/tests/test_interface.py b/tests/test_interface.py index 130bd71c1..ac620684d 100644 --- a/tests/test_interface.py +++ b/tests/test_interface.py @@ -93,6 +93,20 @@ def test_interface_object_returned_through_out_param(): assert hello2.SayHello() == 'hello 2' +def test_interface_out_param_python_impl(): + from Python.Test import IOutArg, OutArgCaller + + class MyOutImpl(IOutArg): + __namespace__ = "Python.Test" + + def MyMethod_Out(self, name, index): + other_index = 101 + return ('MyName', other_index) + + py_impl = MyOutImpl() + + assert 101 == OutArgCaller.CallMyMethod_Out(py_impl) + def test_null_interface_object_returned(): """Test None is used also for methods with interface return types"""