Skip to content
Draft
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
3 changes: 3 additions & 0 deletions Include/internal/pycore_tuple.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ extern PyStatus _PyTuple_InitGlobalObjects(PyInterpreterState *);
PyAPI_FUNC(PyObject *)_PyTuple_FromStackRefStealOnSuccess(const union _PyStackRef *, Py_ssize_t);
PyAPI_FUNC(PyObject *)_PyTuple_FromArraySteal(PyObject *const *, Py_ssize_t);

PyAPI_FUNC(PyObject *)_PyTuple_FromPair(PyObject *, PyObject *);
PyAPI_FUNC(PyObject *)_PyTuple_FromPairSteal(PyObject *, PyObject *);

typedef struct {
PyObject_HEAD
Py_ssize_t it_index;
Expand Down
33 changes: 33 additions & 0 deletions Lib/test/test_capi/test_tuple.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import unittest
import gc
from sys import getrefcount
from test.support import import_helper

_testcapi = import_helper.import_module('_testcapi')
_testlimitedcapi = import_helper.import_module('_testlimitedcapi')
_testinternalcapi = import_helper.import_module('_testinternalcapi')

NULL = None
PY_SSIZE_T_MIN = _testcapi.PY_SSIZE_T_MIN
Expand Down Expand Up @@ -118,6 +120,37 @@ def test_tuple_pack(self):
# CRASHES pack(1, NULL)
# CRASHES pack(2, [1])

def test_tuple_from_pair(self):
# Test _PyTuple_FromPair, _PyTuple_FromPairSteal
ctors = (("_PyTuple_FromPair", _testinternalcapi._tuple_from_pair),
("_PyTuple_FromPairSteal", _testinternalcapi._tuple_from_pair_steal))
Copy link
Contributor

Choose a reason for hiding this comment

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

The _testinternalcapi._tuple_from_pair_steal that wraps _PyTuple_FromPairSteal increments references to the arguments. For that reason there is no way to make a distinction between the _testinternalcapi._tuple_from_pair and _testinternalcapi._tuple_from_pair._tuple_from_pair_steal in the tests)

I would either change the name of _tuple_from_pair_steal or add a comment to make this clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure I understand what you mean. Could you elaborate please?
You are right that tests seem equals from point of view, but we need to test both functions to check a) their behavior and b) absence of the leak if we run it with -R option.

Copy link
Contributor

Choose a reason for hiding this comment

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

The test is good indeed.

