From fec4e30c160b9e32494f8130766b78ce2a7bfaac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 15 Oct 2024 09:57:04 +0200 Subject: [PATCH 01/11] add `object.__reversed__` to `memoryview` objects --- Lib/test/test_memoryview.py | 6 +- Objects/clinic/memoryobject.c.h | 20 +++- Objects/memoryobject.c | 159 ++++++++++++++++++++++---------- Objects/object.c | 2 + 4 files changed, 135 insertions(+), 52 deletions(-) diff --git a/Lib/test/test_memoryview.py b/Lib/test/test_memoryview.py index 2d4bf5f1408df8..0dea813926f810 100644 --- a/Lib/test/test_memoryview.py +++ b/Lib/test/test_memoryview.py @@ -364,8 +364,10 @@ def test_reversed(self): b = tp(self._source) m = self._view(b) aslist = list(reversed(m.tolist())) - self.assertEqual(list(reversed(m)), aslist) - self.assertEqual(list(reversed(m)), list(m[::-1])) + with self.subTest(view_type=type(m)): + self.assertTrue(hasattr(m, '__reversed__')) + self.assertEqual(list(reversed(m)), aslist) + self.assertEqual(list(reversed(m)), list(m[::-1])) def test_toreadonly(self): for tp in self._types: diff --git a/Objects/clinic/memoryobject.c.h b/Objects/clinic/memoryobject.c.h index f199434dacb9e8..16fe1e8a2c1068 100644 --- a/Objects/clinic/memoryobject.c.h +++ b/Objects/clinic/memoryobject.c.h @@ -413,4 +413,22 @@ memoryview_hex(PyMemoryViewObject *self, PyObject *const *args, Py_ssize_t nargs exit: return return_value; } -/*[clinic end generated code: output=7e76a09106921ba2 input=a9049054013a1b77]*/ + +PyDoc_STRVAR(memoryview___reversed____doc__, +"__reversed__($self, /)\n" +"--\n" +"\n" +"Return a reverse iterator over this memory view."); + +#define MEMORYVIEW___REVERSED___METHODDEF \ + {"__reversed__", (PyCFunction)memoryview___reversed__, METH_NOARGS, memoryview___reversed____doc__}, + +static PyObject * +memoryview___reversed___impl(PyMemoryViewObject *self); + +static PyObject * +memoryview___reversed__(PyMemoryViewObject *self, PyObject *Py_UNUSED(ignored)) +{ + return memoryview___reversed___impl(self); +} +/*[clinic end generated code: output=1986ba41a7a4251b input=a9049054013a1b77]*/ diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index a2472d4873641d..2c6c78c7c71107 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3286,6 +3286,7 @@ static PyMethodDef memory_methods[] = { MEMORYVIEW__FROM_FLAGS_METHODDEF {"__enter__", memory_enter, METH_NOARGS, NULL}, {"__exit__", memory_exit, METH_VARARGS, memory_exit_doc}, + MEMORYVIEW___REVERSED___METHODDEF {NULL, NULL} }; @@ -3294,6 +3295,7 @@ static PyMethodDef memory_methods[] = { /**************************************************************************/ PyTypeObject _PyMemoryIter_Type; +PyTypeObject _PyMemoryRevIter_Type; typedef struct { PyObject_HEAD @@ -3303,10 +3305,51 @@ typedef struct { const char *it_fmt; } memoryiterobject; +#define _PyMemoryViewIter_CAST(PTR) ((memoryiterobject *)(PTR)) + +static PyObject * +memoryiter_new(PyObject *self, int reversed) +{ + if (!PyMemoryView_Check(self)) { + PyErr_BadInternalCall(); + return NULL; + } + + PyMemoryViewObject *sequence = _PyMemoryView_CAST(self); + const Py_buffer *view = &sequence->view; + if (view->ndim == 0) { + PyErr_SetString(PyExc_TypeError, "invalid indexing of 0-dim memory"); + return NULL; + } + else if (view->ndim != 1) { + PyErr_SetString(PyExc_NotImplementedError, + "multi-dimensional sub-views are not implemented"); + return NULL; + } + const char *fmt = adjust_fmt(view); + if (fmt == NULL) { + return NULL; + } + + PyTypeObject *itertype = reversed + ? &_PyMemoryRevIter_Type + : &_PyMemoryIter_Type; + memoryiterobject *it = PyObject_GC_New(memoryiterobject, itertype); + if (it == NULL) { + return NULL; + } + it->it_fmt = fmt; + it->it_length = memory_length(self); + it->it_index = reversed ? (it->it_length - 1) : 0; + it->it_seq = _PyMemoryView_CAST(Py_NewRef(self)); + _PyObject_GC_TRACK(it); + return (PyObject *)it; +} + static void memoryiter_dealloc(PyObject *self) { - memoryiterobject *it = (memoryiterobject *)self; + memoryiterobject *it = _PyMemoryViewIter_CAST(self); _PyObject_GC_UNTRACK(it); Py_XDECREF(it->it_seq); PyObject_GC_Del(it); @@ -3315,34 +3358,36 @@ memoryiter_dealloc(PyObject *self) static int memoryiter_traverse(PyObject *self, visitproc visit, void *arg) { - memoryiterobject *it = (memoryiterobject *)self; + memoryiterobject *it = _PyMemoryViewIter_CAST(self); Py_VISIT(it->it_seq); return 0; } +static inline PyObject * +memoryiter_iter_nth(PyMemoryViewObject *seq, Py_ssize_t nth, const char *fmt) +{ + CHECK_RELEASED(seq); + const Py_buffer *view = &seq->view; + const char *head = (const char *)seq->view.buf; + const char *ptr = head + view->strides[0] * nth; + ptr = ADJUST_PTR(ptr, view->suboffsets, 0); + if (ptr == NULL) { + return NULL; + } + return unpack_single(seq, ptr, fmt); +} + static PyObject * memoryiter_next(PyObject *self) { - memoryiterobject *it = (memoryiterobject *)self; - PyMemoryViewObject *seq; - seq = it->it_seq; + memoryiterobject *it = _PyMemoryViewIter_CAST(self); + PyMemoryViewObject *seq = it->it_seq; if (seq == NULL) { return NULL; } - if (it->it_index < it->it_length) { - CHECK_RELEASED(seq); - Py_buffer *view = &(seq->view); - char *ptr = (char *)seq->view.buf; - - ptr += view->strides[0] * it->it_index++; - ptr = ADJUST_PTR(ptr, view->suboffsets, 0); - if (ptr == NULL) { - return NULL; - } - return unpack_single(seq, ptr, it->it_fmt); + return memoryiter_iter_nth(seq, it->it_index++, it->it_fmt); } - it->it_seq = NULL; Py_DECREF(seq); return NULL; @@ -3351,38 +3396,7 @@ memoryiter_next(PyObject *self) static PyObject * memory_iter(PyObject *seq) { - if (!PyMemoryView_Check(seq)) { - PyErr_BadInternalCall(); - return NULL; - } - PyMemoryViewObject *obj = (PyMemoryViewObject *)seq; - int ndims = obj->view.ndim; - if (ndims == 0) { - PyErr_SetString(PyExc_TypeError, "invalid indexing of 0-dim memory"); - return NULL; - } - if (ndims != 1) { - PyErr_SetString(PyExc_NotImplementedError, - "multi-dimensional sub-views are not implemented"); - return NULL; - } - - const char *fmt = adjust_fmt(&obj->view); - if (fmt == NULL) { - return NULL; - } - - memoryiterobject *it; - it = PyObject_GC_New(memoryiterobject, &_PyMemoryIter_Type); - if (it == NULL) { - return NULL; - } - it->it_fmt = fmt; - it->it_length = memory_length((PyObject *)obj); - it->it_index = 0; - it->it_seq = (PyMemoryViewObject*)Py_NewRef(obj); - _PyObject_GC_TRACK(it); - return (PyObject *)it; + return memoryiter_new(seq, 0); } PyTypeObject _PyMemoryIter_Type = { @@ -3398,6 +3412,53 @@ PyTypeObject _PyMemoryIter_Type = { .tp_iternext = memoryiter_next, }; +/**************************************************************************/ +/* Memoryview Reversed Iterator */ +/**************************************************************************/ + +static PyObject * +memorview_reverse_iternext(PyObject *self) +{ + memoryiterobject *it = _PyMemoryViewIter_CAST(self); + PyMemoryViewObject *seq = it->it_seq; + if (seq == NULL) { + return NULL; + } + if (it->it_index >= 0 && it->it_index < it->it_length) { + // FEAT(picnixz): can we omit "it->it_index < it->it_length"? + return memoryiter_iter_nth(seq, it->it_index--, it->it_fmt); + } + it->it_seq = NULL; + Py_DECREF(seq); + return NULL; +} + +/*[clinic input] +memoryview.__reversed__ + +Return a reverse iterator over this memory view. +[clinic start generated code]*/ + +static PyObject * +memoryview___reversed___impl(PyMemoryViewObject *self) +/*[clinic end generated code: output=25f9b4345f4720ad input=3c35e9267ad7fcc6]*/ +{ + return memoryiter_new((PyObject *)self, 1); +} + + +PyTypeObject _PyMemoryRevIter_Type = { + PyVarObject_HEAD_INIT(&PyType_Type, 0) + "memory_reverseiterator", + sizeof(memoryiterobject), + .tp_dealloc = memoryiter_dealloc, + .tp_getattro = PyObject_GenericGetAttr, + .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, + .tp_traverse = memoryiter_traverse, + .tp_iter = PyObject_SelfIter, + .tp_iternext = memorview_reverse_iternext, +}; + PyTypeObject PyMemoryView_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "memoryview", /* tp_name */ diff --git a/Objects/object.c b/Objects/object.c index 4a4c5bf7d7f08a..582940ec7e1474 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -2220,6 +2220,7 @@ extern PyTypeObject _PyAnextAwaitable_Type; extern PyTypeObject _PyLegacyEventHandler_Type; extern PyTypeObject _PyLineIterator; extern PyTypeObject _PyMemoryIter_Type; +extern PyTypeObject _PyMemoryRevIter_Type; extern PyTypeObject _PyPositionsIterator; extern PyTypeObject _Py_GenericAliasIterType; @@ -2328,6 +2329,7 @@ static PyTypeObject* static_types[] = { &_PyLineIterator, &_PyManagedBuffer_Type, &_PyMemoryIter_Type, + &_PyMemoryRevIter_Type, &_PyMethodWrapper_Type, &_PyNamespace_Type, &_PyNone_Type, From f891cc9eb4f61cd9dc44193029cd4e867c54cb86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 15 Oct 2024 10:10:05 +0200 Subject: [PATCH 02/11] update `collections.abc` with memoryview iterator types --- Lib/_collections_abc.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Lib/_collections_abc.py b/Lib/_collections_abc.py index 06667b7434ccef..2ec940a3187037 100644 --- a/Lib/_collections_abc.py +++ b/Lib/_collections_abc.py @@ -73,6 +73,8 @@ def _f(): pass dict_itemiterator = type(iter({}.items())) list_iterator = type(iter([])) list_reverseiterator = type(iter(reversed([]))) +memory_iterator = type(iter(memoryview(b''))) +memory_reverseiterator = type(reversed(memoryview(b''))) range_iterator = type(iter(range(0))) longrange_iterator = type(iter(range(1 << 1000))) set_iterator = type(iter(set())) @@ -325,6 +327,8 @@ def __subclasshook__(cls, C): Iterator.register(dict_itemiterator) Iterator.register(list_iterator) Iterator.register(list_reverseiterator) +Iterator.register(memory_iterator) +Iterator.register(memory_reverseiterator) Iterator.register(range_iterator) Iterator.register(longrange_iterator) Iterator.register(set_iterator) From b020271edaf2be1968770f746ac5e3f540a8e71a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 15 Oct 2024 09:56:41 +0200 Subject: [PATCH 03/11] blurb --- .../2024-10-15-09-56-38.gh-issue-125420.c_VnrS.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-15-09-56-38.gh-issue-125420.c_VnrS.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-15-09-56-38.gh-issue-125420.c_VnrS.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-15-09-56-38.gh-issue-125420.c_VnrS.rst new file mode 100644 index 00000000000000..453a7b87ac5054 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-15-09-56-38.gh-issue-125420.c_VnrS.rst @@ -0,0 +1,2 @@ +Add :meth:`~object.__reversed__` to :class:`memoryview` objects. Patch by +Bénédikt Tran. From 4485dea1a350bdbeaa51649d36c827083c4e6637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 15 Oct 2024 10:19:32 +0200 Subject: [PATCH 04/11] update global objects --- Tools/c-analyzer/cpython/globals-to-fix.tsv | 1 + Tools/c-analyzer/cpython/ignored.tsv | 1 + 2 files changed, 2 insertions(+) diff --git a/Tools/c-analyzer/cpython/globals-to-fix.tsv b/Tools/c-analyzer/cpython/globals-to-fix.tsv index badd7b79102310..93f6457fab031c 100644 --- a/Tools/c-analyzer/cpython/globals-to-fix.tsv +++ b/Tools/c-analyzer/cpython/globals-to-fix.tsv @@ -118,6 +118,7 @@ Objects/codeobject.c - _PyPositionsIterator - Objects/genericaliasobject.c - _Py_GenericAliasIterType - # Not in a .h file: Objects/memoryobject.c - _PyMemoryIter_Type - +Objects/memoryobject.c - _PyMemoryRevIter_Type - Objects/unicodeobject.c - _PyUnicodeASCIIIter_Type - Objects/unionobject.c - _PyUnion_Type - Python/context.c - _PyContextTokenMissing_Type - diff --git a/Tools/c-analyzer/cpython/ignored.tsv b/Tools/c-analyzer/cpython/ignored.tsv index 2605825d3d0078..e4cb01fbf750d1 100644 --- a/Tools/c-analyzer/cpython/ignored.tsv +++ b/Tools/c-analyzer/cpython/ignored.tsv @@ -684,6 +684,7 @@ Modules/posixmodule.c - _Py_open_cloexec_works - Modules/posixmodule.c - environ - Objects/object.c - _Py_GenericAliasIterType - Objects/object.c - _PyMemoryIter_Type - +Objects/object.c - _PyMemoryRevIter_Type - Objects/object.c - _PyLineIterator - Objects/object.c - _PyPositionsIterator - Python/perf_trampoline.c - _Py_trampoline_func_start - From 766ac8618cd1332b2df2618ea4121bbb865f5dee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 17 Oct 2024 13:44:47 +0200 Subject: [PATCH 05/11] Explain performances loss and gains --- Objects/memoryobject.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 2c6c78c7c71107..605868ec3e9e65 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3443,6 +3443,13 @@ static PyObject * memoryview___reversed___impl(PyMemoryViewObject *self) /*[clinic end generated code: output=25f9b4345f4720ad input=3c35e9267ad7fcc6]*/ { + // NOTE(picnixz): generic implementation via reversed(...) is faster + // if the iterator is not used or if zero or few items are iterated. + // + // When the number of items is sufficiently large (>= 2^5), benchmarks + // show that this implementation improves for-loop performances. Note + // that materializing the specialized reversed iterator is likely to + // be slower than materializing a generic reversed object instance. return memoryiter_new((PyObject *)self, 1); } @@ -3459,6 +3466,7 @@ PyTypeObject _PyMemoryRevIter_Type = { .tp_iternext = memorview_reverse_iternext, }; + PyTypeObject PyMemoryView_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "memoryview", /* tp_name */ From ae3ffc3a7a8608b07b8a7b0e1a98a936bae9c492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 17 Oct 2024 18:01:54 +0200 Subject: [PATCH 06/11] use `_PyObject_CAST` instead of `(PyObject *)`. --- Objects/memoryobject.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 605868ec3e9e65..e11918c6688442 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -11,11 +11,13 @@ */ #include "Python.h" -#include "pycore_abstract.h" // _PyIndex_Check() -#include "pycore_memoryobject.h" // _PyManagedBuffer_Type -#include "pycore_object.h" // _PyObject_GC_UNTRACK() -#include "pycore_strhex.h" // _Py_strhex_with_sep() -#include // offsetof() +#include "pycore_abstract.h" // _PyIndex_Check() +#include "pycore_memoryobject.h" // _PyManagedBuffer_Type +#include "pycore_object.h" // _PyObject_GC_UNTRACK() +#include "pycore_strhex.h" // _Py_strhex_with_sep() + +#include // bool +#include // offsetof() /*[clinic input] class memoryview "PyMemoryViewObject *" "&PyMemoryView_Type" @@ -3308,7 +3310,7 @@ typedef struct { #define _PyMemoryViewIter_CAST(PTR) ((memoryiterobject *)(PTR)) static PyObject * -memoryiter_new(PyObject *self, int reversed) +memoryiter_new(PyObject *self, bool reversed) { if (!PyMemoryView_Check(self)) { PyErr_BadInternalCall(); @@ -3343,7 +3345,7 @@ memoryiter_new(PyObject *self, int reversed) it->it_index = reversed ? (it->it_length - 1) : 0; it->it_seq = _PyMemoryView_CAST(Py_NewRef(self)); _PyObject_GC_TRACK(it); - return (PyObject *)it; + return _PyObject_CAST(it); } static void @@ -3450,7 +3452,7 @@ memoryview___reversed___impl(PyMemoryViewObject *self) // show that this implementation improves for-loop performances. Note // that materializing the specialized reversed iterator is likely to // be slower than materializing a generic reversed object instance. - return memoryiter_new((PyObject *)self, 1); + return memoryiter_new(_PyObject_CAST(self), 1); } From 558ce3cd7962874a8524cf63821f725e1156276c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Thu, 17 Oct 2024 18:02:11 +0200 Subject: [PATCH 07/11] small cosmetic changes on iterator types specs --- Objects/memoryobject.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index e11918c6688442..5d7fbba9c0471c 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3405,9 +3405,7 @@ PyTypeObject _PyMemoryIter_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) .tp_name = "memory_iterator", .tp_basicsize = sizeof(memoryiterobject), - // methods .tp_dealloc = memoryiter_dealloc, - .tp_getattro = PyObject_GenericGetAttr, .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, .tp_traverse = memoryiter_traverse, .tp_iter = PyObject_SelfIter, @@ -3458,10 +3456,9 @@ memoryview___reversed___impl(PyMemoryViewObject *self) PyTypeObject _PyMemoryRevIter_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) - "memory_reverseiterator", - sizeof(memoryiterobject), + .tp_name = "memory_reverseiterator", + .tp_basicsize = sizeof(memoryiterobject), .tp_dealloc = memoryiter_dealloc, - .tp_getattro = PyObject_GenericGetAttr, .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC, .tp_traverse = memoryiter_traverse, .tp_iter = PyObject_SelfIter, From fc6c4a2b210b2b1af8b1ad753d4dfdff33ff022c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 22 Oct 2024 17:21:48 +0200 Subject: [PATCH 08/11] add an assertion for flag-like parameter --- Objects/memoryobject.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 5d7fbba9c0471c..cfbfb2baa33f63 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -11,13 +11,11 @@ */ #include "Python.h" -#include "pycore_abstract.h" // _PyIndex_Check() -#include "pycore_memoryobject.h" // _PyManagedBuffer_Type -#include "pycore_object.h" // _PyObject_GC_UNTRACK() -#include "pycore_strhex.h" // _Py_strhex_with_sep() - -#include // bool -#include // offsetof() +#include "pycore_abstract.h" // _PyIndex_Check() +#include "pycore_memoryobject.h" // _PyManagedBuffer_Type +#include "pycore_object.h" // _PyObject_GC_UNTRACK() +#include "pycore_strhex.h" // _Py_strhex_with_sep() +#include // offsetof() /*[clinic input] class memoryview "PyMemoryViewObject *" "&PyMemoryView_Type" @@ -3310,8 +3308,10 @@ typedef struct { #define _PyMemoryViewIter_CAST(PTR) ((memoryiterobject *)(PTR)) static PyObject * -memoryiter_new(PyObject *self, bool reversed) +memoryiter_new(PyObject *self, int reversed) { + assert(reversed == 0 || reversed == 1); + if (!PyMemoryView_Check(self)) { PyErr_BadInternalCall(); return NULL; From 544140a1b9675b056bfb62c661a603fb87b8740c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 25 Oct 2024 11:27:38 +0200 Subject: [PATCH 09/11] Use explicit cast instead of `_PyObject_CAST` --- Objects/memoryobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index cfbfb2baa33f63..60c795fa160d41 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3345,7 +3345,7 @@ memoryiter_new(PyObject *self, int reversed) it->it_index = reversed ? (it->it_length - 1) : 0; it->it_seq = _PyMemoryView_CAST(Py_NewRef(self)); _PyObject_GC_TRACK(it); - return _PyObject_CAST(it); + return (PyObject *)(it); } static void From c3b9f758539d83541fca01be4683b00c07f596db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 25 Oct 2024 16:45:55 +0200 Subject: [PATCH 10/11] Address Peter's review. --- Objects/memoryobject.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index 60c795fa160d41..a29091dff12a13 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3311,11 +3311,7 @@ static PyObject * memoryiter_new(PyObject *self, int reversed) { assert(reversed == 0 || reversed == 1); - - if (!PyMemoryView_Check(self)) { - PyErr_BadInternalCall(); - return NULL; - } + assert(PyMemoryView_Check(self)); PyMemoryViewObject *sequence = _PyMemoryView_CAST(self); const Py_buffer *view = &sequence->view; @@ -3343,9 +3339,9 @@ memoryiter_new(PyObject *self, int reversed) it->it_fmt = fmt; it->it_length = memory_length(self); it->it_index = reversed ? (it->it_length - 1) : 0; - it->it_seq = _PyMemoryView_CAST(Py_NewRef(self)); + it->it_seq = (PyMemoryViewObject *)Py_NewRef(self); _PyObject_GC_TRACK(it); - return (PyObject *)(it); + return (PyObject *)it; } static void @@ -3390,8 +3386,7 @@ memoryiter_next(PyObject *self) if (it->it_index < it->it_length) { return memoryiter_iter_nth(seq, it->it_index++, it->it_fmt); } - it->it_seq = NULL; - Py_DECREF(seq); + Py_CLEAR(it->it_seq); return NULL; } @@ -3428,8 +3423,7 @@ memorview_reverse_iternext(PyObject *self) // FEAT(picnixz): can we omit "it->it_index < it->it_length"? return memoryiter_iter_nth(seq, it->it_index--, it->it_fmt); } - it->it_seq = NULL; - Py_DECREF(seq); + Py_CLEAR(it->it_seq); return NULL; } @@ -3450,7 +3444,7 @@ memoryview___reversed___impl(PyMemoryViewObject *self) // show that this implementation improves for-loop performances. Note // that materializing the specialized reversed iterator is likely to // be slower than materializing a generic reversed object instance. - return memoryiter_new(_PyObject_CAST(self), 1); + return memoryiter_new((PyObject *)self, 1); } From f8fa19bdba2d190732fd254267be6409206a88ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 3 Nov 2024 14:41:32 +0100 Subject: [PATCH 11/11] reject iterating over released objects --- Objects/memoryobject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Objects/memoryobject.c b/Objects/memoryobject.c index a29091dff12a13..7fd5ddfa628b74 100644 --- a/Objects/memoryobject.c +++ b/Objects/memoryobject.c @@ -3313,6 +3313,7 @@ memoryiter_new(PyObject *self, int reversed) assert(reversed == 0 || reversed == 1); assert(PyMemoryView_Check(self)); + CHECK_RELEASED(self); PyMemoryViewObject *sequence = _PyMemoryView_CAST(self); const Py_buffer *view = &sequence->view; if (view->ndim == 0) {