Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-111489: Add PyList_FromArrayMoveRef() function #112975

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Doc/c-api/list.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,19 @@ List Objects
setting all items to a real object with :c:func:`PyList_SetItem`.


.. c:function:: PyObject* PyList_FromArrayMoveRef(PyObject *const *array, Py_ssize_t size)

Create a list of *size* items and move *array* :term:`strong references
<strong reference>` to the new list.

* Return a new reference on success. *array* references become
:term:`borrowed references <borrowed reference>`.
* Set an exception and return ``NULL`` on error. *array* is left unchanged,
the caller should clear *array* strong references.

.. versionadded:: 3.13


.. c:function:: Py_ssize_t PyList_Size(PyObject *list)

.. index:: pair: built-in function; len
Expand Down
24 changes: 24 additions & 0 deletions Doc/c-api/tuple.rst
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ Tuple Objects
Return a new tuple object of size *len*, or ``NULL`` on failure.


.. c:function:: PyObject* PyTuple_FromArray(PyObject *const *array, Py_ssize_t size)

Create a tuple of *size* items and copy references from *array* to the new
tuple.

* Return a new reference on success.
* Set an exception and return NULL on error.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Set an exception and return NULL on error.
* Set an exception and return ``NULL`` on error.


.. versionadded:: 3.13


.. c:function:: PyObject* PyTuple_FromArrayMoveRef(PyObject *const *array, Py_ssize_t size)

Create a tuple of *size* items and move *array* :term:`strong references
<strong reference>` to the new tuple.

* Return a new reference on success. *array* references become :term:`borrowed
references <borrowed reference>`.
* Set an exception and return ``NULL`` on error. *array* is left unchanged,
the caller should clear *array* strong references.

.. versionadded:: 3.13


.. c:function:: PyObject* PyTuple_Pack(Py_ssize_t n, ...)

Return a new tuple object of size *n*, or ``NULL`` on failure. The tuple values
Expand Down
8 changes: 8 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1269,6 +1269,14 @@ New Features
* Add :c:func:`Py_HashPointer` function to hash a pointer.
(Contributed by Victor Stinner in :gh:`111545`.)

* Add new functions to create :class:`tuple` and :class:`list` from arrays:

* :c:func:`PyTuple_FromArray`.
* :c:func:`PyTuple_FromArrayMoveRef`.
* :c:func:`PyList_FromArrayMoveRef`.

(Contributed by Victor Stinner in :gh:`111489`.)


Porting to Python 3.13
----------------------
Expand Down
3 changes: 3 additions & 0 deletions Include/cpython/listobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,6 @@ PyList_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value) {

PyAPI_FUNC(int) PyList_Extend(PyObject *self, PyObject *iterable);
PyAPI_FUNC(int) PyList_Clear(PyObject *self);
PyAPI_FUNC(PyObject*) PyList_FromArrayMoveRef(
PyObject *const *array,
Py_ssize_t size);
7 changes: 7 additions & 0 deletions Include/cpython/tupleobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,10 @@ PyTuple_SET_ITEM(PyObject *op, Py_ssize_t index, PyObject *value) {
}
#define PyTuple_SET_ITEM(op, index, value) \
PyTuple_SET_ITEM(_PyObject_CAST(op), (index), _PyObject_CAST(value))

PyAPI_FUNC(PyObject*) PyTuple_FromArray(
PyObject *const *array,
Py_ssize_t size);
PyAPI_FUNC(PyObject*) PyTuple_FromArrayMoveRef(
PyObject *const *array,
Py_ssize_t size);
4 changes: 3 additions & 1 deletion Include/internal/pycore_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ typedef struct {
PyListObject *it_seq; /* Set to NULL when iterator is exhausted */
} _PyListIterObject;

extern PyObject *_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n);
extern PyObject *_PyList_FromArraySteal(
PyObject *const *array,
Py_ssize_t size);