What I meant is that _testinternalcapi._tuple_from_pair_steal is not stealing references (since the arguments are incremented before passing to _PyTuple_FromPairSteal. So a (way too long) descriptive name would be _testinternalcapi._wraps_tuple_from_pair_steal_but_does_not_actually_steal.


for name, ctor in ctors:
with self.subTest(name):
self.assertEqual(type(ctor(1, 2)), tuple)
self.assertEqual(ctor(1, 2), (1, 2))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.assertEqual(ctor(1, 2), (1, 2))
self.assertEqual(ctor(1, 145325), (1, 145325))

The series of asserts contained only immortal elements for the tuple. Changing 2 to a semi-random higher number that is not part of the cpython small ints.

self.assertEqual(ctor(None, None), (None, None))
self.assertEqual(ctor(True, False), (True, False))

# user class supports gc
class Temp:
pass
temp = Temp()
temp_rc = getrefcount(temp)
self.assertEqual(ctor(temp, temp), (temp, temp))
self.assertEqual(getrefcount(temp), temp_rc)

self.assertRaises(TypeError, ctor, 1, 2, 3)
self.assertRaises(TypeError, ctor, 1)
self.assertRaises(TypeError, ctor)

self.assertFalse(gc.is_tracked(ctor(1, 2)))
self.assertFalse(gc.is_tracked(ctor(None, None)))
self.assertFalse(gc.is_tracked(ctor(True, False)))
self.assertTrue(gc.is_tracked(ctor(temp, (1, 2))))
self.assertTrue(gc.is_tracked(ctor(temp, 1)))
self.assertTrue(gc.is_tracked(ctor([], {})))

def test_tuple_size(self):
# Test PyTuple_Size()
size = _testlimitedcapi.tuple_size
Expand Down
2 changes: 1 addition & 1 deletion Modules/Setup.stdlib.in
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@
@MODULE_XXSUBTYPE_TRUE@xxsubtype xxsubtype.c
@MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c
@MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c
@MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c _testinternalcapi/test_lock.c _testinternalcapi/pytime.c _testinternalcapi/set.c _testinternalcapi/test_critical_sections.c _testinternalcapi/complex.c _testinternalcapi/interpreter.c
@MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c _testinternalcapi/test_lock.c _testinternalcapi/pytime.c _testinternalcapi/set.c _testinternalcapi/test_critical_sections.c _testinternalcapi/complex.c _testinternalcapi/interpreter.c _testinternalcapi/tuple.c
@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/heaptype.c _testcapi/abstract.c _testcapi/unicode.c _testcapi/dict.c _testcapi/set.c _testcapi/list.c _testcapi/tuple.c _testcapi/getargs.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/complex.c _testcapi/numbers.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/buffer.c _testcapi/pyatomic.c _testcapi/run.c _testcapi/file.c _testcapi/codec.c _testcapi/immortal.c _testcapi/gc.c _testcapi/hash.c _testcapi/time.c _testcapi/bytes.c _testcapi/object.c _testcapi/modsupport.c _testcapi/monitoring.c _testcapi/config.c _testcapi/import.c _testcapi/frame.c _testcapi/type.c _testcapi/function.c _testcapi/module.c
@MODULE__TESTLIMITEDCAPI_TRUE@_testlimitedcapi _testlimitedcapi.c _testlimitedcapi/abstract.c _testlimitedcapi/bytearray.c _testlimitedcapi/bytes.c _testlimitedcapi/codec.c _testlimitedcapi/complex.c _testlimitedcapi/dict.c _testlimitedcapi/eval.c _testlimitedcapi/float.c _testlimitedcapi/heaptype_relative.c _testlimitedcapi/import.c _testlimitedcapi/list.c _testlimitedcapi/long.c _testlimitedcapi/object.c _testlimitedcapi/pyos.c _testlimitedcapi/set.c _testlimitedcapi/sys.c _testlimitedcapi/tuple.c _testlimitedcapi/unicode.c _testlimitedcapi/vectorcall_limited.c _testlimitedcapi/version.c _testlimitedcapi/file.c
@MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c
Expand Down
3 changes: 3 additions & 0 deletions Modules/_testinternalcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -2981,6 +2981,9 @@ module_exec(PyObject *module)
if (_PyTestInternalCapi_Init_CriticalSection(module) < 0) {
return 1;
}
if (_PyTestInternalCapi_Init_Tuple(module) < 0) {
return 1;
}

Py_ssize_t sizeof_gc_head = 0;
#ifndef Py_GIL_DISABLED
Expand Down
1 change: 1 addition & 0 deletions Modules/_testinternalcapi/parts.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ int _PyTestInternalCapi_Init_PyTime(PyObject *module);
int _PyTestInternalCapi_Init_Set(PyObject *module);
int _PyTestInternalCapi_Init_Complex(PyObject *module);
int _PyTestInternalCapi_Init_CriticalSection(PyObject *module);
int _PyTestInternalCapi_Init_Tuple(PyObject *module);

#endif // Py_TESTINTERNALCAPI_PARTS_H
43 changes: 43 additions & 0 deletions Modules/_testinternalcapi/tuple.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
#include "parts.h"

#include "pycore_tuple.h"


static PyObject *
_tuple_from_pair(PyObject *Py_UNUSED(module), PyObject *args)
{
PyObject *one, *two;
if (!PyArg_ParseTuple(args, "OO", &one, &two)) {
return NULL;
}

return _PyTuple_FromPair(one, two);
}

static PyObject *
_tuple_from_pair_steal(PyObject *Py_UNUSED(module), PyObject *args)
{
PyObject *one, *two;
if (!PyArg_ParseTuple(args, "OO", &one, &two)) {
return NULL;
}

return _PyTuple_FromPairSteal(Py_NewRef(one), Py_NewRef(two));
}


static PyMethodDef test_methods[] = {
{"_tuple_from_pair", _tuple_from_pair, METH_VARARGS},
{"_tuple_from_pair_steal", _tuple_from_pair_steal, METH_VARARGS},
{NULL},
};

int
_PyTestInternalCapi_Init_Tuple(PyObject *m)
{
if (PyModule_AddFunctions(m, test_methods) < 0) {
return -1;
}

return 0;
}
52 changes: 52 additions & 0 deletions Objects/tupleobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,58 @@ PyTuple_Pack(Py_ssize_t n, ...)
return (PyObject *)result;
}

static PyTupleObject *
tuple_alloc_2(void)
{
Py_ssize_t size = 2;
Py_ssize_t index = size - 1;
PyTupleObject *result = _Py_FREELIST_POP(PyTupleObject, tuples[index]);
if (result == NULL) {
result = PyObject_GC_NewVar(PyTupleObject, &PyTuple_Type, size);
}
if (result != NULL) {
_PyTuple_RESET_HASH_CACHE(result);
}
return result;
}

PyObject *
_PyTuple_FromPair(PyObject *one, PyObject *two)
{
assert (one != NULL);
assert (two != NULL);

PyTupleObject *op = tuple_alloc_2();
if (op == NULL) {
return NULL;
}
op->ob_item[0] = Py_NewRef(one);
op->ob_item[1] = Py_NewRef(two);
if (maybe_tracked(one) || maybe_tracked(two)) {
_PyObject_GC_TRACK(op);
}
return (PyObject *)op;
}

PyObject *
_PyTuple_FromPairSteal(PyObject *one, PyObject *two)
{
assert (one != NULL);
assert (two != NULL);

PyTupleObject *op = tuple_alloc_2();
if (op == NULL) {
Py_DECREF(one);
Py_DECREF(two);
return NULL;
}
op->ob_item[0] = one;
op->ob_item[1] = two;
if (maybe_tracked(one) || maybe_tracked(two)) {
_PyObject_GC_TRACK(op);
}
return (PyObject *)op;
}

/* Methods */

Expand Down
1 change: 1 addition & 0 deletions PCbuild/_testinternalcapi.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
<ClCompile Include="..\Modules\_testinternalcapi\set.c" />
<ClCompile Include="..\Modules\_testinternalcapi\complex.c" />
<ClCompile Include="..\Modules\_testinternalcapi\interpreter.c" />
<ClCompile Include="..\Modules\_testinternalcapi\tuple.c" />
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="..\PC\python_nt.rc" />
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/_testinternalcapi.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
<ClCompile Include="..\Modules\_testinternalcapi\complex.c">
<Filter>Source Files</Filter>
</ClCompile>
<ClCompile Include="..\Modules\_testinternalcapi\tuple.c">
<Filter>Source Files</Filter>
</ClCompile>
</ItemGroup>
<ItemGroup>
<ResourceCompile Include="..\PC\python_nt.rc">
Expand Down
Loading