From 1dd36ae3c978c1105ceac892a142e6980b4a0640 Mon Sep 17 00:00:00 2001 From: Daniel Abrahamsson Date: Fri, 25 Sep 2020 23:21:26 +0200 Subject: [PATCH 1/4] Wrap returned objects in interface if method return type is interface This allows callers to call all methods of an interface, regardless of whether the method was implemented implicitly or explicitly. Before this change, you had to make an explicit cast to the interface to be able to call the explicitly implemented method. Consider the following code: ```C# namespace Python.Test { public interface ITestInterface { void Foo(); void Bar(); } public class TestImpl : ITestInterface { public void Foo() { }; public void ITestInterface.Bar() { }; public void Baz() { }; public static ITestInterface GetInterface() { return new TestImpl(); } } } ``` And the following Python code, demonstrating the behavior before this change: ```python from Python.Test import TestImpl, ITestInterface test = TestImpl.GetInterface() test.Foo() # works test.Bar() # AttributeError: 'TestImpl' object has no attribute 'Bar' test.Baz() # works! - baz ``` After this change, the behavior is as follows: ``` test = TestImpl.GetInterface() test.Foo() # works test.Bar() # works test.Baz() # AttributeError: 'ITestInterface' object has no attribute 'Baz' ``` This is a breaking change due to that `Baz` is no longer visible in Python. --- src/runtime/converter.cs | 7 +++++++ src/runtime/methodbinder.cs | 2 +- src/runtime/typemanager.cs | 4 ++-- src/testing/interfacetest.cs | 17 ++++++++++++++++- src/testing/subclasstest.cs | 17 ++++++++++++++--- src/tests/test_array.py | 5 +++-- src/tests/test_generic.py | 7 ++++--- src/tests/test_interface.py | 32 ++++++++++++++++++++++++++++++++ src/tests/test_method.py | 9 ++++++--- src/tests/test_subclass.py | 12 +++++++----- 10 files changed, 92 insertions(+), 20 deletions(-) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 58b372fa1..50edb1107 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -173,6 +173,12 @@ internal static IntPtr ToPython(object value, Type type) } } + if (type.IsInterface) + { + var ifaceObj = (InterfaceObject)ClassManager.GetClass(type); + return CLRObject.GetInstHandle(value, ifaceObj.pyHandle); + } + // it the type is a python subclass of a managed type then return the // underlying python object rather than construct a new wrapper object. var pyderived = value as IPythonDerivedType; @@ -182,6 +188,7 @@ internal static IntPtr ToPython(object value, Type type) return ClassDerivedObject.ToPython(pyderived); } + // hmm - from Python, we almost never care what the declared // type is. we'd rather have the object bound to the actual // implementing class. diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index df8579ca4..e2d8581b3 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -744,7 +744,7 @@ internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw, MethodBase i Type pt = pi[i].ParameterType; if (pi[i].IsOut || pt.IsByRef) { - v = Converter.ToPython(binding.args[i], pt); + v = Converter.ToPython(binding.args[i], pt.GetElementType()); Runtime.PyTuple_SetItem(t, n, v); n++; } diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index be3ad1b88..985f7f19a 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -164,13 +164,13 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) // we want to do this after the slot stuff above in case the class itself implements a slot method InitializeSlots(type, impl.GetType()); - if (!clrType.GetInterfaces().Any(ifc => ifc == typeof(IEnumerable) || ifc == typeof(IEnumerator))) + if (!typeof(IEnumerable).IsAssignableFrom(clrType) && + !typeof(IEnumerator).IsAssignableFrom(clrType)) { // The tp_iter slot should only be set for enumerable types. Marshal.WriteIntPtr(type, TypeOffset.tp_iter, IntPtr.Zero); } - if (base_ != IntPtr.Zero) { Marshal.WriteIntPtr(type, TypeOffset.tp_base, base_); diff --git a/src/testing/interfacetest.cs b/src/testing/interfacetest.cs index 2c24596bc..418667ff0 100644 --- a/src/testing/interfacetest.cs +++ b/src/testing/interfacetest.cs @@ -11,7 +11,6 @@ internal interface IInternalInterface { } - public interface ISayHello1 { string SayHello(); @@ -43,6 +42,22 @@ string ISayHello2.SayHello() return "hello 2"; } + public ISayHello1 GetISayHello1() + { + return this; + } + + public void GetISayHello2(out ISayHello2 hello2) + { + hello2 = this; + } + + public ISayHello1 GetNoSayHello(out ISayHello2 hello2) + { + hello2 = null; + return null; + } + public interface IPublic { } diff --git a/src/testing/subclasstest.cs b/src/testing/subclasstest.cs index 9817d865e..ab0b73368 100644 --- a/src/testing/subclasstest.cs +++ b/src/testing/subclasstest.cs @@ -89,13 +89,24 @@ public static string test_bar(IInterfaceTest x, string s, int i) } // test instances can be constructed in managed code - public static IInterfaceTest create_instance(Type t) + public static SubClassTest create_instance(Type t) + { + return (SubClassTest)t.GetConstructor(new Type[] { }).Invoke(new object[] { }); + } + + public static IInterfaceTest create_instance_interface(Type t) { return (IInterfaceTest)t.GetConstructor(new Type[] { }).Invoke(new object[] { }); } - // test instances pass through managed code unchanged - public static IInterfaceTest pass_through(IInterfaceTest s) + // test instances pass through managed code unchanged ... + public static SubClassTest pass_through(SubClassTest s) + { + return s; + } + + // ... but the return type is an interface type, objects get wrapped + public static IInterfaceTest pass_through_interface(IInterfaceTest s) { return s; } diff --git a/src/tests/test_array.py b/src/tests/test_array.py index 427958ec7..990af550a 100644 --- a/src/tests/test_array.py +++ b/src/tests/test_array.py @@ -1288,9 +1288,10 @@ def test_special_array_creation(): assert value[1].__class__ == inst.__class__ assert value.Length == 2 + iface_class = ISayHello1(inst).__class__ value = Array[ISayHello1]([inst, inst]) - assert value[0].__class__ == inst.__class__ - assert value[1].__class__ == inst.__class__ + assert value[0].__class__ == iface_class + assert value[1].__class__ == iface_class assert value.Length == 2 inst = System.Exception("badness") diff --git a/src/tests/test_generic.py b/src/tests/test_generic.py index 9c410271d..c7e5a8d4d 100644 --- a/src/tests/test_generic.py +++ b/src/tests/test_generic.py @@ -319,7 +319,6 @@ def test_generic_method_type_handling(): assert_generic_method_by_type(ShortEnum, ShortEnum.Zero) assert_generic_method_by_type(System.Object, InterfaceTest()) assert_generic_method_by_type(InterfaceTest, InterfaceTest(), 1) - assert_generic_method_by_type(ISayHello1, InterfaceTest(), 1) def test_correct_overload_selection(): @@ -548,10 +547,11 @@ def test_method_overload_selection_with_generic_types(): value = MethodTest.Overloaded.__overloads__[vtype](input_) assert value.value.__class__ == inst.__class__ + iface_class = ISayHello1(inst).__class__ vtype = GenericWrapper[ISayHello1] input_ = vtype(inst) value = MethodTest.Overloaded.__overloads__[vtype](input_) - assert value.value.__class__ == inst.__class__ + assert value.value.__class__ == iface_class vtype = System.Array[GenericWrapper[int]] input_ = vtype([GenericWrapper[int](0), GenericWrapper[int](1)]) @@ -726,11 +726,12 @@ def test_overload_selection_with_arrays_of_generic_types(): assert value[0].value.__class__ == inst.__class__ assert value.Length == 2 + iface_class = ISayHello1(inst).__class__ gtype = GenericWrapper[ISayHello1] vtype = System.Array[gtype] input_ = vtype([gtype(inst), gtype(inst)]) value = MethodTest.Overloaded.__overloads__[vtype](input_) - assert value[0].value.__class__ == inst.__class__ + assert value[0].value.__class__ == iface_class assert value.Length == 2 diff --git a/src/tests/test_interface.py b/src/tests/test_interface.py index 6b72c1a12..0cf43268d 100644 --- a/src/tests/test_interface.py +++ b/src/tests/test_interface.py @@ -67,3 +67,35 @@ def test_explicit_cast_to_interface(): assert i2.SayHello() == 'hello 2' assert hasattr(i2, 'SayHello') assert not hasattr(i2, 'HelloProperty') + + +def test_interface_object_returned_through_method(): + """Test interface type is used if method return type is interface""" + from Python.Test import InterfaceTest + + ob = InterfaceTest() + hello1 = ob.GetISayHello1() + assert type(hello1).__name__ == 'ISayHello1' + + assert hello1.SayHello() == 'hello 1' + + +def test_interface_object_returned_through_out_param(): + """Test interface type is used for out parameters of interface types""" + from Python.Test import InterfaceTest + + ob = InterfaceTest() + hello2 = ob.GetISayHello2(None) + assert type(hello2).__name__ == 'ISayHello2' + + assert hello2.SayHello() == 'hello 2' + + +def test_null_interface_object_returned(): + """Test None is used also for methods with interface return types""" + from Python.Test import InterfaceTest + + ob = InterfaceTest() + hello1, hello2 = ob.GetNoSayHello(None) + assert hello1 is None + assert hello2 is None diff --git a/src/tests/test_method.py b/src/tests/test_method.py index a358025a5..15327cb3d 100644 --- a/src/tests/test_method.py +++ b/src/tests/test_method.py @@ -564,8 +564,10 @@ def test_explicit_overload_selection(): value = MethodTest.Overloaded.__overloads__[InterfaceTest](inst) assert value.__class__ == inst.__class__ + iface_class = ISayHello1(InterfaceTest()).__class__ value = MethodTest.Overloaded.__overloads__[ISayHello1](inst) - assert value.__class__ == inst.__class__ + assert value.__class__ != inst.__class__ + assert value.__class__ == iface_class atype = Array[System.Object] value = MethodTest.Overloaded.__overloads__[str, int, atype]( @@ -718,11 +720,12 @@ def test_overload_selection_with_array_types(): assert value[0].__class__ == inst.__class__ assert value[1].__class__ == inst.__class__ + iface_class = ISayHello1(inst).__class__ vtype = Array[ISayHello1] input_ = vtype([inst, inst]) value = MethodTest.Overloaded.__overloads__[vtype](input_) - assert value[0].__class__ == inst.__class__ - assert value[1].__class__ == inst.__class__ + assert value[0].__class__ == iface_class + assert value[1].__class__ == iface_class def test_explicit_overload_selection_failure(): diff --git a/src/tests/test_subclass.py b/src/tests/test_subclass.py index 07eaf7f82..968f6a788 100644 --- a/src/tests/test_subclass.py +++ b/src/tests/test_subclass.py @@ -104,8 +104,10 @@ def test_interface(): assert ob.bar("bar", 2) == "bar/bar" assert FunctionsTest.test_bar(ob, "bar", 2) == "bar/bar" - x = FunctionsTest.pass_through(ob) - assert id(x) == id(ob) + # pass_through will convert from InterfaceTestClass -> IInterfaceTest, + # causing a new wrapper object to be created. Hence id will differ. + x = FunctionsTest.pass_through_interface(ob) + assert id(x) != id(ob) def test_derived_class(): @@ -173,14 +175,14 @@ def test_create_instance(): assert id(x) == id(ob) InterfaceTestClass = interface_test_class_fixture(test_create_instance.__name__) - ob2 = FunctionsTest.create_instance(InterfaceTestClass) + ob2 = FunctionsTest.create_instance_interface(InterfaceTestClass) assert ob2.foo() == "InterfaceTestClass" assert FunctionsTest.test_foo(ob2) == "InterfaceTestClass" assert ob2.bar("bar", 2) == "bar/bar" assert FunctionsTest.test_bar(ob2, "bar", 2) == "bar/bar" - y = FunctionsTest.pass_through(ob2) - assert id(y) == id(ob2) + y = FunctionsTest.pass_through_interface(ob2) + assert id(y) != id(ob2) def test_events(): From a74664cc70adf0b2d47afe2811b4afb1b92f052e Mon Sep 17 00:00:00 2001 From: Daniel Abrahamsson Date: Tue, 29 Sep 2020 06:37:55 +0200 Subject: [PATCH 2/4] Return eleements of interface arrays as interface objects Even when a method is declared to return an array of interfaces, the CLR may use an array of the concrete type. Keep track of the intended type in `ArrayObject` so that elements of the array can be properly wrapped in `InterfaceObject` when accessed. --- src/runtime/arrayobject.cs | 3 ++- src/runtime/converter.cs | 10 +++++++++- src/runtime/managedtype.cs | 19 +++++++++++++++++++ src/testing/interfacetest.cs | 5 +++++ src/tests/test_interface.py | 8 ++++++++ 5 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/runtime/arrayobject.cs b/src/runtime/arrayobject.cs index 1ef318473..2d50b3a1d 100644 --- a/src/runtime/arrayobject.cs +++ b/src/runtime/arrayobject.cs @@ -43,8 +43,9 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw) public static IntPtr mp_subscript(IntPtr ob, IntPtr idx) { var obj = (CLRObject)GetManagedObject(ob); + var arrObj = (ArrayObject)GetManagedObjectType(ob); var items = obj.inst as Array; - Type itemType = obj.inst.GetType().GetElementType(); + Type itemType = arrObj.type.GetElementType(); int rank = items.Rank; int index; object value; diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 50edb1107..f7cf03996 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -179,6 +179,15 @@ internal static IntPtr ToPython(object value, Type type) return CLRObject.GetInstHandle(value, ifaceObj.pyHandle); } + // We need to special case interface array handling to ensure we + // produce the correct type. Value may be an array of some concrete + // type (FooImpl[]), but we want access to go via the interface type + // (IFoo[]). + if (type.IsArray && type.GetElementType().IsInterface) + { + return CLRObject.GetInstHandle(value, type); + } + // it the type is a python subclass of a managed type then return the // underlying python object rather than construct a new wrapper object. var pyderived = value as IPythonDerivedType; @@ -188,7 +197,6 @@ internal static IntPtr ToPython(object value, Type type) return ClassDerivedObject.ToPython(pyderived); } - // hmm - from Python, we almost never care what the declared // type is. we'd rather have the object bound to the actual // implementing class. diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index 23f5898d1..afa3bf2d7 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -45,6 +45,25 @@ internal static ManagedType GetManagedObject(IntPtr ob) return null; } + /// + /// Given a Python object, return the associated managed object type or null. + /// + internal static ManagedType GetManagedObjectType(IntPtr ob) + { + if (ob != IntPtr.Zero) + { + IntPtr tp = Runtime.PyObject_TYPE(ob); + var flags = Util.ReadCLong(tp, TypeOffset.tp_flags); + if ((flags & TypeFlags.Managed) != 0) + { + tp = Marshal.ReadIntPtr(tp, TypeOffset.magic()); + var gc = (GCHandle)tp; + return (ManagedType)gc.Target; + } + } + return null; + } + internal static ManagedType GetManagedObjectErr(IntPtr ob) { diff --git a/src/testing/interfacetest.cs b/src/testing/interfacetest.cs index 418667ff0..0158d64da 100644 --- a/src/testing/interfacetest.cs +++ b/src/testing/interfacetest.cs @@ -58,6 +58,11 @@ public ISayHello1 GetNoSayHello(out ISayHello2 hello2) return null; } + public ISayHello1 [] GetISayHello1Array() + { + return new[] { this }; + } + public interface IPublic { } diff --git a/src/tests/test_interface.py b/src/tests/test_interface.py index 0cf43268d..14f0fc75a 100644 --- a/src/tests/test_interface.py +++ b/src/tests/test_interface.py @@ -99,3 +99,11 @@ def test_null_interface_object_returned(): hello1, hello2 = ob.GetNoSayHello(None) assert hello1 is None assert hello2 is None + +def test_interface_array_returned(): + """Test interface type used for methods returning interface arrays""" + from Python.Test import InterfaceTest + + ob = InterfaceTest() + hellos = ob.GetISayHello1Array() + assert type(hellos[0]).__name__ == 'ISayHello1' From 44c4e18282fd1db63463e6b47c0b87c8567fb699 Mon Sep 17 00:00:00 2001 From: Daniel Abrahamsson Date: Wed, 30 Sep 2020 17:37:49 +0200 Subject: [PATCH 3/4] Add properties for accessing the object implementing an interface --- src/runtime/converter.cs | 2 +- src/runtime/interfaceobject.cs | 38 +++++++++++++++++++++++++++++++++- src/tests/test_interface.py | 13 ++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index f7cf03996..98fe99141 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -176,7 +176,7 @@ internal static IntPtr ToPython(object value, Type type) if (type.IsInterface) { var ifaceObj = (InterfaceObject)ClassManager.GetClass(type); - return CLRObject.GetInstHandle(value, ifaceObj.pyHandle); + return ifaceObj.WrapObject(value); } // We need to special case interface array handling to ensure we diff --git a/src/runtime/interfaceobject.cs b/src/runtime/interfaceobject.cs index 616ced6bd..74396f50c 100644 --- a/src/runtime/interfaceobject.cs +++ b/src/runtime/interfaceobject.cs @@ -71,7 +71,43 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw) return IntPtr.Zero; } - return CLRObject.GetInstHandle(obj, self.pyHandle); + return self.WrapObject(obj); + } + + /// + /// Wrap the given object in an interface object, so that only methods + /// of the interface are available. + /// + public IntPtr WrapObject(object impl) + { + var objPtr = CLRObject.GetInstHandle(impl, pyHandle); + return objPtr; + } + + /// + /// Expose the wrapped implementation through attributes in both + /// converted/encoded (__implementation__) and raw (__raw_implementation__) form. + /// + public static IntPtr tp_getattro(IntPtr ob, IntPtr key) + { + var clrObj = (CLRObject)GetManagedObject(ob); + + if (!Runtime.PyString_Check(key)) + { + return Exceptions.RaiseTypeError("string expected"); + } + + string name = Runtime.GetManagedString(key); + if (name == "__implementation__") + { + return Converter.ToPython(clrObj.inst); + } + else if (name == "__raw_implementation__") + { + return CLRObject.GetInstHandle(clrObj.inst); + } + + return Runtime.PyObject_GenericGetAttr(ob, key); } } } diff --git a/src/tests/test_interface.py b/src/tests/test_interface.py index 14f0fc75a..e6c6ba64b 100644 --- a/src/tests/test_interface.py +++ b/src/tests/test_interface.py @@ -61,6 +61,8 @@ def test_explicit_cast_to_interface(): assert hasattr(i1, 'SayHello') assert i1.SayHello() == 'hello 1' assert not hasattr(i1, 'HelloProperty') + assert i1.__implementation__ == ob + assert i1.__raw_implementation__ == ob i2 = Test.ISayHello2(ob) assert type(i2).__name__ == 'ISayHello2' @@ -76,6 +78,7 @@ def test_interface_object_returned_through_method(): ob = InterfaceTest() hello1 = ob.GetISayHello1() assert type(hello1).__name__ == 'ISayHello1' + assert hello1.__implementation__.__class__.__name__ == "InterfaceTest" assert hello1.SayHello() == 'hello 1' @@ -107,3 +110,13 @@ def test_interface_array_returned(): ob = InterfaceTest() hellos = ob.GetISayHello1Array() assert type(hellos[0]).__name__ == 'ISayHello1' + assert hellos[0].__implementation__.__class__.__name__ == "InterfaceTest" + +def test_implementation_access(): + """Test the __implementation__ and __raw_implementation__ properties""" + import System + clrVal = System.Int32(100) + i = System.IComparable(clrVal) + assert 100 == i.__implementation__ + assert clrVal == i.__raw_implementation__ + assert i.__implementation__ != i.__raw_implementation__ From c46ab75286451fd0b4541e6bb664bc0056b4c676 Mon Sep 17 00:00:00 2001 From: Daniel Abrahamsson Date: Thu, 1 Oct 2020 06:40:12 +0200 Subject: [PATCH 4/4] Update CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0c48ecb90..60494e67a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. details about the cause of the failure - `clr.AddReference` no longer adds ".dll" implicitly - `PyIter(PyObject)` constructor replaced with static `PyIter.GetIter(PyObject)` method +- Return values from .NET methods that return an interface are now automatically + wrapped in that interface. This is a breaking change for users that rely on being + able to access members that are part of the implementation class, but not the + interface. Use the new __implementation__ or __raw_implementation__ properties to + if you need to "downcast" to the implementation class. ### Fixed