review comments

Author: eendebakpt

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

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__":

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},

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},
 };