From 9c870c120718ce8c7cb7729cf20189587a32c402 Mon Sep 17 00:00:00 2001 From: Daniel Abrahamsson Date: Mon, 5 Oct 2020 15:34:22 +0200 Subject: [PATCH 1/2] Make indexers work for interface objects Makes the following work instead of throwing an exception: ```python from System.Collections.Generic import Dictionary, IDictionary d = IDictionary[str, str](Dictionary[str, str]()) d["one"] = "1" assert d["one"] == "1" ``` --- CHANGELOG.md | 1 + src/runtime/arrayobject.cs | 4 +- src/runtime/classbase.cs | 124 ++++++++++++++++++++++++++++++++++++ src/runtime/classobject.cs | 126 ------------------------------------- src/tests/test_indexer.py | 8 +++ 5 files changed, 135 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51859c060..964fc77b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ details about the cause of the failure - Fixed a bug where all .NET class instances were considered Iterable - Fix incorrect choice of method to invoke when using keyword arguments. - Fix non-delegate types incorrectly appearing as callable. +- Indexers can now be used with interface objects ## [2.5.0][] - 2020-06-14 diff --git a/src/runtime/arrayobject.cs b/src/runtime/arrayobject.cs index 2d50b3a1d..364d91969 100644 --- a/src/runtime/arrayobject.cs +++ b/src/runtime/arrayobject.cs @@ -40,7 +40,7 @@ public static IntPtr tp_new(IntPtr tp, IntPtr args, IntPtr kw) /// /// Implements __getitem__ for array types. /// - public static IntPtr mp_subscript(IntPtr ob, IntPtr idx) + public new static IntPtr mp_subscript(IntPtr ob, IntPtr idx) { var obj = (CLRObject)GetManagedObject(ob); var arrObj = (ArrayObject)GetManagedObjectType(ob); @@ -133,7 +133,7 @@ public static IntPtr mp_subscript(IntPtr ob, IntPtr idx) /// /// Implements __setitem__ for array types. /// - public static int mp_ass_subscript(IntPtr ob, IntPtr idx, IntPtr v) + public static new int mp_ass_subscript(IntPtr ob, IntPtr idx, IntPtr v) { var obj = (CLRObject)GetManagedObject(ob); var items = obj.inst as Array; diff --git a/src/runtime/classbase.cs b/src/runtime/classbase.cs index 43ec6ea72..972380928 100644 --- a/src/runtime/classbase.cs +++ b/src/runtime/classbase.cs @@ -302,5 +302,129 @@ public static void tp_dealloc(IntPtr ob) Runtime.XDecref(self.tpHandle); self.gcHandle.Free(); } + + + /// + /// Implements __getitem__ for reflected classes and value types. + /// + public static IntPtr mp_subscript(IntPtr ob, IntPtr idx) + { + IntPtr tp = Runtime.PyObject_TYPE(ob); + var cls = (ClassBase)GetManagedObject(tp); + + if (cls.indexer == null || !cls.indexer.CanGet) + { + Exceptions.SetError(Exceptions.TypeError, "unindexable object"); + return IntPtr.Zero; + } + + // Arg may be a tuple in the case of an indexer with multiple + // parameters. If so, use it directly, else make a new tuple + // with the index arg (method binders expect arg tuples). + IntPtr args = idx; + var free = false; + + if (!Runtime.PyTuple_Check(idx)) + { + args = Runtime.PyTuple_New(1); + Runtime.XIncref(idx); + Runtime.PyTuple_SetItem(args, 0, idx); + free = true; + } + + IntPtr value; + + try + { + value = cls.indexer.GetItem(ob, args); + } + finally + { + if (free) + { + Runtime.XDecref(args); + } + } + return value; + } + + + /// + /// Implements __setitem__ for reflected classes and value types. + /// + public static int mp_ass_subscript(IntPtr ob, IntPtr idx, IntPtr v) + { + IntPtr tp = Runtime.PyObject_TYPE(ob); + var cls = (ClassBase)GetManagedObject(tp); + + if (cls.indexer == null || !cls.indexer.CanSet) + { + Exceptions.SetError(Exceptions.TypeError, "object doesn't support item assignment"); + return -1; + } + + // Arg may be a tuple in the case of an indexer with multiple + // parameters. If so, use it directly, else make a new tuple + // with the index arg (method binders expect arg tuples). + IntPtr args = idx; + var free = false; + + if (!Runtime.PyTuple_Check(idx)) + { + args = Runtime.PyTuple_New(1); + Runtime.XIncref(idx); + Runtime.PyTuple_SetItem(args, 0, idx); + free = true; + } + + // Get the args passed in. + var i = Runtime.PyTuple_Size(args); + IntPtr defaultArgs = cls.indexer.GetDefaultArgs(args); + var numOfDefaultArgs = Runtime.PyTuple_Size(defaultArgs); + var temp = i + numOfDefaultArgs; + IntPtr real = Runtime.PyTuple_New(temp + 1); + for (var n = 0; n < i; n++) + { + IntPtr item = Runtime.PyTuple_GetItem(args, n); + Runtime.XIncref(item); + Runtime.PyTuple_SetItem(real, n, item); + } + + // Add Default Args if needed + for (var n = 0; n < numOfDefaultArgs; n++) + { + IntPtr item = Runtime.PyTuple_GetItem(defaultArgs, n); + Runtime.XIncref(item); + Runtime.PyTuple_SetItem(real, n + i, item); + } + // no longer need defaultArgs + Runtime.XDecref(defaultArgs); + i = temp; + + // Add value to argument list + Runtime.XIncref(v); + Runtime.PyTuple_SetItem(real, i, v); + + try + { + cls.indexer.SetItem(ob, real); + } + finally + { + Runtime.XDecref(real); + + if (free) + { + Runtime.XDecref(args); + } + } + + if (Exceptions.ErrorOccurred()) + { + return -1; + } + + return 0; + } } } diff --git a/src/runtime/classobject.cs b/src/runtime/classobject.cs index ada24358a..d7624ed6e 100644 --- a/src/runtime/classobject.cs +++ b/src/runtime/classobject.cs @@ -152,131 +152,5 @@ public override IntPtr type_subscript(IntPtr idx) } return Exceptions.RaiseTypeError("unsubscriptable object"); } - - - /// - /// Implements __getitem__ for reflected classes and value types. - /// - public static IntPtr mp_subscript(IntPtr ob, IntPtr idx) - { - //ManagedType self = GetManagedObject(ob); - IntPtr tp = Runtime.PyObject_TYPE(ob); - var cls = (ClassBase)GetManagedObject(tp); - - if (cls.indexer == null || !cls.indexer.CanGet) - { - Exceptions.SetError(Exceptions.TypeError, "unindexable object"); - return IntPtr.Zero; - } - - // Arg may be a tuple in the case of an indexer with multiple - // parameters. If so, use it directly, else make a new tuple - // with the index arg (method binders expect arg tuples). - IntPtr args = idx; - var free = false; - - if (!Runtime.PyTuple_Check(idx)) - { - args = Runtime.PyTuple_New(1); - Runtime.XIncref(idx); - Runtime.PyTuple_SetItem(args, 0, idx); - free = true; - } - - IntPtr value; - - try - { - value = cls.indexer.GetItem(ob, args); - } - finally - { - if (free) - { - Runtime.XDecref(args); - } - } - return value; - } - - - /// - /// Implements __setitem__ for reflected classes and value types. - /// - public static int mp_ass_subscript(IntPtr ob, IntPtr idx, IntPtr v) - { - //ManagedType self = GetManagedObject(ob); - IntPtr tp = Runtime.PyObject_TYPE(ob); - var cls = (ClassBase)GetManagedObject(tp); - - if (cls.indexer == null || !cls.indexer.CanSet) - { - Exceptions.SetError(Exceptions.TypeError, "object doesn't support item assignment"); - return -1; - } - - // Arg may be a tuple in the case of an indexer with multiple - // parameters. If so, use it directly, else make a new tuple - // with the index arg (method binders expect arg tuples). - IntPtr args = idx; - var free = false; - - if (!Runtime.PyTuple_Check(idx)) - { - args = Runtime.PyTuple_New(1); - Runtime.XIncref(idx); - Runtime.PyTuple_SetItem(args, 0, idx); - free = true; - } - - // Get the args passed in. - var i = Runtime.PyTuple_Size(args); - IntPtr defaultArgs = cls.indexer.GetDefaultArgs(args); - var numOfDefaultArgs = Runtime.PyTuple_Size(defaultArgs); - var temp = i + numOfDefaultArgs; - IntPtr real = Runtime.PyTuple_New(temp + 1); - for (var n = 0; n < i; n++) - { - IntPtr item = Runtime.PyTuple_GetItem(args, n); - Runtime.XIncref(item); - Runtime.PyTuple_SetItem(real, n, item); - } - - // Add Default Args if needed - for (var n = 0; n < numOfDefaultArgs; n++) - { - IntPtr item = Runtime.PyTuple_GetItem(defaultArgs, n); - Runtime.XIncref(item); - Runtime.PyTuple_SetItem(real, n + i, item); - } - // no longer need defaultArgs - Runtime.XDecref(defaultArgs); - i = temp; - - // Add value to argument list - Runtime.XIncref(v); - Runtime.PyTuple_SetItem(real, i, v); - - try - { - cls.indexer.SetItem(ob, real); - } - finally - { - Runtime.XDecref(real); - - if (free) - { - Runtime.XDecref(args); - } - } - - if (Exceptions.ErrorOccurred()) - { - return -1; - } - - return 0; - } } } diff --git a/src/tests/test_indexer.py b/src/tests/test_indexer.py index 6a36a2519..fae7176fb 100644 --- a/src/tests/test_indexer.py +++ b/src/tests/test_indexer.py @@ -595,3 +595,11 @@ def test_indexer_abuse(): with pytest.raises(AttributeError): del ob.__setitem__ + + +def test_indexer_accessed_through_interface(): + """Test that indexers can be accessed through interfaces""" + from System.Collections.Generic import Dictionary, IDictionary + d = IDictionary[str, str](Dictionary[str, str]()) + d["one"] = "1" + assert d["one"] == "1" From 12fdaab6a8f62123a253acd4ca2e173a9b2e096e Mon Sep 17 00:00:00 2001 From: Daniel Abrahamsson Date: Tue, 6 Oct 2020 07:23:53 +0200 Subject: [PATCH 2/2] Only set mp_subscript and mp_ass_subscript for indexable types --- src/runtime/typemanager.cs | 22 ++++++++++++++++++++++ src/tests/test_array.py | 12 +++++------- src/tests/test_indexer.py | 15 +++++++++++++-- 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 985f7f19a..a64f91e75 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -171,6 +171,28 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) Marshal.WriteIntPtr(type, TypeOffset.tp_iter, IntPtr.Zero); } + + // Only set mp_subscript and mp_ass_subscript for types with indexers + if (impl is ClassBase cb) + { + if (!(impl is ArrayObject)) + { + if (cb.indexer == null || !cb.indexer.CanGet) + { + Marshal.WriteIntPtr(type, TypeOffset.mp_subscript, IntPtr.Zero); + } + if (cb.indexer == null || !cb.indexer.CanSet) + { + Marshal.WriteIntPtr(type, TypeOffset.mp_ass_subscript, IntPtr.Zero); + } + } + } + else + { + Marshal.WriteIntPtr(type, TypeOffset.mp_subscript, IntPtr.Zero); + Marshal.WriteIntPtr(type, TypeOffset.mp_ass_subscript, IntPtr.Zero); + } + if (base_ != IntPtr.Zero) { Marshal.WriteIntPtr(type, TypeOffset.tp_base, base_); diff --git a/src/tests/test_array.py b/src/tests/test_array.py index 990af550a..015b5bdde 100644 --- a/src/tests/test_array.py +++ b/src/tests/test_array.py @@ -1321,16 +1321,14 @@ def test_array_abuse(): with pytest.raises(TypeError): Test.PublicArrayTest.__getitem__(0, 0) - with pytest.raises(TypeError): + with pytest.raises(AttributeError): Test.PublicArrayTest.__setitem__(0, 0, 0) - with pytest.raises(TypeError): - desc = Test.PublicArrayTest.__dict__['__getitem__'] - desc(0, 0) + with pytest.raises(KeyError): + Test.PublicArrayTest.__dict__['__getitem__'] - with pytest.raises(TypeError): - desc = Test.PublicArrayTest.__dict__['__setitem__'] - desc(0, 0, 0) + with pytest.raises(KeyError): + Test.PublicArrayTest.__dict__['__setitem__'] def test_iterator_to_array(): diff --git a/src/tests/test_indexer.py b/src/tests/test_indexer.py index fae7176fb..6da8e1386 100644 --- a/src/tests/test_indexer.py +++ b/src/tests/test_indexer.py @@ -42,7 +42,7 @@ def test_internal_indexer(): with pytest.raises(TypeError): Test.InternalIndexerTest.__getitem__(ob, 0) - with pytest.raises(TypeError): + with pytest.raises(AttributeError): ob.__getitem__(0) @@ -56,7 +56,7 @@ def test_private_indexer(): with pytest.raises(TypeError): Test.PrivateIndexerTest.__getitem__(ob, 0) - with pytest.raises(TypeError): + with pytest.raises(AttributeError): ob.__getitem__(0) @@ -603,3 +603,14 @@ def test_indexer_accessed_through_interface(): d = IDictionary[str, str](Dictionary[str, str]()) d["one"] = "1" assert d["one"] == "1" + + +def test_using_indexer_on_object_without_indexer(): + """Test using subscript syntax on an object an without indexer raises""" + from System import Object + o = Object() + with pytest.raises(TypeError): + o[0] + + with pytest.raises(TypeError): + o[0] = 1