Add PyModule_Add() function

PyModule_AddObjectRef() now raises SystemError if the value is NULL and no exception is set.
python · Jul 18, 2023 · e3f0eba · e3f0eba
1 parent 7515dae
commit e3f0eba
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 15 deletions.
diff --git a/docs/api.rst b/docs/api.rst
@@ -51,6 +51,10 @@ Python 3.13
 
    See `PyMapping_GetOptionalItemString() documentation <https://docs.python.org/dev/c-api/mapping.html#c.PyMapping_GetOptionalItemString>`__.
 
+.. c:function:: int PyModule_Add(PyObject *module, const char *name, PyObject *value)
+
+   See `PyModule_Add() documentation <https://docs.python.org/dev/c-api/module.html#c.PyModule_Add>`__.
+
 
 Python 3.12
 -----------

diff --git a/docs/changelog.rst b/docs/changelog.rst
@@ -1,6 +1,7 @@
 Changelog
 =========
 
+* 2023-07-18: Add ``PyModule_Add()`` function.
 * 2023-07-12: Add ``PyObject_GetOptionalAttr()``,
   ``PyObject_GetOptionalAttrString()``,
   ``PyMapping_GetOptionalItem()``

diff --git a/pythoncapi_compat.h b/pythoncapi_compat.h
@@ -422,6 +422,15 @@ PYCAPI_COMPAT_STATIC_INLINE(int)
 PyModule_AddObjectRef(PyObject *module, const char *name, PyObject *value)
 {
     int res;
+
+    if (!value && !PyErr_Occurred()) {
+        // PyModule_AddObject() raises TypeError in this case
+        PyErr_SetString(PyExc_SystemError,
+                        "PyModule_AddObjectRef() must be called "
+                        "with an exception raised if value is NULL");
+        return -1;
+    }
+
     Py_XINCREF(value);
     res = PyModule_AddObject(module, name, value);
     if (res < 0) {
@@ -782,6 +791,18 @@ PyMapping_GetOptionalItemString(PyObject *obj, const char *key, PyObject **resul
 #endif
 
 
+// gh-106307 added PyModule_Add() to Python 3.13.0a1
+#if PY_VERSION_HEX < 0x030D00A1
+PYCAPI_COMPAT_STATIC_INLINE(int)
+PyModule_Add(PyObject *mod, const char *name, PyObject *value)
+{
+    int res = PyModule_AddObjectRef(mod, name, value);
+    Py_XDECREF(value);
+    return res;
+}
+#endif
+
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/tests/test_pythoncapi_compat_cext.c b/tests/test_pythoncapi_compat_cext.c
@@ -386,6 +386,23 @@ test_gc(PyObject *Py_UNUSED(module), PyObject* Py_UNUSED(ignored))
 }
 
 
+static int
+check_module_attr(PyObject *module, const char *name, PyObject *expected)
+{
+    PyObject *attr = PyObject_GetAttrString(module, name);
+    if (attr == _Py_NULL) {
+        return -1;
+    }
+    assert(attr == expected);
+    Py_DECREF(attr);
+
+    if (PyObject_DelAttrString(module, name) < 0) {
+        return -1;
+    }
+    return 0;
+}
+
+
 // test PyModule_AddType()
 static int
 test_module_add_type(PyObject *module)
@@ -407,14 +424,7 @@ test_module_add_type(PyObject *module)
     ASSERT_REFCNT(Py_REFCNT(type) == refcnt + 1);
 #endif
 
-    PyObject *attr = PyObject_GetAttrString(module, type_name);
-    if (attr == _Py_NULL) {
-        return -1;
-    }
-    assert(attr == _Py_CAST(PyObject*, type));
-    Py_DECREF(attr);
-
-    if (PyObject_DelAttrString(module, type_name) < 0) {
+    if (check_module_attr(module, type_name, _Py_CAST(PyObject*, type)) < 0) {
         return -1;
     }
     ASSERT_REFCNT(Py_REFCNT(type) == refcnt);
@@ -426,30 +436,70 @@ test_module_add_type(PyObject *module)
 static int
 test_module_addobjectref(PyObject *module)
 {
-    PyObject *obj = Py_True;
     const char *name = "test_module_addobjectref";
+    PyObject *obj = PyUnicode_FromString(name);
+    assert(obj != NULL);
 #ifdef CHECK_REFCNT
     Py_ssize_t refcnt = Py_REFCNT(obj);
 #endif
 
     if (PyModule_AddObjectRef(module, name, obj) < 0) {
         ASSERT_REFCNT(Py_REFCNT(obj) == refcnt);
+        Py_DECREF(obj);
         return -1;
     }
-#ifndef IMMORTAL_OBJS
     ASSERT_REFCNT(Py_REFCNT(obj) == refcnt + 1);
-#endif
 
-    if (PyObject_DelAttrString(module, name) < 0) {
+    if (check_module_attr(module, name, obj) < 0) {
+        Py_DECREF(obj);
         return -1;
     }
     ASSERT_REFCNT(Py_REFCNT(obj) == refcnt);
 
     // PyModule_AddObjectRef() with value=NULL must not crash
+    assert(!PyErr_Occurred());
     int res = PyModule_AddObjectRef(module, name, _Py_NULL);
     assert(res < 0);
+    assert(PyErr_ExceptionMatches(PyExc_SystemError));
+    PyErr_Clear();
+
+    Py_DECREF(obj);
+    return 0;
+}
+
+
+// test PyModule_Add()
+static int
+test_module_add(PyObject *module)
+{
+    const char *name = "test_module_add";
+    PyObject *obj = PyUnicode_FromString(name);
+    assert(obj != NULL);
+#ifdef CHECK_REFCNT
+    Py_ssize_t refcnt = Py_REFCNT(obj);
+#endif
+
+    if (PyModule_Add(module, name, Py_NewRef(obj)) < 0) {
+        ASSERT_REFCNT(Py_REFCNT(obj) == refcnt);
+        Py_DECREF(obj);
+        return -1;
+    }
+    ASSERT_REFCNT(Py_REFCNT(obj) == refcnt + 1);
+
+    if (check_module_attr(module, name, obj) < 0) {
+        Py_DECREF(obj);
+        return -1;
+    }
+    ASSERT_REFCNT(Py_REFCNT(obj) == refcnt);
+
+    // PyModule_Add() with value=NULL must not crash
+    assert(!PyErr_Occurred());
+    int res = PyModule_Add(module, name, _Py_NULL);
+    assert(res < 0);
+    assert(PyErr_ExceptionMatches(PyExc_SystemError));
     PyErr_Clear();
 
+    Py_DECREF(obj);
     return 0;
 }
 
@@ -458,18 +508,20 @@ static PyObject *
 test_module(PyObject *Py_UNUSED(module), PyObject* Py_UNUSED(ignored))
 {
     PyObject *module = PyImport_ImportModule("sys");
-    if (module == _Py_NULL) {
-        return _Py_NULL;
+    if (module == NULL) {
+        return NULL;
     }
     assert(PyModule_Check(module));
 
     if (test_module_add_type(module) < 0) {
         goto error;
     }
-
     if (test_module_addobjectref(module) < 0) {
         goto error;
     }
+    if (test_module_add(module) < 0) {
+        goto error;
+    }
 
     Py_DECREF(module);
     Py_RETURN_NONE;