From a3292c221b806fb623c4dc8132939895cdc48eb2 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 16 Oct 2025 21:36:08 +0200 Subject: [PATCH 01/29] Do not track frozenset objects with immutables --- Objects/setobject.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 213bd821d8a1b9..a2d3985bc0bbbf 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1174,6 +1174,23 @@ make_new_set_basetype(PyTypeObject *type, PyObject *iterable) return make_new_set(type, iterable); } +void +_PyFrozenSet_MaybeUntrack(PyObject *op) +{ + if ((op ==NULL) || !(PyFrozenSet_CheckExact(op))) { + return; + } + // the frozenset is tracked by the GC. if all elements are immutable we can untrack + Py_ssize_t pos = 0; + setentry *entry; + while (set_next((PySetObject *)op, &pos, &entry)) { + if (_PyObject_GC_MAY_BE_TRACKED(entry->key)) { + return; + } + } + _PyObject_GC_UNTRACK(op); +} + static PyObject * make_new_frozenset(PyTypeObject *type, PyObject *iterable) { @@ -1185,7 +1202,9 @@ make_new_frozenset(PyTypeObject *type, PyObject *iterable) /* frozenset(f) is idempotent */ return Py_NewRef(iterable); } - return make_new_set(type, iterable); + PyObject *obj = make_new_set(type, iterable); + _PyFrozenSet_MaybeUntrack(obj); + return obj; } static PyObject * @@ -2710,7 +2729,11 @@ PySet_New(PyObject *iterable) PyObject * PyFrozenSet_New(PyObject *iterable) { - return make_new_set(&PyFrozenSet_Type, iterable); + PyObject *result = make_new_set(&PyFrozenSet_Type, iterable); + if (result != NULL) { + _PyFrozenSet_MaybeUntrack(result); + } + return result; } Py_ssize_t From cd294a67f9f55059e1942fd988b36aa8f256158d Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 16 Oct 2025 23:00:19 +0200 Subject: [PATCH 02/29] cleanup --- Objects/setobject.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index a2d3985bc0bbbf..29cce311d92689 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2730,9 +2730,7 @@ PyObject * PyFrozenSet_New(PyObject *iterable) { PyObject *result = make_new_set(&PyFrozenSet_Type, iterable); - if (result != NULL) { - _PyFrozenSet_MaybeUntrack(result); - } + _PyFrozenSet_MaybeUntrack(result); return result; } From 7e28cf2eb6a648a6db6eb774dbb1c7ef34bf70d8 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 16 Oct 2025 23:01:26 +0200 Subject: [PATCH 03/29] cleanup --- Objects/setobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 29cce311d92689..f1fba0196802ff 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1180,7 +1180,7 @@ _PyFrozenSet_MaybeUntrack(PyObject *op) if ((op ==NULL) || !(PyFrozenSet_CheckExact(op))) { return; } - // the frozenset is tracked by the GC. if all elements are immutable we can untrack + // if all elements of a frozenset are not tracked, we untrack the object Py_ssize_t pos = 0; setentry *entry; while (set_next((PySetObject *)op, &pos, &entry)) { From c4deb03b3af009b9feaffbd29304a4713dc7fb86 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Thu, 16 Oct 2025 23:20:11 +0200 Subject: [PATCH 04/29] fix test --- Lib/test/test_sys.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py index 1198c6d35113c8..779f48750326d5 100644 --- a/Lib/test/test_sys.py +++ b/Lib/test/test_sys.py @@ -1876,7 +1876,10 @@ class S(set): check(S(), set(), '3P') class FS(frozenset): __slots__ = 'a', 'b', 'c' - check(FS(), frozenset(), '3P') + + class mytuple(tuple): + pass + check(FS([mytuple()]), frozenset([mytuple()]), '3P') from collections import OrderedDict class OD(OrderedDict): __slots__ = 'a', 'b', 'c' From 607237adc21fc916e5e89384ab6ca9e90ffb45db Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Thu, 16 Oct 2025 22:36:10 +0000 Subject: [PATCH 05/29] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20b?= =?UTF-8?q?lurb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2025-10-16-22-36-05.gh-issue-140232.u3srgv.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-22-36-05.gh-issue-140232.u3srgv.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-22-36-05.gh-issue-140232.u3srgv.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-22-36-05.gh-issue-140232.u3srgv.rst new file mode 100644 index 00000000000000..e40daacbc45b7b --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-10-16-22-36-05.gh-issue-140232.u3srgv.rst @@ -0,0 +1 @@ +Frozenset objects with immutable elements are no longer tracked by the garbage collector. From 2735a71d05ff2c8b63631b972d1400eebf0c7f37 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 17 Oct 2025 08:00:06 +0200 Subject: [PATCH 06/29] Update Objects/setobject.c Co-authored-by: Mikhail Efimov --- Objects/setobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index f1fba0196802ff..56ca0d5de0026a 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1177,7 +1177,7 @@ make_new_set_basetype(PyTypeObject *type, PyObject *iterable) void _PyFrozenSet_MaybeUntrack(PyObject *op) { - if ((op ==NULL) || !(PyFrozenSet_CheckExact(op))) { + if ((op == NULL) || !(PyFrozenSet_CheckExact(op))) { return; } // if all elements of a frozenset are not tracked, we untrack the object From c05db549484c84cd10d2a60b3b0152634f8825c0 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 24 Oct 2025 12:41:05 +0200 Subject: [PATCH 07/29] make sure PySet_Add tracks frozensets if needed --- Modules/_testcapimodule.c | 63 ++++++++++++++++++++++++++++++++++++++- Objects/setobject.c | 3 ++ 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 4e73be20e1b709..8d52a4eb5bddfd 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2435,6 +2435,66 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) } + +static PyObject * +test_pyset_add(PyObject* self, PyObject *Py_UNUSED(args)) +{ + + PyObject *set = NULL, *empty_tuple=NULL, *tracked_object; + + + tracked_object = PyImport_ImportModule("sys"); + if (tracked_object == NULL) { + goto failed; + } + if (!PyObject_GC_IsTracked(tracked_object)) { + PyErr_SetString(PyExc_ValueError, "Test item is not tracked by GC"); + goto failed; + } + + + int return_value; + empty_tuple = PyTuple_New(0); + if (empty_tuple == NULL) { + goto failed; + } + set = PyFrozenSet_New(empty_tuple); + if (set == NULL) { + return NULL; + } + + if (PyObject_GC_IsTracked(set)) { + PyErr_SetString(PyExc_ValueError, "Empty frozenset object is tracked by GC"); + goto failed; + } + return_value = PySet_Add(set, empty_tuple); + if (return_value<0) { + goto failed; + } + if (PyObject_GC_IsTracked(set)) { + PyErr_SetString(PyExc_ValueError, "Frozenset object with immutable is tracked by GC"); + goto failed; + } + + PySet_Add(set, tracked_object); + if (return_value<0) { + goto failed; + } + if (!PyObject_GC_IsTracked(set)) { + PyErr_SetString(PyExc_ValueError, "Frozenset object with tracked objects is not tracked by GC"); + goto failed; + } + + Py_RETURN_NONE; + +failed: + Py_XDECREF(tracked_object); + Py_XDECREF(empty_tuple); + Py_XDECREF(set); + return NULL; + +} + // Used by `finalize_thread_hang`. #if defined(_POSIX_THREADS) && !defined(__wasi__) static void finalize_thread_hang_cleanup_callback(void *Py_UNUSED(arg)) { @@ -2625,7 +2685,7 @@ static PyMethodDef TestMethods[] = { {"return_null_without_error", return_null_without_error, METH_NOARGS}, {"return_result_with_error", return_result_with_error, METH_NOARGS}, {"getitem_with_error", getitem_with_error, METH_VARARGS}, - {"Py_CompileString", pycompilestring, METH_O}, + {"Py_CompileString", pycompilestring, METH_O}, {"raise_SIGINT_then_send_None", raise_SIGINT_then_send_None, METH_VARARGS}, {"stack_pointer", stack_pointer, METH_NOARGS}, #ifdef W_STOPCODE @@ -2646,6 +2706,7 @@ static PyMethodDef TestMethods[] = { {"gen_get_code", gen_get_code, METH_O, NULL}, {"get_feature_macros", get_feature_macros, METH_NOARGS, NULL}, {"test_code_api", test_code_api, METH_NOARGS, NULL}, + {"test_pyset_add", test_pyset_add, METH_NOARGS, NULL}, {"settrace_to_error", settrace_to_error, METH_O, NULL}, {"settrace_to_record", settrace_to_record, METH_O, NULL}, {"test_macros", test_macros, METH_NOARGS, NULL}, diff --git a/Objects/setobject.c b/Objects/setobject.c index a2d3985bc0bbbf..729a8f182908f6 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2802,6 +2802,9 @@ PySet_Add(PyObject *anyset, PyObject *key) return -1; } + if (PyFrozenSet_Check(anyset) && PyObject_GC_IsTracked(key) && !PyObject_GC_IsTracked(anyset) ) { + _PyObject_GC_TRACK(anyset); + } int rv; Py_BEGIN_CRITICAL_SECTION(anyset); rv = set_add_key((PySetObject *)anyset, key); From 0b97604bcd2e3c2e9595921115b5a95cda12a448 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 24 Oct 2025 14:30:30 +0200 Subject: [PATCH 08/29] review comment --- Modules/_testcapimodule.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 8d52a4eb5bddfd..739a4cf1c9f205 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2441,7 +2441,7 @@ test_pyset_add(PyObject* self, PyObject *Py_UNUSED(args)) { PyObject *set = NULL, *empty_tuple=NULL, *tracked_object; - + int return_value; tracked_object = PyImport_ImportModule("sys"); if (tracked_object == NULL) { @@ -2452,15 +2452,13 @@ test_pyset_add(PyObject* self, PyObject *Py_UNUSED(args)) goto failed; } - - int return_value; empty_tuple = PyTuple_New(0); if (empty_tuple == NULL) { goto failed; } set = PyFrozenSet_New(empty_tuple); if (set == NULL) { - return NULL; + goto failed; } if (PyObject_GC_IsTracked(set)) { From 08e22c30a6268d77cfc769ff2cd21daba62155a6 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Oct 2025 20:18:04 +0100 Subject: [PATCH 09/29] use _testcapi for testing --- Lib/test/test_set.py | 26 ++++++++++++++++ Modules/_testcapimodule.c | 63 ++++++++------------------------------- 2 files changed, 39 insertions(+), 50 deletions(-) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index c0df9507bd7f5e..10a152759ee693 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -9,6 +9,7 @@ import warnings import weakref from random import randrange, shuffle +import _testcapi from test import support from test.support import warnings_helper @@ -2154,6 +2155,31 @@ def test_cuboctahedron(self): for cubevert in edge: self.assertIn(cubevert, g) +class TestPySet_Add(unittest.TestCase): + def test_set(self): + # Test the PySet_Add c-api for set objects + s = set() + assert _testcapi.pyset_add(s, 1) == {1} + self.assertRaises(TypeError, _testcapi.pyset_add, s, []) + + def test_frozenset(self): + # Test the PySet_Add c-api for fronzetset objects + + assert _testcapi.pyset_add(frozenset(), 1) == frozenset([1]) + frozen_set = frozenset() + self.assertRaises(SystemError, _testcapi.pyset_add, frozen_set, 1) + + def test_frozenset_gc_tracking(self): + # see gh-140234 + class TrackedHashableClass(): + pass + + a = TrackedHashableClass() + result_set = _testcapi.pyset_add(frozenset(), 1) + assert not gc.is_tracked(result_set) + result_set = _testcapi.pyset_add(frozenset(), a) + assert gc.is_tracked(result_set) + #============================================================================== diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 739a4cf1c9f205..6ccb28c185f97d 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2437,60 +2437,23 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) static PyObject * -test_pyset_add(PyObject* self, PyObject *Py_UNUSED(args)) +// Interface to pyset_add, returning the set +pyset_add(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) { - - PyObject *set = NULL, *empty_tuple=NULL, *tracked_object; - int return_value; - - tracked_object = PyImport_ImportModule("sys"); - if (tracked_object == NULL) { - goto failed; - } - if (!PyObject_GC_IsTracked(tracked_object)) { - PyErr_SetString(PyExc_ValueError, "Test item is not tracked by GC"); - goto failed; - } - - empty_tuple = PyTuple_New(0); - if (empty_tuple == NULL) { - goto failed; - } - set = PyFrozenSet_New(empty_tuple); - if (set == NULL) { - goto failed; - } - - if (PyObject_GC_IsTracked(set)) { - PyErr_SetString(PyExc_ValueError, "Empty frozenset object is tracked by GC"); - goto failed; - } - return_value = PySet_Add(set, empty_tuple); - if (return_value<0) { - goto failed; - } - if (PyObject_GC_IsTracked(set)) { - PyErr_SetString(PyExc_ValueError, "Frozenset object with immutable is tracked by GC"); - goto failed; + Py_ssize_t nargs = _PyVectorcall_NARGS(nargsf); + if (nargs != 2) { + PyErr_SetString(PyExc_ValueError, "pyset_add requires exactly two arguments"); + return NULL; } + PyObject *set = args[0]; + PyObject *item = args[1]; - PySet_Add(set, tracked_object); - if (return_value<0) { - goto failed; - } - if (!PyObject_GC_IsTracked(set)) { - PyErr_SetString(PyExc_ValueError, "Frozenset object with tracked objects is not tracked by GC"); - goto failed; + int return_value = PySet_Add(set, item); + if (return_value < 0) { + return NULL; } - Py_RETURN_NONE; - -failed: - Py_XDECREF(tracked_object); - Py_XDECREF(empty_tuple); - Py_XDECREF(set); - return NULL; - + return Py_NewRef(set); } // Used by `finalize_thread_hang`. @@ -2704,7 +2667,7 @@ static PyMethodDef TestMethods[] = { {"gen_get_code", gen_get_code, METH_O, NULL}, {"get_feature_macros", get_feature_macros, METH_NOARGS, NULL}, {"test_code_api", test_code_api, METH_NOARGS, NULL}, - {"test_pyset_add", test_pyset_add, METH_NOARGS, NULL}, + {"pyset_add", _PyCFunction_CAST(pyset_add), METH_FASTCALL, NULL}, {"settrace_to_error", settrace_to_error, METH_O, NULL}, {"settrace_to_record", settrace_to_record, METH_O, NULL}, {"test_macros", test_macros, METH_NOARGS, NULL}, From 62afc766180c7f877c96e0c2becd71548cc81e60 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Oct 2025 20:24:24 +0100 Subject: [PATCH 10/29] whitespace --- Lib/test/test_set.py | 1 - Modules/_testcapimodule.c | 2 +- Objects/setobject.c | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index 10a152759ee693..ef86ead324111f 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -2164,7 +2164,6 @@ def test_set(self): def test_frozenset(self): # Test the PySet_Add c-api for fronzetset objects - assert _testcapi.pyset_add(frozenset(), 1) == frozenset([1]) frozen_set = frozenset() self.assertRaises(SystemError, _testcapi.pyset_add, frozen_set, 1) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 6ccb28c185f97d..0cd70cee4692d4 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2437,7 +2437,7 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) static PyObject * -// Interface to pyset_add, returning the set +// Interface to PySet_Add, returning the set pyset_add(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) { Py_ssize_t nargs = _PyVectorcall_NARGS(nargsf); diff --git a/Objects/setobject.c b/Objects/setobject.c index 8e89e637881bd1..cf3abccf558569 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1180,7 +1180,7 @@ _PyFrozenSet_MaybeUntrack(PyObject *op) if ((op == NULL) || !(PyFrozenSet_CheckExact(op))) { return; } - // if all elements of a frozenset are not tracked, we untrack the object + // if no elements of a frozenset are tracked, we untrack the object Py_ssize_t pos = 0; setentry *entry; while (set_next((PySetObject *)op, &pos, &entry)) { From 37fc61d77ad098376d9cb5c09ba46db26e7139bd Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Oct 2025 21:35:14 +0100 Subject: [PATCH 11/29] Apply suggestions from code review Co-authored-by: Mikhail Efimov --- Lib/test/test_set.py | 2 +- Objects/setobject.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index ef86ead324111f..dc93bdfbceabb4 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -2163,7 +2163,7 @@ def test_set(self): self.assertRaises(TypeError, _testcapi.pyset_add, s, []) def test_frozenset(self): - # Test the PySet_Add c-api for fronzetset objects + # Test the PySet_Add c-api for frozenset objects assert _testcapi.pyset_add(frozenset(), 1) == frozenset([1]) frozen_set = frozenset() self.assertRaises(SystemError, _testcapi.pyset_add, frozen_set, 1) diff --git a/Objects/setobject.c b/Objects/setobject.c index cf3abccf558569..ea49faacbf0255 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1177,7 +1177,7 @@ make_new_set_basetype(PyTypeObject *type, PyObject *iterable) void _PyFrozenSet_MaybeUntrack(PyObject *op) { - if ((op == NULL) || !(PyFrozenSet_CheckExact(op))) { + if (op == NULL || !PyFrozenSet_CheckExact(op)) { return; } // if no elements of a frozenset are tracked, we untrack the object From 4f8bda77afd1ef008fb874f82dd0e6ae4c550762 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 26 Oct 2025 21:42:23 +0100 Subject: [PATCH 12/29] review comment --- Objects/setobject.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index cf3abccf558569..8f89252ed268f1 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -2800,7 +2800,7 @@ PySet_Add(PyObject *anyset, PyObject *key) return -1; } - if (PyFrozenSet_Check(anyset) && PyObject_GC_IsTracked(key) && !PyObject_GC_IsTracked(anyset) ) { + if (PyFrozenSet_CheckExact(anyset) && PyObject_GC_IsTracked(key) && !PyObject_GC_IsTracked(anyset) ) { _PyObject_GC_TRACK(anyset); } int rv; From 4b39149cbec2f07d5e4fe569d42affdf07ff41b7 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 28 Oct 2025 21:57:59 +0100 Subject: [PATCH 13/29] Apply suggestions from code review Co-authored-by: Victor Stinner --- Lib/test/test_set.py | 6 +++--- Modules/_testcapimodule.c | 1 - Objects/setobject.c | 6 ++++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index dc93bdfbceabb4..779a30d6d93dbf 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -2159,7 +2159,7 @@ class TestPySet_Add(unittest.TestCase): def test_set(self): # Test the PySet_Add c-api for set objects s = set() - assert _testcapi.pyset_add(s, 1) == {1} + self.assertEqual(_testcapi.pyset_add(s, 1), {1}) self.assertRaises(TypeError, _testcapi.pyset_add, s, []) def test_frozenset(self): @@ -2175,9 +2175,9 @@ class TrackedHashableClass(): a = TrackedHashableClass() result_set = _testcapi.pyset_add(frozenset(), 1) - assert not gc.is_tracked(result_set) + self.assertFalse(gc.is_tracked(result_set)) result_set = _testcapi.pyset_add(frozenset(), a) - assert gc.is_tracked(result_set) + self.assertTrue(gc.is_tracked(result_set)) #============================================================================== diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 0cd70cee4692d4..176d807c586305 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2452,7 +2452,6 @@ pyset_add(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) if (return_value < 0) { return NULL; } - return Py_NewRef(set); } diff --git a/Objects/setobject.c b/Objects/setobject.c index 8a189e56507190..618b739a61b5c2 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1180,7 +1180,7 @@ _PyFrozenSet_MaybeUntrack(PyObject *op) if (op == NULL || !PyFrozenSet_CheckExact(op)) { return; } - // if no elements of a frozenset are tracked, we untrack the object + // if no elements of a frozenset are tracked by the GC, we untrack the object Py_ssize_t pos = 0; setentry *entry; while (set_next((PySetObject *)op, &pos, &entry)) { @@ -1203,7 +1203,9 @@ make_new_frozenset(PyTypeObject *type, PyObject *iterable) return Py_NewRef(iterable); } PyObject *obj = make_new_set(type, iterable); - _PyFrozenSet_MaybeUntrack(obj); + if (obj != NULL) { + _PyFrozenSet_MaybeUntrack(obj); + } return obj; } From 28598021f67bc20effea7d7754930c13f42e5838 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 28 Oct 2025 22:09:42 +0100 Subject: [PATCH 14/29] review comments --- Lib/test/test_set.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index 779a30d6d93dbf..9e4c45c840ff5e 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -2164,8 +2164,9 @@ def test_set(self): def test_frozenset(self): # Test the PySet_Add c-api for frozenset objects - assert _testcapi.pyset_add(frozenset(), 1) == frozenset([1]) + self.assertEqual(_testcapi.pyset_add(frozenset(), 1), frozenset([1])) frozen_set = frozenset() + # if the argument to PySet_Add is a frozenset that is not uniquely references an error is generated self.assertRaises(SystemError, _testcapi.pyset_add, frozen_set, 1) def test_frozenset_gc_tracking(self): From 08f43c514f7a0ec92391a70c1ab15b080a49f5fc Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Tue, 28 Oct 2025 22:18:01 +0100 Subject: [PATCH 15/29] review comments --- Objects/setobject.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index 618b739a61b5c2..60542a06e69323 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1175,9 +1175,12 @@ make_new_set_basetype(PyTypeObject *type, PyObject *iterable) } void +// gh-140232: check whether a frozenset can be untracked from the GC _PyFrozenSet_MaybeUntrack(PyObject *op) { - if (op == NULL || !PyFrozenSet_CheckExact(op)) { + assert(op != NULL); + // subclasses of a frozenset can generate reference cycles, so do not untrack + if (!PyFrozenSet_CheckExact(op)) { return; } // if no elements of a frozenset are tracked by the GC, we untrack the object @@ -2732,7 +2735,9 @@ PyObject * PyFrozenSet_New(PyObject *iterable) { PyObject *result = make_new_set(&PyFrozenSet_Type, iterable); - _PyFrozenSet_MaybeUntrack(result); + if (result != 0) { + _PyFrozenSet_MaybeUntrack(result); + } return result; } From 5f8b1f235f2b5b1d1fd7f186476a8363b0d473c1 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 18 Jan 2026 21:22:12 +0100 Subject: [PATCH 16/29] Update Objects/setobject.c --- Objects/setobject.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Objects/setobject.c b/Objects/setobject.c index f6502fed5f4585..cd9fd32c3fcc85 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -3038,7 +3038,6 @@ PySet_Add(PyObject *anyset, PyObject *key) // API limits the usage of `PySet_Add` to "fill in the values of brand // new frozensets before they are exposed to other code". In this case, // this can be done without a lock. - // since another key is added to the set, we must track the frozenset if needed if (PyFrozenSet_CheckExact(anyset) && PyObject_GC_IsTracked(key) && !PyObject_GC_IsTracked(anyset) ) { _PyObject_GC_TRACK(anyset); From ae5cc7f1fd16a0aa2376057df6286568bd726a21 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 23 Jan 2026 23:07:41 +0100 Subject: [PATCH 17/29] review comments --- Lib/test/test_capi/test_set.py | 27 +++++++++++++++++++++++++++ Lib/test/test_set.py | 27 --------------------------- Modules/_testcapimodule.c | 22 ---------------------- Modules/_testlimitedcapi/set.c | 17 +++++++++++++++++ Programs/test_frozenmain.h | 12 ++++++------ 5 files changed, 50 insertions(+), 55 deletions(-) diff --git a/Lib/test/test_capi/test_set.py b/Lib/test/test_capi/test_set.py index 62d90a3f94326d..02211a73388a90 100644 --- a/Lib/test/test_capi/test_set.py +++ b/Lib/test/test_capi/test_set.py @@ -1,3 +1,4 @@ +import gc import unittest from test.support import import_helper @@ -220,6 +221,32 @@ def test_clear(self): # CRASHES: clear(NULL) +class TestPySet_Add(unittest.TestCase): + def test_set(self): + # Test the PySet_Add c-api for set objects + s = set() + self.assertEqual(_testlimitedcapi.pyset_add(s, 1), {1}) + self.assertRaises(TypeError, _testlimitedcapi.pyset_add, s, []) + + def test_frozenset(self): + # Test the PySet_Add c-api for frozenset objects + self.assertEqual(_testlimitedcapi.pyset_add(frozenset(), 1), frozenset([1])) + frozen_set = frozenset() + # if the argument to PySet_Add is a frozenset that is not uniquely references an error is generated + self.assertRaises(SystemError, _testlimitedcapi.pyset_add, frozen_set, 1) + + def test_frozenset_gc_tracking(self): + # see gh-140234 + class TrackedHashableClass(): + pass + + a = TrackedHashableClass() + result_set = _testlimitedcapi.pyset_add(frozenset(), 1) + self.assertFalse(gc.is_tracked(result_set)) + result_set = _testlimitedcapi.pyset_add(frozenset(), a) + self.assertTrue(gc.is_tracked(result_set)) + + class TestInternalCAPI(BaseSetTests, unittest.TestCase): def test_set_update(self): update = _testinternalcapi.set_update diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index b9022d5e82e59f..e72507efbba323 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -9,7 +9,6 @@ import warnings import weakref from random import randrange, shuffle -import _testcapi from test import support from test.support import warnings_helper @@ -2213,32 +2212,6 @@ def test_cuboctahedron(self): for cubevert in edge: self.assertIn(cubevert, g) -class TestPySet_Add(unittest.TestCase): - def test_set(self): - # Test the PySet_Add c-api for set objects - s = set() - self.assertEqual(_testcapi.pyset_add(s, 1), {1}) - self.assertRaises(TypeError, _testcapi.pyset_add, s, []) - - def test_frozenset(self): - # Test the PySet_Add c-api for frozenset objects - self.assertEqual(_testcapi.pyset_add(frozenset(), 1), frozenset([1])) - frozen_set = frozenset() - # if the argument to PySet_Add is a frozenset that is not uniquely references an error is generated - self.assertRaises(SystemError, _testcapi.pyset_add, frozen_set, 1) - - def test_frozenset_gc_tracking(self): - # see gh-140234 - class TrackedHashableClass(): - pass - - a = TrackedHashableClass() - result_set = _testcapi.pyset_add(frozenset(), 1) - self.assertFalse(gc.is_tracked(result_set)) - result_set = _testcapi.pyset_add(frozenset(), a) - self.assertTrue(gc.is_tracked(result_set)) - - #============================================================================== if __name__ == "__main__": diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ce0b77c4efb6b6..9755c84334e338 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2434,27 +2434,6 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } - - -static PyObject * -// Interface to PySet_Add, returning the set -pyset_add(PyObject* self, PyObject* const* args, Py_ssize_t nargsf) -{ - Py_ssize_t nargs = _PyVectorcall_NARGS(nargsf); - if (nargs != 2) { - PyErr_SetString(PyExc_ValueError, "pyset_add requires exactly two arguments"); - return NULL; - } - PyObject *set = args[0]; - PyObject *item = args[1]; - - int return_value = PySet_Add(set, item); - if (return_value < 0) { - return NULL; - } - return Py_NewRef(set); -} - // Used by `finalize_thread_hang`. #if defined(_POSIX_THREADS) && !defined(__wasi__) static void finalize_thread_hang_cleanup_callback(void *Py_UNUSED(arg)) { @@ -2699,7 +2678,6 @@ static PyMethodDef TestMethods[] = { {"gen_get_code", gen_get_code, METH_O, NULL}, {"get_feature_macros", get_feature_macros, METH_NOARGS, NULL}, {"test_code_api", test_code_api, METH_NOARGS, NULL}, - {"pyset_add", _PyCFunction_CAST(pyset_add), METH_FASTCALL, NULL}, {"settrace_to_error", settrace_to_error, METH_O, NULL}, {"settrace_to_record", settrace_to_record, METH_O, NULL}, {"test_macros", test_macros, METH_NOARGS, NULL}, diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index 34ed6b1d60b5a4..62e88507d7acfc 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -200,6 +200,22 @@ test_set_contains_does_not_convert_unhashable_key(PyObject *self, PyObject *Py_U return NULL; } +// Interface to PySet_Add, returning the set +static PyObject * +pyset_add(PyObject *self, PyObject *args) +{ + PyObject *set, *item; + if (!PyArg_ParseTuple(args, "OO", &set, &item)) { + return NULL; + } + + int return_value = PySet_Add(set, item); + if (return_value < 0) { + return NULL; + } + return Py_NewRef(set); +} + static PyMethodDef test_methods[] = { {"set_check", set_check, METH_O}, {"set_checkexact", set_checkexact, METH_O}, @@ -221,6 +237,7 @@ static PyMethodDef test_methods[] = { {"test_frozenset_add_in_capi", test_frozenset_add_in_capi, METH_NOARGS}, {"test_set_contains_does_not_convert_unhashable_key", test_set_contains_does_not_convert_unhashable_key, METH_NOARGS}, + {"pyset_add", pyset_add, METH_VARARGS}, {NULL}, }; diff --git a/Programs/test_frozenmain.h b/Programs/test_frozenmain.h index dbeedb7ffe0ce6..b9bf4134b59718 100644 --- a/Programs/test_frozenmain.h +++ b/Programs/test_frozenmain.h @@ -13,10 +13,10 @@ unsigned char M_test_frozenmain[] = { 82,5,93,6,12,0,82,6,93,5,93,6,44,26,0,0, 0,0,0,0,0,0,0,0,12,0,50,4,52,1,0,0, 0,0,0,0,31,0,75,26,0,0,9,0,30,0,82,1, - 35,0,41,8,233,0,0,0,0,78,122,18,70,114,111,122, - 101,110,32,72,101,108,108,111,32,87,111,114,108,100,122,8, + 35,0,41,8,233,0,0,0,0,78,218,18,70,114,111,122, + 101,110,32,72,101,108,108,111,32,87,111,114,108,100,218,8, 115,121,115,46,97,114,103,118,218,6,99,111,110,102,105,103, - 122,7,99,111,110,102,105,103,32,122,2,58,32,41,5,218, + 218,7,99,111,110,102,105,103,32,218,2,58,32,41,5,218, 12,112,114,111,103,114,97,109,95,110,97,109,101,218,10,101, 120,101,99,117,116,97,98,108,101,218,15,117,115,101,95,101, 110,118,105,114,111,110,109,101,110,116,218,17,99,111,110,102, @@ -25,15 +25,15 @@ unsigned char M_test_frozenmain[] = { 3,115,121,115,218,17,95,116,101,115,116,105,110,116,101,114, 110,97,108,99,97,112,105,218,5,112,114,105,110,116,218,4, 97,114,103,118,218,11,103,101,116,95,99,111,110,102,105,103, - 115,114,3,0,0,0,218,3,107,101,121,169,0,243,0,0, + 115,114,5,0,0,0,218,3,107,101,121,169,0,243,0,0, 0,0,218,18,116,101,115,116,95,102,114,111,122,101,110,109, 97,105,110,46,112,121,218,8,60,109,111,100,117,108,101,62, - 114,18,0,0,0,1,0,0,0,115,94,0,0,0,240,3, + 114,22,0,0,0,1,0,0,0,115,94,0,0,0,240,3, 1,1,1,243,8,0,1,11,219,0,24,225,0,5,208,6, 26,212,0,27,217,0,5,128,106,144,35,151,40,145,40,212, 0,27,216,9,26,215,9,38,210,9,38,211,9,40,168,24, 213,9,50,128,6,243,2,6,12,2,128,67,241,14,0,5, 10,136,71,144,67,144,53,152,2,152,54,160,35,157,59,152, - 45,208,10,40,214,4,41,243,15,6,12,2,114,16,0,0, + 45,208,10,40,214,4,41,243,15,6,12,2,114,20,0,0, 0, }; From 786019dec247814fee0bf06184f67dc9327be5f4 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 23 Jan 2026 23:09:29 +0100 Subject: [PATCH 18/29] Update Lib/test/test_set.py --- Lib/test/test_set.py | 1 + 1 file changed, 1 insertion(+) diff --git a/Lib/test/test_set.py b/Lib/test/test_set.py index e72507efbba323..554716aed1e98b 100644 --- a/Lib/test/test_set.py +++ b/Lib/test/test_set.py @@ -2212,6 +2212,7 @@ def test_cuboctahedron(self): for cubevert in edge: self.assertIn(cubevert, g) + #============================================================================== if __name__ == "__main__": From f37f46ec227cdab894ce0f7da4736e2693b7252c Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 23 Jan 2026 23:10:19 +0100 Subject: [PATCH 19/29] Update Modules/_testcapimodule.c --- Modules/_testcapimodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 9755c84334e338..836fde3450efb8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2434,6 +2434,7 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } + // Used by `finalize_thread_hang`. #if defined(_POSIX_THREADS) && !defined(__wasi__) static void finalize_thread_hang_cleanup_callback(void *Py_UNUSED(arg)) { From 377e6d8a666e4f9a68736e343bc77a5eb20f689b Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Fri, 23 Jan 2026 23:26:07 +0100 Subject: [PATCH 20/29] Apply suggestions from code review --- Programs/test_frozenmain.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Programs/test_frozenmain.h b/Programs/test_frozenmain.h index b9bf4134b59718..dbeedb7ffe0ce6 100644 --- a/Programs/test_frozenmain.h +++ b/Programs/test_frozenmain.h @@ -13,10 +13,10 @@ unsigned char M_test_frozenmain[] = { 82,5,93,6,12,0,82,6,93,5,93,6,44,26,0,0, 0,0,0,0,0,0,0,0,12,0,50,4,52,1,0,0, 0,0,0,0,31,0,75,26,0,0,9,0,30,0,82,1, - 35,0,41,8,233,0,0,0,0,78,218,18,70,114,111,122, - 101,110,32,72,101,108,108,111,32,87,111,114,108,100,218,8, + 35,0,41,8,233,0,0,0,0,78,122,18,70,114,111,122, + 101,110,32,72,101,108,108,111,32,87,111,114,108,100,122,8, 115,121,115,46,97,114,103,118,218,6,99,111,110,102,105,103, - 218,7,99,111,110,102,105,103,32,218,2,58,32,41,5,218, + 122,7,99,111,110,102,105,103,32,122,2,58,32,41,5,218, 12,112,114,111,103,114,97,109,95,110,97,109,101,218,10,101, 120,101,99,117,116,97,98,108,101,218,15,117,115,101,95,101, 110,118,105,114,111,110,109,101,110,116,218,17,99,111,110,102, @@ -25,15 +25,15 @@ unsigned char M_test_frozenmain[] = { 3,115,121,115,218,17,95,116,101,115,116,105,110,116,101,114, 110,97,108,99,97,112,105,218,5,112,114,105,110,116,218,4, 97,114,103,118,218,11,103,101,116,95,99,111,110,102,105,103, - 115,114,5,0,0,0,218,3,107,101,121,169,0,243,0,0, + 115,114,3,0,0,0,218,3,107,101,121,169,0,243,0,0, 0,0,218,18,116,101,115,116,95,102,114,111,122,101,110,109, 97,105,110,46,112,121,218,8,60,109,111,100,117,108,101,62, - 114,22,0,0,0,1,0,0,0,115,94,0,0,0,240,3, + 114,18,0,0,0,1,0,0,0,115,94,0,0,0,240,3, 1,1,1,243,8,0,1,11,219,0,24,225,0,5,208,6, 26,212,0,27,217,0,5,128,106,144,35,151,40,145,40,212, 0,27,216,9,26,215,9,38,210,9,38,211,9,40,168,24, 213,9,50,128,6,243,2,6,12,2,128,67,241,14,0,5, 10,136,71,144,67,144,53,152,2,152,54,160,35,157,59,152, - 45,208,10,40,214,4,41,243,15,6,12,2,114,20,0,0, + 45,208,10,40,214,4,41,243,15,6,12,2,114,16,0,0, 0, }; From c62fa9a70a3809d2e188805924dfc4af74cae971 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sat, 24 Jan 2026 15:35:56 +0100 Subject: [PATCH 21/29] revert to fastcall --- Modules/_testlimitedcapi/set.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index 62e88507d7acfc..e7eefadea7964a 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -1,6 +1,12 @@ +#if !defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) + // Need limited C API for METH_FASTCALL + #define Py_LIMITED_API 0x030d0000 +#endif + #include "parts.h" #include "util.h" + static PyObject * set_check(PyObject *self, PyObject *obj) { @@ -202,12 +208,15 @@ test_set_contains_does_not_convert_unhashable_key(PyObject *self, PyObject *Py_U // Interface to PySet_Add, returning the set static PyObject * -pyset_add(PyObject *self, PyObject *args) +pyset_add(PyObject *self, PyObject *const *args, Py_ssize_t nargs) { - PyObject *set, *item; - if (!PyArg_ParseTuple(args, "OO", &set, &item)) { + if (nargs != 2) { + PyErr_SetString(PyExc_TypeError, + "pyset_add requires exactly 2 arguments"); return NULL; } + PyObject *set = args[0]; + PyObject *item = args[1]; int return_value = PySet_Add(set, item); if (return_value < 0) { @@ -237,7 +246,7 @@ static PyMethodDef test_methods[] = { {"test_frozenset_add_in_capi", test_frozenset_add_in_capi, METH_NOARGS}, {"test_set_contains_does_not_convert_unhashable_key", test_set_contains_does_not_convert_unhashable_key, METH_NOARGS}, - {"pyset_add", pyset_add, METH_VARARGS}, + {"pyset_add", _PyCFunction_CAST(pyset_add), METH_FASTCALL}, {NULL}, }; From 71d9eba9e1b517f2cdeb3c79639d30ba7e4549f0 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 25 Jan 2026 20:34:11 +0100 Subject: [PATCH 22/29] fix header --- Modules/_testlimitedcapi/set.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index e7eefadea7964a..08e1fba55f5114 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -1,3 +1,5 @@ +#include "pyconfig.h" // Py_GIL_DISABLED + #if !defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) // Need limited C API for METH_FASTCALL #define Py_LIMITED_API 0x030d0000 From 5c19de20a1df284ea7772cc76a5dce734cfa74ca Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Sun, 25 Jan 2026 23:32:45 +0100 Subject: [PATCH 23/29] Fully write test in C --- Lib/test/test_capi/test_set.py | 4 + Modules/_testcapimodule.c | 149 +++++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/Lib/test/test_capi/test_set.py b/Lib/test/test_capi/test_set.py index 02211a73388a90..a2c2cb52e078de 100644 --- a/Lib/test/test_capi/test_set.py +++ b/Lib/test/test_capi/test_set.py @@ -222,6 +222,10 @@ def test_clear(self): class TestPySet_Add(unittest.TestCase): + def test_pyset_add(self): + # Run C-level tests for PySet_Add + _testcapi.test_pyset_add() + def test_set(self): # Test the PySet_Add c-api for set objects s = set() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 836fde3450efb8..d11d4c8436a9a9 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2434,6 +2434,153 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } +static PyObject * +test_pyset_add_exact_set(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + // Test: Adding to a regular set + PyObject *set = PySet_New(NULL); + if (set == NULL) { + return NULL; + } + PyObject *one = PyLong_FromLong(1); + assert(one); + + if (PySet_Add(set, one) < 0) { + Py_DECREF(set); + return raiseTestError(self, "test_pyset_add", + "PySet_Add to empty set failed"); + } + if (PySet_Size(set) != 1) { + Py_DECREF(set); + return raiseTestError(self, "test_pyset_add", + "set size should be 1 after adding one element"); + } + if (PySet_Contains(set, one) != 1) { + Py_DECREF(set); + return raiseTestError(self, "test_pyset_add", + "set should contain the added element"); + } + Py_DECREF(set); + + // Test: Adding unhashable item should raise TypeError + set = PySet_New(NULL); + if (set == NULL) { + return NULL; + } + PyObject *unhashable = PyList_New(0); + if (unhashable == NULL) { + Py_DECREF(set); + return NULL; + } + if (PySet_Add(set, unhashable) != -1) { + Py_DECREF(unhashable); + Py_DECREF(set); + return raiseTestError(self, "test_pyset_add", + "PySet_Add with unhashable should fail"); + } + if (!PyErr_ExceptionMatches(PyExc_TypeError)) { + Py_DECREF(unhashable); + Py_DECREF(set); + return raiseTestError(self, "test_pyset_add", + "PySet_Add with unhashable should raise TypeError"); + } + PyErr_Clear(); + Py_DECREF(unhashable); + Py_DECREF(set); + + Py_RETURN_NONE; +} + +static PyObject * +test_pyset_add_frozenset(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *one = PyLong_FromLong(1); + assert(one); + + // Test: Adding to uniquely-referenced frozenset should succeed + PyObject *frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + + // frozenset is uniquely referenced here, so PySet_Add should work + if (PySet_Add(frozenset, one) < 0) { + Py_DECREF(frozenset); + return raiseTestError(self, "test_pyset_add_frozenset", + "PySet_Add to uniquely-referenced frozenset failed"); + } + Py_DECREF(frozenset); + + // Test: Adding to non-uniquely-referenced frozenset should raise SystemError + frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + Py_INCREF(frozenset); // Make it non-uniquely referenced + + if (PySet_Add(frozenset, one) != -1) { + Py_DECREF(frozenset); + Py_DECREF(frozenset); + return raiseTestError(self, "test_pyset_add_frozenset", + "PySet_Add to non-uniquely-referenced frozenset should fail"); + } + if (!PyErr_ExceptionMatches(PyExc_SystemError)) { + Py_DECREF(frozenset); + Py_DECREF(frozenset); + return raiseTestError(self, "test_pyset_add_frozenset", + "PySet_Add to non-uniquely-referenced frozenset should raise SystemError"); + } + PyErr_Clear(); + Py_DECREF(frozenset); + Py_DECREF(frozenset); + + // Test: GC tracking - frozenset with only immutable items should not be tracked + frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + if (PySet_Add(frozenset, one) < 0) { + Py_DECREF(frozenset); + return NULL; + } + if (PyObject_GC_IsTracked(frozenset)) { + Py_DECREF(frozenset); + return raiseTestError(self, "test_pyset_add_frozenset", + "frozenset with only int should not be GC tracked"); + } + Py_DECREF(frozenset); + + // Test: GC tracking - frozenset with tracked object should be tracked + frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + + PyObject *tracked_obj = PyErr_NewException("hashable_and_tracked", NULL, NULL); + if (tracked_obj == NULL) { + Py_DECREF(frozenset); + return NULL; + } + if (!PyObject_GC_IsTracked(tracked_obj)) { + return raiseTestError(self, "test_pyset_add_frozenset", + "test object should be tracked"); + } + if (PySet_Add(frozenset, tracked_obj) < 0) { + Py_DECREF(frozenset); + Py_DECREF(tracked_obj); + return NULL; + } + + if (!PyObject_GC_IsTracked(frozenset)) { + Py_DECREF(frozenset); + Py_DECREF(tracked_obj); + return raiseTestError(self, "test_pyset_add_frozenset", + "frozenset with with GC tracked object should be tracked"); + } + + Py_RETURN_NONE; +} + // Used by `finalize_thread_hang`. #if defined(_POSIX_THREADS) && !defined(__wasi__) @@ -2685,6 +2832,8 @@ static PyMethodDef TestMethods[] = { {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, {"function_set_warning", function_set_warning, METH_NOARGS}, {"test_critical_sections", test_critical_sections, METH_NOARGS}, + {"test_pyset_add_exact_set", test_pyset_add_exact_set, METH_NOARGS}, + {"test_pyset_add_frozenset", test_pyset_add_frozenset, METH_NOARGS}, {"finalize_thread_hang", finalize_thread_hang, METH_O, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL}, From e7dd2481097077090f64b2d271f923f9fa6f9b12 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 26 Jan 2026 09:11:28 +0100 Subject: [PATCH 24/29] adjust tests --- Lib/test/test_capi/test_set.py | 30 ++++-------------------------- Modules/_testcapimodule.c | 10 +++++----- 2 files changed, 9 insertions(+), 31 deletions(-) diff --git a/Lib/test/test_capi/test_set.py b/Lib/test/test_capi/test_set.py index a2c2cb52e078de..500acf524baf9b 100644 --- a/Lib/test/test_capi/test_set.py +++ b/Lib/test/test_capi/test_set.py @@ -1,4 +1,3 @@ -import gc import unittest from test.support import import_helper @@ -222,33 +221,12 @@ def test_clear(self): class TestPySet_Add(unittest.TestCase): - def test_pyset_add(self): - # Run C-level tests for PySet_Add - _testcapi.test_pyset_add() + def test_pyset_add_exact_set(self): + _testcapi.test_pyset_add_exact_set() - def test_set(self): - # Test the PySet_Add c-api for set objects - s = set() - self.assertEqual(_testlimitedcapi.pyset_add(s, 1), {1}) - self.assertRaises(TypeError, _testlimitedcapi.pyset_add, s, []) + def test_pyset_add_frozenset(self): + _testcapi.test_pyset_add_frozenset() - def test_frozenset(self): - # Test the PySet_Add c-api for frozenset objects - self.assertEqual(_testlimitedcapi.pyset_add(frozenset(), 1), frozenset([1])) - frozen_set = frozenset() - # if the argument to PySet_Add is a frozenset that is not uniquely references an error is generated - self.assertRaises(SystemError, _testlimitedcapi.pyset_add, frozen_set, 1) - - def test_frozenset_gc_tracking(self): - # see gh-140234 - class TrackedHashableClass(): - pass - - a = TrackedHashableClass() - result_set = _testlimitedcapi.pyset_add(frozenset(), 1) - self.assertFalse(gc.is_tracked(result_set)) - result_set = _testlimitedcapi.pyset_add(frozenset(), a) - self.assertTrue(gc.is_tracked(result_set)) class TestInternalCAPI(BaseSetTests, unittest.TestCase): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index d11d4c8436a9a9..7d88c50e30ae34 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2447,17 +2447,17 @@ test_pyset_add_exact_set(PyObject *self, PyObject *Py_UNUSED(ignored)) if (PySet_Add(set, one) < 0) { Py_DECREF(set); - return raiseTestError(self, "test_pyset_add", + return raiseTestError(self, "test_pyset_add_exact_set", "PySet_Add to empty set failed"); } if (PySet_Size(set) != 1) { Py_DECREF(set); - return raiseTestError(self, "test_pyset_add", + return raiseTestError(self, "test_pyset_add_exact_set", "set size should be 1 after adding one element"); } if (PySet_Contains(set, one) != 1) { Py_DECREF(set); - return raiseTestError(self, "test_pyset_add", + return raiseTestError(self, "test_pyset_add_exact_set", "set should contain the added element"); } Py_DECREF(set); @@ -2475,13 +2475,13 @@ test_pyset_add_exact_set(PyObject *self, PyObject *Py_UNUSED(ignored)) if (PySet_Add(set, unhashable) != -1) { Py_DECREF(unhashable); Py_DECREF(set); - return raiseTestError(self, "test_pyset_add", + return raiseTestError(self, "test_pyset_add_exact_set", "PySet_Add with unhashable should fail"); } if (!PyErr_ExceptionMatches(PyExc_TypeError)) { Py_DECREF(unhashable); Py_DECREF(set); - return raiseTestError(self, "test_pyset_add", + return raiseTestError(self, "test_pyset_add_exact_set", "PySet_Add with unhashable should raise TypeError"); } PyErr_Clear(); From 075b58245666d8e8f0d17643df0b8e6030f59033 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 26 Jan 2026 14:56:30 +0100 Subject: [PATCH 25/29] cleanup tests --- Lib/test/test_capi/test_set.py | 9 -- Modules/_testcapimodule.c | 149 ------------------------------ Modules/_testlimitedcapi/set.c | 163 ++++++++++++++++++++++++++++++++- 3 files changed, 161 insertions(+), 160 deletions(-) diff --git a/Lib/test/test_capi/test_set.py b/Lib/test/test_capi/test_set.py index 500acf524baf9b..62d90a3f94326d 100644 --- a/Lib/test/test_capi/test_set.py +++ b/Lib/test/test_capi/test_set.py @@ -220,15 +220,6 @@ def test_clear(self): # CRASHES: clear(NULL) -class TestPySet_Add(unittest.TestCase): - def test_pyset_add_exact_set(self): - _testcapi.test_pyset_add_exact_set() - - def test_pyset_add_frozenset(self): - _testcapi.test_pyset_add_frozenset() - - - class TestInternalCAPI(BaseSetTests, unittest.TestCase): def test_set_update(self): update = _testinternalcapi.set_update diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 7d88c50e30ae34..836fde3450efb8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2434,153 +2434,6 @@ test_critical_sections(PyObject *module, PyObject *Py_UNUSED(args)) Py_RETURN_NONE; } -static PyObject * -test_pyset_add_exact_set(PyObject *self, PyObject *Py_UNUSED(ignored)) -{ - // Test: Adding to a regular set - PyObject *set = PySet_New(NULL); - if (set == NULL) { - return NULL; - } - PyObject *one = PyLong_FromLong(1); - assert(one); - - if (PySet_Add(set, one) < 0) { - Py_DECREF(set); - return raiseTestError(self, "test_pyset_add_exact_set", - "PySet_Add to empty set failed"); - } - if (PySet_Size(set) != 1) { - Py_DECREF(set); - return raiseTestError(self, "test_pyset_add_exact_set", - "set size should be 1 after adding one element"); - } - if (PySet_Contains(set, one) != 1) { - Py_DECREF(set); - return raiseTestError(self, "test_pyset_add_exact_set", - "set should contain the added element"); - } - Py_DECREF(set); - - // Test: Adding unhashable item should raise TypeError - set = PySet_New(NULL); - if (set == NULL) { - return NULL; - } - PyObject *unhashable = PyList_New(0); - if (unhashable == NULL) { - Py_DECREF(set); - return NULL; - } - if (PySet_Add(set, unhashable) != -1) { - Py_DECREF(unhashable); - Py_DECREF(set); - return raiseTestError(self, "test_pyset_add_exact_set", - "PySet_Add with unhashable should fail"); - } - if (!PyErr_ExceptionMatches(PyExc_TypeError)) { - Py_DECREF(unhashable); - Py_DECREF(set); - return raiseTestError(self, "test_pyset_add_exact_set", - "PySet_Add with unhashable should raise TypeError"); - } - PyErr_Clear(); - Py_DECREF(unhashable); - Py_DECREF(set); - - Py_RETURN_NONE; -} - -static PyObject * -test_pyset_add_frozenset(PyObject *self, PyObject *Py_UNUSED(ignored)) -{ - PyObject *one = PyLong_FromLong(1); - assert(one); - - // Test: Adding to uniquely-referenced frozenset should succeed - PyObject *frozenset = PyFrozenSet_New(NULL); - if (frozenset == NULL) { - return NULL; - } - - // frozenset is uniquely referenced here, so PySet_Add should work - if (PySet_Add(frozenset, one) < 0) { - Py_DECREF(frozenset); - return raiseTestError(self, "test_pyset_add_frozenset", - "PySet_Add to uniquely-referenced frozenset failed"); - } - Py_DECREF(frozenset); - - // Test: Adding to non-uniquely-referenced frozenset should raise SystemError - frozenset = PyFrozenSet_New(NULL); - if (frozenset == NULL) { - return NULL; - } - Py_INCREF(frozenset); // Make it non-uniquely referenced - - if (PySet_Add(frozenset, one) != -1) { - Py_DECREF(frozenset); - Py_DECREF(frozenset); - return raiseTestError(self, "test_pyset_add_frozenset", - "PySet_Add to non-uniquely-referenced frozenset should fail"); - } - if (!PyErr_ExceptionMatches(PyExc_SystemError)) { - Py_DECREF(frozenset); - Py_DECREF(frozenset); - return raiseTestError(self, "test_pyset_add_frozenset", - "PySet_Add to non-uniquely-referenced frozenset should raise SystemError"); - } - PyErr_Clear(); - Py_DECREF(frozenset); - Py_DECREF(frozenset); - - // Test: GC tracking - frozenset with only immutable items should not be tracked - frozenset = PyFrozenSet_New(NULL); - if (frozenset == NULL) { - return NULL; - } - if (PySet_Add(frozenset, one) < 0) { - Py_DECREF(frozenset); - return NULL; - } - if (PyObject_GC_IsTracked(frozenset)) { - Py_DECREF(frozenset); - return raiseTestError(self, "test_pyset_add_frozenset", - "frozenset with only int should not be GC tracked"); - } - Py_DECREF(frozenset); - - // Test: GC tracking - frozenset with tracked object should be tracked - frozenset = PyFrozenSet_New(NULL); - if (frozenset == NULL) { - return NULL; - } - - PyObject *tracked_obj = PyErr_NewException("hashable_and_tracked", NULL, NULL); - if (tracked_obj == NULL) { - Py_DECREF(frozenset); - return NULL; - } - if (!PyObject_GC_IsTracked(tracked_obj)) { - return raiseTestError(self, "test_pyset_add_frozenset", - "test object should be tracked"); - } - if (PySet_Add(frozenset, tracked_obj) < 0) { - Py_DECREF(frozenset); - Py_DECREF(tracked_obj); - return NULL; - } - - if (!PyObject_GC_IsTracked(frozenset)) { - Py_DECREF(frozenset); - Py_DECREF(tracked_obj); - return raiseTestError(self, "test_pyset_add_frozenset", - "frozenset with with GC tracked object should be tracked"); - } - - Py_RETURN_NONE; -} - // Used by `finalize_thread_hang`. #if defined(_POSIX_THREADS) && !defined(__wasi__) @@ -2832,8 +2685,6 @@ static PyMethodDef TestMethods[] = { {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, {"function_set_warning", function_set_warning, METH_NOARGS}, {"test_critical_sections", test_critical_sections, METH_NOARGS}, - {"test_pyset_add_exact_set", test_pyset_add_exact_set, METH_NOARGS}, - {"test_pyset_add_frozenset", test_pyset_add_frozenset, METH_NOARGS}, {"finalize_thread_hang", finalize_thread_hang, METH_O, NULL}, {"test_atexit", test_atexit, METH_NOARGS}, {"code_offset_to_line", _PyCFunction_CAST(code_offset_to_line), METH_FASTCALL}, diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index 08e1fba55f5114..de0aea705e47f2 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -8,7 +8,6 @@ #include "parts.h" #include "util.h" - static PyObject * set_check(PyObject *self, PyObject *obj) { @@ -128,6 +127,73 @@ set_clear(PyObject *self, PyObject *obj) RETURN_INT(PySet_Clear(obj)); } +/* Raise AssertionError with test_name + ": " + msg, and return NULL. */ + +static PyObject * +raiseTestError(const char* test_name, const char* msg) +{ + PyObject *exc = PyErr_GetRaisedException(); + PyErr_Format(exc, "%s: %s", test_name, msg); + return NULL; +} + +static PyObject * +test_pyset_add_exact_set(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + // Test: Adding to a regular set + PyObject *set = PySet_New(NULL); + if (set == NULL) { + return NULL; + } + PyObject *one = PyLong_FromLong(1); + assert(one); + + if (PySet_Add(set, one) < 0) { + Py_DECREF(set); + return raiseTestError("test_pyset_add_exact_set", + "PySet_Add to empty set failed"); + } + if (PySet_Size(set) != 1) { + Py_DECREF(set); + return raiseTestError("test_pyset_add_exact_set", + "set size should be 1 after adding one element"); + } + if (PySet_Contains(set, one) != 1) { + Py_DECREF(set); + return raiseTestError("test_pyset_add_exact_set", + "set should contain the added element"); + } + Py_DECREF(set); + + // Test: Adding unhashable item should raise TypeError + set = PySet_New(NULL); + if (set == NULL) { + return NULL; + } + PyObject *unhashable = PyList_New(0); + if (unhashable == NULL) { + Py_DECREF(set); + return NULL; + } + if (PySet_Add(set, unhashable) != -1) { + Py_DECREF(unhashable); + Py_DECREF(set); + return raiseTestError("test_pyset_add_exact_set", + "PySet_Add with unhashable should fail"); + } + if (!PyErr_ExceptionMatches(PyExc_TypeError)) { + Py_DECREF(unhashable); + Py_DECREF(set); + return raiseTestError("test_pyset_add_exact_set", + "PySet_Add with unhashable should raise TypeError"); + } + PyErr_Clear(); + Py_DECREF(unhashable); + Py_DECREF(set); + + Py_RETURN_NONE; +} + static PyObject * test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj)) { @@ -163,6 +229,98 @@ test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj)) return NULL; } +static PyObject * +test_pyset_add_frozenset(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyObject *one = PyLong_FromLong(1); + assert(one); + + // Test: Adding to uniquely-referenced frozenset should succeed + PyObject *frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + + // frozenset is uniquely referenced here, so PySet_Add should work + if (PySet_Add(frozenset, one) < 0) { + Py_DECREF(frozenset); + return raiseTestError("test_pyset_add_frozenset", + "PySet_Add to uniquely-referenced frozenset failed"); + } + Py_DECREF(frozenset); + + // Test: Adding to non-uniquely-referenced frozenset should raise SystemError + frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + Py_INCREF(frozenset); // Make it non-uniquely referenced + + if (PySet_Add(frozenset, one) != -1) { + Py_DECREF(frozenset); + Py_DECREF(frozenset); + return raiseTestError("test_pyset_add_frozenset", + "PySet_Add to non-uniquely-referenced frozenset should fail"); + } + if (!PyErr_ExceptionMatches(PyExc_SystemError)) { + Py_DECREF(frozenset); + Py_DECREF(frozenset); + return raiseTestError("test_pyset_add_frozenset", + "PySet_Add to non-uniquely-referenced frozenset should raise SystemError"); + } + PyErr_Clear(); + Py_DECREF(frozenset); + Py_DECREF(frozenset); + + // Test: GC tracking - frozenset with only immutable items should not be tracked + frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + if (PySet_Add(frozenset, one) < 0) { + Py_DECREF(frozenset); + return NULL; + } + if (PyObject_GC_IsTracked(frozenset)) { + Py_DECREF(frozenset); + return raiseTestError("test_pyset_add_frozenset", + "frozenset with only int should not be GC tracked"); + } + Py_DECREF(frozenset); + + // Test: GC tracking - frozenset with tracked object should be tracked + frozenset = PyFrozenSet_New(NULL); + if (frozenset == NULL) { + return NULL; + } + + PyObject *tracked_obj = PyErr_NewException("_testlimitedcapi.py_set_add", NULL, NULL); + if (tracked_obj == NULL) { + Py_DECREF(frozenset); + return NULL; + } + if (!PyObject_GC_IsTracked(tracked_obj)) { + return raiseTestError("test_pyset_add_frozenset", + "test object should be tracked"); + } + Py_RETURN_NONE; + if (PySet_Add(frozenset, tracked_obj) < 0) { + Py_DECREF(frozenset); + Py_DECREF(tracked_obj); + return NULL; + } + + if (!PyObject_GC_IsTracked(frozenset)) { + Py_DECREF(frozenset); + Py_DECREF(tracked_obj); + return raiseTestError("test_pyset_add_frozenset", + "frozenset with with GC tracked object should be tracked"); + } + + Py_RETURN_NONE; +} + + static PyObject * test_set_contains_does_not_convert_unhashable_key(PyObject *self, PyObject *Py_UNUSED(obj)) { @@ -248,7 +406,8 @@ static PyMethodDef test_methods[] = { {"test_frozenset_add_in_capi", test_frozenset_add_in_capi, METH_NOARGS}, {"test_set_contains_does_not_convert_unhashable_key", test_set_contains_does_not_convert_unhashable_key, METH_NOARGS}, - {"pyset_add", _PyCFunction_CAST(pyset_add), METH_FASTCALL}, + {"test_pyset_add_exact_set", test_pyset_add_exact_set, METH_NOARGS}, + {"test_pyset_add_frozenset", test_pyset_add_frozenset, METH_NOARGS}, {NULL}, }; From 3dd4c958fd344f75dcb293cfebec38c1d738dcd7 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 26 Jan 2026 14:59:55 +0100 Subject: [PATCH 26/29] cleanup tests --- Modules/_testcapimodule.c | 2 +- Modules/_testlimitedcapi/set.c | 27 --------------------------- 2 files changed, 1 insertion(+), 28 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 836fde3450efb8..c0ab35cda191c8 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2658,7 +2658,7 @@ static PyMethodDef TestMethods[] = { {"return_null_without_error", return_null_without_error, METH_NOARGS}, {"return_result_with_error", return_result_with_error, METH_NOARGS}, {"getitem_with_error", getitem_with_error, METH_VARARGS}, - {"Py_CompileString", pycompilestring, METH_O}, + {"Py_CompileString", pycompilestring, METH_O}, {"raise_SIGINT_then_send_None", raise_SIGINT_then_send_None, METH_VARARGS}, {"stack_pointer", stack_pointer, METH_NOARGS}, #ifdef W_STOPCODE diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index de0aea705e47f2..09c713b2085af9 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -1,10 +1,3 @@ -#include "pyconfig.h" // Py_GIL_DISABLED - -#if !defined(Py_GIL_DISABLED) && !defined(Py_LIMITED_API) - // Need limited C API for METH_FASTCALL - #define Py_LIMITED_API 0x030d0000 -#endif - #include "parts.h" #include "util.h" @@ -303,13 +296,11 @@ test_pyset_add_frozenset(PyObject *self, PyObject *Py_UNUSED(ignored)) return raiseTestError("test_pyset_add_frozenset", "test object should be tracked"); } - Py_RETURN_NONE; if (PySet_Add(frozenset, tracked_obj) < 0) { Py_DECREF(frozenset); Py_DECREF(tracked_obj); return NULL; } - if (!PyObject_GC_IsTracked(frozenset)) { Py_DECREF(frozenset); Py_DECREF(tracked_obj); @@ -366,24 +357,6 @@ test_set_contains_does_not_convert_unhashable_key(PyObject *self, PyObject *Py_U return NULL; } -// Interface to PySet_Add, returning the set -static PyObject * -pyset_add(PyObject *self, PyObject *const *args, Py_ssize_t nargs) -{ - if (nargs != 2) { - PyErr_SetString(PyExc_TypeError, - "pyset_add requires exactly 2 arguments"); - return NULL; - } - PyObject *set = args[0]; - PyObject *item = args[1]; - - int return_value = PySet_Add(set, item); - if (return_value < 0) { - return NULL; - } - return Py_NewRef(set); -} static PyMethodDef test_methods[] = { {"set_check", set_check, METH_O}, From 8d864ef70b8c8c052028e2ee20fda583b297d40d Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 26 Jan 2026 17:31:04 +0100 Subject: [PATCH 27/29] rework --- Lib/test/test_capi/test_set.py | 6 +++ Modules/_testlimitedcapi/set.c | 96 +--------------------------------- 2 files changed, 7 insertions(+), 95 deletions(-) diff --git a/Lib/test/test_capi/test_set.py b/Lib/test/test_capi/test_set.py index 62d90a3f94326d..a41c2fcaf12270 100644 --- a/Lib/test/test_capi/test_set.py +++ b/Lib/test/test_capi/test_set.py @@ -166,6 +166,12 @@ def test_add(self): # CRASHES: add(instance, NULL) # CRASHES: add(NULL, NULL) + def test_add_frozenset(self): + add = _testlimitedcapi.set_add + frozen_set = frozenset() + # test adding an element to a non-uniquely referenced frozenset throws an exception + self.assertRaises(SystemError, add, frozen_set, 1) + def test_discard(self): discard = _testlimitedcapi.set_discard for cls in (set, set_subclass): diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index 09c713b2085af9..1b243595a2dcb5 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -130,62 +130,6 @@ raiseTestError(const char* test_name, const char* msg) return NULL; } -static PyObject * -test_pyset_add_exact_set(PyObject *self, PyObject *Py_UNUSED(ignored)) -{ - // Test: Adding to a regular set - PyObject *set = PySet_New(NULL); - if (set == NULL) { - return NULL; - } - PyObject *one = PyLong_FromLong(1); - assert(one); - - if (PySet_Add(set, one) < 0) { - Py_DECREF(set); - return raiseTestError("test_pyset_add_exact_set", - "PySet_Add to empty set failed"); - } - if (PySet_Size(set) != 1) { - Py_DECREF(set); - return raiseTestError("test_pyset_add_exact_set", - "set size should be 1 after adding one element"); - } - if (PySet_Contains(set, one) != 1) { - Py_DECREF(set); - return raiseTestError("test_pyset_add_exact_set", - "set should contain the added element"); - } - Py_DECREF(set); - - // Test: Adding unhashable item should raise TypeError - set = PySet_New(NULL); - if (set == NULL) { - return NULL; - } - PyObject *unhashable = PyList_New(0); - if (unhashable == NULL) { - Py_DECREF(set); - return NULL; - } - if (PySet_Add(set, unhashable) != -1) { - Py_DECREF(unhashable); - Py_DECREF(set); - return raiseTestError("test_pyset_add_exact_set", - "PySet_Add with unhashable should fail"); - } - if (!PyErr_ExceptionMatches(PyExc_TypeError)) { - Py_DECREF(unhashable); - Py_DECREF(set); - return raiseTestError("test_pyset_add_exact_set", - "PySet_Add with unhashable should raise TypeError"); - } - PyErr_Clear(); - Py_DECREF(unhashable); - Py_DECREF(set); - - Py_RETURN_NONE; -} static PyObject * test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj)) @@ -223,48 +167,11 @@ test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj)) } static PyObject * -test_pyset_add_frozenset(PyObject *self, PyObject *Py_UNUSED(ignored)) +test_frozenset_add_in_capi_tracking(PyObject *self, PyObject *Py_UNUSED(ignored)) { PyObject *one = PyLong_FromLong(1); assert(one); - // Test: Adding to uniquely-referenced frozenset should succeed - PyObject *frozenset = PyFrozenSet_New(NULL); - if (frozenset == NULL) { - return NULL; - } - - // frozenset is uniquely referenced here, so PySet_Add should work - if (PySet_Add(frozenset, one) < 0) { - Py_DECREF(frozenset); - return raiseTestError("test_pyset_add_frozenset", - "PySet_Add to uniquely-referenced frozenset failed"); - } - Py_DECREF(frozenset); - - // Test: Adding to non-uniquely-referenced frozenset should raise SystemError - frozenset = PyFrozenSet_New(NULL); - if (frozenset == NULL) { - return NULL; - } - Py_INCREF(frozenset); // Make it non-uniquely referenced - - if (PySet_Add(frozenset, one) != -1) { - Py_DECREF(frozenset); - Py_DECREF(frozenset); - return raiseTestError("test_pyset_add_frozenset", - "PySet_Add to non-uniquely-referenced frozenset should fail"); - } - if (!PyErr_ExceptionMatches(PyExc_SystemError)) { - Py_DECREF(frozenset); - Py_DECREF(frozenset); - return raiseTestError("test_pyset_add_frozenset", - "PySet_Add to non-uniquely-referenced frozenset should raise SystemError"); - } - PyErr_Clear(); - Py_DECREF(frozenset); - Py_DECREF(frozenset); - // Test: GC tracking - frozenset with only immutable items should not be tracked frozenset = PyFrozenSet_New(NULL); if (frozenset == NULL) { @@ -379,7 +286,6 @@ static PyMethodDef test_methods[] = { {"test_frozenset_add_in_capi", test_frozenset_add_in_capi, METH_NOARGS}, {"test_set_contains_does_not_convert_unhashable_key", test_set_contains_does_not_convert_unhashable_key, METH_NOARGS}, - {"test_pyset_add_exact_set", test_pyset_add_exact_set, METH_NOARGS}, {"test_pyset_add_frozenset", test_pyset_add_frozenset, METH_NOARGS}, {NULL}, From e262963af92e95c6d6ec34f398c4d5e20ba9a140 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 26 Jan 2026 17:47:29 +0100 Subject: [PATCH 28/29] refactor code --- Modules/_testlimitedcapi/set.c | 61 +++++++++++++++++----------------- 1 file changed, 31 insertions(+), 30 deletions(-) diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index 1b243595a2dcb5..a94dc8912371d9 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -120,17 +120,6 @@ set_clear(PyObject *self, PyObject *obj) RETURN_INT(PySet_Clear(obj)); } -/* Raise AssertionError with test_name + ": " + msg, and return NULL. */ - -static PyObject * -raiseTestError(const char* test_name, const char* msg) -{ - PyObject *exc = PyErr_GetRaisedException(); - PyErr_Format(exc, "%s: %s", test_name, msg); - return NULL; -} - - static PyObject * test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj)) { @@ -167,55 +156,67 @@ test_frozenset_add_in_capi(PyObject *self, PyObject *Py_UNUSED(obj)) } static PyObject * -test_frozenset_add_in_capi_tracking(PyObject *self, PyObject *Py_UNUSED(ignored)) +raiseTestError(const char* test_name, const char* msg) { - PyObject *one = PyLong_FromLong(1); - assert(one); + PyObject *exc = PyErr_GetRaisedException(); + PyErr_Format(PyErr_GetRaisedException(), "%s: %s", test_name, msg); + return NULL; +} +static PyObject * +test_frozenset_add_in_capi_tracking_immutable(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ // Test: GC tracking - frozenset with only immutable items should not be tracked - frozenset = PyFrozenSet_New(NULL); + PyObject *frozenset = PyFrozenSet_New(NULL); if (frozenset == NULL) { return NULL; } - if (PySet_Add(frozenset, one) < 0) { + if (PySet_Add(frozenset, Py_True) < 0) { Py_DECREF(frozenset); return NULL; } if (PyObject_GC_IsTracked(frozenset)) { Py_DECREF(frozenset); - return raiseTestError("test_pyset_add_frozenset", - "frozenset with only int should not be GC tracked"); + return raiseTestError("test_frozenset_add_in_capi_tracking_immutable", + "frozenset with only int should not be GC tracked"); } Py_DECREF(frozenset); + Py_RETURN_NONE; +} +static PyObject * +test_frozenset_add_in_capi_tracking(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ // Test: GC tracking - frozenset with tracked object should be tracked - frozenset = PyFrozenSet_New(NULL); + PyObject *frozenset = PyFrozenSet_New(NULL); if (frozenset == NULL) { return NULL; } PyObject *tracked_obj = PyErr_NewException("_testlimitedcapi.py_set_add", NULL, NULL); if (tracked_obj == NULL) { - Py_DECREF(frozenset); - return NULL; + goto error; } if (!PyObject_GC_IsTracked(tracked_obj)) { - return raiseTestError("test_pyset_add_frozenset", + return raiseTestError("test_frozenset_add_in_capi_tracking", "test object should be tracked"); } if (PySet_Add(frozenset, tracked_obj) < 0) { - Py_DECREF(frozenset); - Py_DECREF(tracked_obj); - return NULL; + goto error; } + Py_DECREF(tracked_obj); if (!PyObject_GC_IsTracked(frozenset)) { Py_DECREF(frozenset); - Py_DECREF(tracked_obj); - return raiseTestError("test_pyset_add_frozenset", + return raiseTestError("test_frozenset_add_in_capi_tracking", "frozenset with with GC tracked object should be tracked"); } - + Py_DECREF(frozenset); Py_RETURN_NONE; + +error: + Py_DECREF(frozenset); + Py_XDECREF(tracked_obj); + return NULL; } @@ -264,7 +265,6 @@ test_set_contains_does_not_convert_unhashable_key(PyObject *self, PyObject *Py_U return NULL; } - static PyMethodDef test_methods[] = { {"set_check", set_check, METH_O}, {"set_checkexact", set_checkexact, METH_O}, @@ -284,9 +284,10 @@ static PyMethodDef test_methods[] = { {"set_clear", set_clear, METH_O}, {"test_frozenset_add_in_capi", test_frozenset_add_in_capi, METH_NOARGS}, + {"test_frozenset_add_in_capi_tracking", test_frozenset_add_in_capi_tracking, METH_NOARGS}, + {"test_frozenset_add_in_capi_tracking_immutable", test_frozenset_add_in_capi_tracking_immutable, METH_NOARGS}, {"test_set_contains_does_not_convert_unhashable_key", test_set_contains_does_not_convert_unhashable_key, METH_NOARGS}, - {"test_pyset_add_frozenset", test_pyset_add_frozenset, METH_NOARGS}, {NULL}, }; From 60273914e47e9456fee39e9b39114caaf2eac266 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Mon, 26 Jan 2026 17:53:00 +0100 Subject: [PATCH 29/29] cleanup --- Modules/_testlimitedcapi/set.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testlimitedcapi/set.c b/Modules/_testlimitedcapi/set.c index a94dc8912371d9..950de494f525c9 100644 --- a/Modules/_testlimitedcapi/set.c +++ b/Modules/_testlimitedcapi/set.c @@ -159,7 +159,7 @@ static PyObject * raiseTestError(const char* test_name, const char* msg) { PyObject *exc = PyErr_GetRaisedException(); - PyErr_Format(PyErr_GetRaisedException(), "%s: %s", test_name, msg); + PyErr_Format(exc, "%s: %s", test_name, msg); return NULL; }