#ifdef __cplusplus
}
Expand Down
7 changes: 5 additions & 2 deletions Include/internal/pycore_tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,11 @@ struct _Py_tuple_state {

#define _PyTuple_ITEMS(op) _Py_RVALUE(_PyTuple_CAST(op)->ob_item)

extern PyObject *_PyTuple_FromArray(PyObject *const *, Py_ssize_t);
extern PyObject *_PyTuple_FromArraySteal(PyObject *const *, Py_ssize_t);
// Alias for backward compatibility
#define _PyTuple_FromArray PyTuple_FromArray
extern PyObject *_PyTuple_FromArraySteal(
PyObject *const *array,
Py_ssize_t size);


typedef struct {
Expand Down
8 changes: 8 additions & 0 deletions Lib/test/test_capi/test_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -342,3 +342,11 @@ def test_list_extend(self):

# CRASHES list_extend(NULL, [])
# CRASHES list_extend([], NULL)

def test_list_fromarraymoveref(self):
# Test PyList_FromArrayMoveRef()
list_fromarraymoveref = _testcapi.list_fromarraymoveref

lst = [object() for _ in range(5)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use the following example:

Suggested change
lst = [object() for _ in range(5)]
lst = [[i] for _ in range(5)]

Better repr.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, I used list("abdef"), but all items are immortal and so tests didn't detect refcount bugs in my C code. I used object() to make sure that each object is unique.

I don't think that repr is important here, the test is not supposed to fail :-)

copy = list_fromarraymoveref(lst)
self.assertEqual(copy, lst)
Comment on lines +351 to +352
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test also with zero and negative size. with too large size (causing MemoryError or OverflowError), with array=NULL, and with NULLs in the array. We should know what cases raise exceptions, what cases crash and what cases are valid, and how errors are handled.

21 changes: 21 additions & 0 deletions Lib/test/test_capi/test_tuple.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import unittest
from test.support import import_helper
_testcapi = import_helper.import_module('_testcapi')


class CAPITest(unittest.TestCase):
def test_tuple_fromarray(self):
# Test PyTuple_FromArray()
tuple_fromarray = _testcapi.tuple_fromarraymoveref

tup = tuple(object() for _ in range(5))
copy = tuple_fromarray(tup)
self.assertEqual(copy, tup)

def test_tuple_fromarraymoveref(self):
# Test PyTuple_FromArrayMoveRef()
tuple_fromarraymoveref = _testcapi.tuple_fromarraymoveref

tup = tuple(object() for _ in range(5))
copy = tuple_fromarraymoveref(tup)
self.assertEqual(copy, tup)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Add new functions to create :class:`tuple` and :class:`list` from arrays:

* :c:func:`PyTuple_FromArray`.
* :c:func:`PyTuple_FromArrayMoveRef`.
* :c:func:`PyList_FromArrayMoveRef`.

Patch by Victor Stinner.
40 changes: 40 additions & 0 deletions Modules/_testcapi/list.c
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,45 @@ list_extend(PyObject* Py_UNUSED(module), PyObject *args)
}


static PyObject *
list_fromarraymoveref(PyObject* Py_UNUSED(module), PyObject *args)
{
PyObject *src;
if (!PyArg_ParseTuple(args, "O!", &PyList_Type, &src)) {
return NULL;
}

Py_ssize_t size = PyList_GET_SIZE(src);
PyObject **array = PyMem_Malloc(size * sizeof(PyObject**));
if (array == NULL) {
PyErr_NoMemory();
return NULL;
}
for (Py_ssize_t i = 0; i < size; i++) {
array[i] = Py_NewRef(PyList_GET_ITEM(src, i));
}

PyObject *list = PyList_FromArrayMoveRef(array, size);

for (Py_ssize_t i = 0; i < size; i++) {
// check that the array is not modified
assert(array[i] == PyList_GET_ITEM(src, i));

// check that the array was copied properly
assert(PyList_GET_ITEM(list, i) == array[i]);
}

if (list == NULL) {
for (Py_ssize_t i = 0; i < size; i++) {
Py_DECREF(array[i]);
}
}
PyMem_Free(array);

return list;
}


static PyMethodDef test_methods[] = {
{"list_check", list_check, METH_O},
{"list_check_exact", list_check_exact, METH_O},
Expand All @@ -202,6 +241,7 @@ static PyMethodDef test_methods[] = {
{"list_astuple", list_astuple, METH_O},
{"list_clear", list_clear, METH_O},
{"list_extend", list_extend, METH_VARARGS},
{"list_fromarraymoveref", list_fromarraymoveref, METH_VARARGS},
{NULL},
};

Expand Down
61 changes: 61 additions & 0 deletions Modules/_testcapi/tuple.c
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,68 @@
#include "util.h"


static PyObject *
tuple_fromarray_impl(PyObject *args, int move_ref)
{
PyObject *src;
if (!PyArg_ParseTuple(args, "O!", &PyTuple_Type, &src)) {
return NULL;
}

Py_ssize_t size = PyTuple_GET_SIZE(src);
PyObject **array = PyMem_Malloc(size * sizeof(PyObject**));
if (array == NULL) {
PyErr_NoMemory();
return NULL;
}
for (Py_ssize_t i = 0; i < size; i++) {
array[i] = Py_NewRef(PyTuple_GET_ITEM(src, i));
}

PyObject *tuple;
if (move_ref) {
tuple = PyTuple_FromArrayMoveRef(array, size);
}
else {
tuple = PyTuple_FromArray(array, size);
}

for (Py_ssize_t i = 0; i < size; i++) {
// check that the array is not modified
assert(array[i] == PyTuple_GET_ITEM(src, i));

// check that the array was copied properly
assert(PyTuple_GET_ITEM(tuple, i) == array[i]);
}

if (!move_ref || tuple == NULL) {
for (Py_ssize_t i = 0; i < size; i++) {
Py_DECREF(array[i]);
}
}
PyMem_Free(array);

return tuple;
}


static PyObject *
tuple_fromarray(PyObject* Py_UNUSED(module), PyObject *args)
{
return tuple_fromarray_impl(args, 0);
}


static PyObject *
tuple_fromarraymoveref(PyObject* Py_UNUSED(module), PyObject *args)
{
return tuple_fromarray_impl(args, 1);
}


static PyMethodDef test_methods[] = {
{"tuple_fromarray", tuple_fromarray, METH_VARARGS},
{"tuple_fromarraymoveref", tuple_fromarraymoveref, METH_VARARGS},
{NULL},
};

Expand Down
33 changes: 22 additions & 11 deletions Objects/listobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -2622,25 +2622,36 @@ PyList_AsTuple(PyObject *v)
return _PyTuple_FromArray(((PyListObject *)v)->ob_item, Py_SIZE(v));
}

PyObject *
_PyList_FromArraySteal(PyObject *const *src, Py_ssize_t n)
PyObject*
PyList_FromArrayMoveRef(PyObject *const *array, Py_ssize_t size)
{
if (n == 0) {
if (size == 0) {
return PyList_New(0);
}

PyListObject *list = (PyListObject *)PyList_New(n);
if (list == NULL) {
for (Py_ssize_t i = 0; i < n; i++) {
Py_DECREF(src[i]);
}
PyObject *op = PyList_New(size);
if (op == NULL) {
return NULL;
}
PyListObject *list = (PyListObject *)op;

PyObject **dst = list->ob_item;
memcpy(dst, src, n * sizeof(PyObject *));
PyObject **items = list->ob_item;
// PyList_New(size) returns NULL is size*sizeof(PyObject*) is greater than
// PY_SSIZE_T_MAX. No need to check for integer overflow again.
memcpy(items, array, size * sizeof(PyObject *));
return op;
}

return (PyObject *)list;
PyObject *
_PyList_FromArraySteal(PyObject *const *array, Py_ssize_t size)
{
PyObject *list = PyList_FromArrayMoveRef(array, size);
if (list == NULL) {
for (Py_ssize_t i = 0; i < size; i++) {
Py_DECREF(array[i]);
}
}
return list;
}

/*[clinic input]
Expand Down
Loading
Loading