Skip to content

Commit cc47b10

Browse files
committed
Address Serhiy's review
1 parent b0fbeae commit cc47b10

File tree

4 files changed

+43
-41
lines changed

4 files changed

+43
-41
lines changed

Doc/c-api/tuple.rst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ Tuple Objects
4242
Create a tuple of *size* items and copy references from *array* to the new
4343
tuple.
4444
45-
* Return a new reference on success.
46-
* Set an exception and return ``NULL`` on error.
45+
*array* can be NULL if *size* is ``0``.
46+
47+
On success, return a new reference.
48+
On error, set an exception and return ``NULL``.
4749
4850
.. versionadded:: next
4951

Lib/test/test_capi/test_tuple.py

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,28 @@ def test_tuple_new(self):
6262
self.assertRaises(SystemError, tuple_new, PY_SSIZE_T_MIN)
6363
self.assertRaises(MemoryError, tuple_new, PY_SSIZE_T_MAX)
6464

65+
def test_tuple_fromarray(self):
66+
# Test PyTuple_FromArray()
67+
tuple_fromarray = _testcapi.tuple_fromarray
68+
69+
tup = tuple([i] for i in range(5))
70+
copy = tuple_fromarray(tup)
71+
self.assertEqual(copy, tup)
72+
73+
tup = ()
74+
copy = tuple_fromarray(tup)
75+
self.assertIs(copy, tup)
76+
77+
copy = tuple_fromarray(NULL, 0)
78+
self.assertIs(copy, ())
79+
80+
with self.assertRaises(ValueError):
81+
tuple_fromarray(NULL, -1)
82+
with self.assertRaises(ValueError):
83+
tuple_fromarray(NULL, PY_SSIZE_T_MIN)
84+
with self.assertRaises(MemoryError):
85+
tuple_fromarray(NULL, PY_SSIZE_T_MAX)
86+
6587
def test_tuple_pack(self):
6688
# Test PyTuple_Pack()
6789
pack = _testlimitedcapi.tuple_pack
@@ -279,18 +301,6 @@ def my_iter():
279301
self.assertEqual(tuple(my_iter()), (TAG, *range(10)))
280302
self.assertEqual(tuples, [])
281303

282-
def test_tuple_fromarray(self):
283-
# Test PyTuple_FromArray()
284-
tuple_fromarray = _testcapi.tuple_fromarray
285-
286-
tup = tuple(object() for _ in range(5))
287-
copy = tuple_fromarray(tup)
288-
self.assertEqual(copy, tup)
289-
290-
tup = ()
291-
copy = tuple_fromarray(tup)
292-
self.assertIs(copy, tup)
293-
294304

295305
if __name__ == "__main__":
296306
unittest.main()

Modules/_testcapi/tuple.c

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -108,40 +108,26 @@ static PyObject *
108108
tuple_fromarray(PyObject* Py_UNUSED(module), PyObject *args)
109109
{
110110
PyObject *src;
111-
if (!PyArg_ParseTuple(args, "O!", &PyTuple_Type, &src)) {
111+
Py_ssize_t size = -1;
112+
if (!PyArg_ParseTuple(args, "O|n", &src, &size)) {
112113
return NULL;
113114
}
114-
115-
Py_ssize_t size = PyTuple_GET_SIZE(src);
116-
PyObject **array = PyMem_Malloc(size * sizeof(PyObject**));
117-
if (array == NULL) {
118-
PyErr_NoMemory();
115+
if (src != Py_None && !PyTuple_Check(src)) {
116+
PyErr_SetString(PyExc_TypeError, "expect a tuple");
119117
return NULL;
120118
}
121-
for (Py_ssize_t i = 0; i < size; i++) {
122-
array[i] = Py_NewRef(PyTuple_GET_ITEM(src, i));
123-
}
124-
125-
PyObject *tuple = PyTuple_FromArray(array, size);
126-
if (tuple == NULL) {
127-
goto done;
128-
}
129-
130-
for (Py_ssize_t i = 0; i < size; i++) {
131-
// check that the array is not modified
132-
assert(array[i] == PyTuple_GET_ITEM(src, i));
133119

134-
// check that the array was copied properly
135-
assert(PyTuple_GET_ITEM(tuple, i) == array[i]);
120+
PyObject **items;
121+
if (src != Py_None) {
122+
items = &PyTuple_GET_ITEM(src, 0);
123+
if (size < 0) {
124+
size = PyTuple_GET_SIZE(src);
125+
}
136126
}
137-
138-
done:
139-
for (Py_ssize_t i = 0; i < size; i++) {
140-
Py_DECREF(array[i]);
127+
else {
128+
items = NULL;
141129
}
142-
PyMem_Free(array);
143-
144-
return tuple;
130+
return PyTuple_FromArray(items, size);
145131
}
146132

147133

Objects/tupleobject.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,10 @@ PyTuple_FromArray(PyObject *const *src, Py_ssize_t n)
371371
if (n == 0) {
372372
return tuple_get_empty();
373373
}
374+
if (n < 0) {
375+
PyErr_SetString(PyExc_ValueError, "size must be nonnegative");
376+
return NULL;
377+
}
374378

375379
PyTupleObject *tuple = tuple_alloc(n);
376380
if (tuple == NULL) {

0 commit comments

Comments
 (0)