From b3e7de44f2ff653cf833bd45653ebda2a6630a3c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 10 Nov 2023 09:55:52 +0100 Subject: [PATCH 1/6] gh-111262: Add PyDict_Pop() function Change the API of the internal _PyDict_Pop_KnownHash() function to return an int. Co-Authored-By: Stefan Behnel --- Doc/c-api/dict.rst | 17 ++++ Doc/data/stable_abi.dat | 1 + Doc/whatsnew/3.13.rst | 4 + Include/dictobject.h | 5 ++ Include/internal/pycore_dict.h | 7 +- Lib/test/test_capi/test_dict.py | 43 +++++++++ Lib/test/test_stable_abi_ctypes.py | 1 + ...-11-10-10-21-38.gh-issue-111262.2utB5m.rst | 3 + Misc/stable_abi.toml | 2 + Modules/_functoolsmodule.c | 4 +- Modules/_testcapi/dict.c | 21 ++++- Objects/dictobject.c | 88 +++++++++++++------ Objects/odictobject.c | 2 +- PC/python3dll.c | 1 + 14 files changed, 165 insertions(+), 34 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-11-10-10-21-38.gh-issue-111262.2utB5m.rst diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index 8ee351918006e4b..5cbc6a22a19a3bf 100644 --- a/Doc/c-api/dict.rst +++ b/Doc/c-api/dict.rst @@ -173,6 +173,23 @@ Dictionary Objects .. versionadded:: 3.4 + +.. c:function:: int PyDict_Pop(PyObject *p, PyObject *key, PyObject *default_value, PyObject *result) + + Remove *key* from dictionary *p* and return the removed value. + + - If the key is present, set *\*result* to a :term:`strong reference` to the + removed value, and return ``1``. + - If the key is missing and *default_value* is not NULL, set *\*result* + to a :term:`strong reference` to *default_value*, and return ``0``. + - If the key is missing and *default_value* is NULL, raise :exc:`KeyError`, + and return -1. + - On error, raise an exception and return ``-1``. + + This is the same as :meth:`dict.pop`. + + .. versionadded:: 3.13 + .. c:function:: PyObject* PyDict_Items(PyObject *p) Return a :c:type:`PyListObject` containing all the items from the dictionary. diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 811b1bd84d24174..6f0c3ca3fdfc9c8 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -121,6 +121,7 @@ function,PyDict_Merge,3.2,, function,PyDict_MergeFromSeq2,3.2,, function,PyDict_New,3.2,, function,PyDict_Next,3.2,, +function,PyDict_Pop,3.13,, function,PyDict_SetItem,3.2,, function,PyDict_SetItemString,3.2,, function,PyDict_Size,3.2,, diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index ae3c88d28d73a08..063876503895d6c 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -1164,6 +1164,10 @@ New Features :c:func:`PyErr_WriteUnraisable`, but allow to customize the warning mesage. (Contributed by Serhiy Storchaka in :gh:`108082`.) +* Add :c:func:`PyDict_Pop` function: remove a key from a dictionary and return + the removed value. This is the same as :meth:`dict.pop`. + (Contributed by Victor Stinner in :gh:`111262`.) + Porting to Python 3.13 ---------------------- diff --git a/Include/dictobject.h b/Include/dictobject.h index 1bbeec1ab699e75..6453265d74d0d11 100644 --- a/Include/dictobject.h +++ b/Include/dictobject.h @@ -66,6 +66,11 @@ PyAPI_FUNC(int) PyDict_DelItemString(PyObject *dp, const char *key); // - On error, raise an exception and return -1. PyAPI_FUNC(int) PyDict_GetItemRef(PyObject *mp, PyObject *key, PyObject **result); PyAPI_FUNC(int) PyDict_GetItemStringRef(PyObject *mp, const char *key, PyObject **result); + +PyAPI_FUNC(int) PyDict_Pop(PyObject *dict, + PyObject *key, + PyObject *default_value, + PyObject **result); #endif #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 diff --git a/Include/internal/pycore_dict.h b/Include/internal/pycore_dict.h index d01ef55de51f5d1..6fada82e7b00df9 100644 --- a/Include/internal/pycore_dict.h +++ b/Include/internal/pycore_dict.h @@ -116,7 +116,12 @@ extern PyObject *_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObject *); extern int _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value); extern int _PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr, PyObject *name, PyObject *value); -extern PyObject *_PyDict_Pop_KnownHash(PyObject *, PyObject *, Py_hash_t, PyObject *); +extern int _PyDict_Pop_KnownHash( + PyDictObject *dict, + PyObject *key, + Py_hash_t hash, + PyObject *default_value, + PyObject **result); #define DKIX_EMPTY (-1) #define DKIX_DUMMY (-2) /* Used internally */ diff --git a/Lib/test/test_capi/test_dict.py b/Lib/test/test_capi/test_dict.py index 67f12a56313b6f4..5c517b64e466481 100644 --- a/Lib/test/test_capi/test_dict.py +++ b/Lib/test/test_capi/test_dict.py @@ -432,6 +432,49 @@ def test_dict_mergefromseq2(self): # CRASHES mergefromseq2({}, NULL, 0) # CRASHES mergefromseq2(NULL, {}, 0) + def test_dict_pop(self): + # Test PyDict_Pop() + dict_pop = _testcapi.dict_pop + default = object() + + def expect_value(mydict, key, expected_value): + self.assertEqual(dict_pop(mydict.copy(), key, default), + (1, expected_value)) + self.assertEqual(dict_pop(mydict.copy(), key, NULL), + (1, expected_value)) + + def check_default(mydict, key): + result, value = dict_pop(mydict, key, default) + self.assertIs(value, default) + self.assertEqual(result, 0) + + # key present + mydict = {"key": 2, "key2": 5} + expect_value(mydict, "key", 2) + expect_value(mydict, "key2", 5) + + # key missing + check_default({}, "key") # empty dict has a fast path + check_default({"a": 1}, "key") + self.assertRaises(KeyError, dict_pop, {}, "key", NULL) + self.assertRaises(KeyError, dict_pop, {"a": 1}, "key", NULL) + + # dict error + not_dict = "string" + self.assertRaises(SystemError, dict_pop, not_dict, "key", default) + + # key error + not_hashable_key = ["list"] + check_default({}, not_hashable_key) # don't hash key if dict is empty + with self.assertRaises(TypeError): + dict_pop({'key': 1}, not_hashable_key, NULL) + with self.assertRaises(TypeError): + dict_pop({'key': 1}, not_hashable_key, default) + dict_pop({}, NULL, default) # don't check key if dict is empty + + # CRASHES dict_pop(NULL, "key") + # CRASHES dict_pop({"a": 1}, NULL, default) + if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 4976ac3642bbe46..6cd9eb5794c19a9 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -154,6 +154,7 @@ def test_windows_feature_macros(self): "PyDict_MergeFromSeq2", "PyDict_New", "PyDict_Next", + "PyDict_Pop", "PyDict_SetItem", "PyDict_SetItemString", "PyDict_Size", diff --git a/Misc/NEWS.d/next/C API/2023-11-10-10-21-38.gh-issue-111262.2utB5m.rst b/Misc/NEWS.d/next/C API/2023-11-10-10-21-38.gh-issue-111262.2utB5m.rst new file mode 100644 index 000000000000000..ceb20fca086aa79 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-11-10-10-21-38.gh-issue-111262.2utB5m.rst @@ -0,0 +1,3 @@ +Add :c:func:`PyDict_Pop` function: remove a key from a dictionary and return +the removed value. This is the same as :meth:`dict.pop`. Patch by Stefan +Behnel and Victor Stinner. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 22b25dd0ec141fd..e6f464986fbb1fa 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2481,3 +2481,5 @@ [function._Py_SetRefcnt] added = '3.13' abi_only = true +[function.PyDict_Pop] + added = '3.13' diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 8ea493ad9ab278e..94745072fb7e41e 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1087,8 +1087,8 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds The cache dict holds one reference to the link. We created one other reference when the link was created. The linked list only has borrowed references. */ - popresult = _PyDict_Pop_KnownHash(self->cache, link->key, - link->hash, Py_None); + (void)_PyDict_Pop_KnownHash((PyDictObject*)self->cache, link->key, + link->hash, Py_None, &popresult); if (popresult == Py_None) { /* Getting here means that the user function call or another thread has already removed the old key from the dictionary. diff --git a/Modules/_testcapi/dict.c b/Modules/_testcapi/dict.c index 5f6a1a037dcba29..cb0ca6cf480c4c6 100644 --- a/Modules/_testcapi/dict.c +++ b/Modules/_testcapi/dict.c @@ -331,6 +331,25 @@ dict_mergefromseq2(PyObject *self, PyObject *args) } +static PyObject * +dict_pop(PyObject *self, PyObject *args) +{ + PyObject *dict, *key, *default_value; + if (!PyArg_ParseTuple(args, "OOO", &dict, &key, &default_value)) { + return NULL; + } + NULLABLE(dict); + NULLABLE(key); + NULLABLE(default_value); + PyObject *result = UNINITIALIZED_PTR; + int res = PyDict_Pop(dict, key, default_value, &result); + if (result == NULL) { + return NULL; + } + return Py_BuildValue("iN", res, result); +} + + static PyMethodDef test_methods[] = { {"dict_check", dict_check, METH_O}, {"dict_checkexact", dict_checkexact, METH_O}, @@ -358,7 +377,7 @@ static PyMethodDef test_methods[] = { {"dict_merge", dict_merge, METH_VARARGS}, {"dict_update", dict_update, METH_VARARGS}, {"dict_mergefromseq2", dict_mergefromseq2, METH_VARARGS}, - + {"dict_pop", dict_pop, METH_VARARGS}, {NULL}, }; diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 719d438897ca6cf..1c804c6cb32b923 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2226,64 +2226,94 @@ PyDict_Next(PyObject *op, Py_ssize_t *ppos, PyObject **pkey, PyObject **pvalue) return _PyDict_Next(op, ppos, pkey, pvalue, NULL); } + /* Internal version of dict.pop(). */ -PyObject * -_PyDict_Pop_KnownHash(PyObject *dict, PyObject *key, Py_hash_t hash, PyObject *deflt) +int +_PyDict_Pop_KnownHash(PyDictObject *mp, PyObject *key, Py_hash_t hash, + PyObject *default_value, PyObject **result) { - Py_ssize_t ix; - PyObject *old_value; - PyDictObject *mp; - PyInterpreterState *interp = _PyInterpreterState_GET(); - - assert(PyDict_Check(dict)); - mp = (PyDictObject *)dict; + assert(PyDict_Check(mp)); if (mp->ma_used == 0) { - if (deflt) { - return Py_NewRef(deflt); + if (default_value) { + *result = Py_NewRef(default_value); + return 0; } + *result = NULL; _PyErr_SetKeyError(key); - return NULL; + return -1; } - ix = _Py_dict_lookup(mp, key, hash, &old_value); - if (ix == DKIX_ERROR) - return NULL; + + PyObject *old_value; + Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &old_value); + if (ix == DKIX_ERROR) { + *result = NULL; + return -1; + } + if (ix == DKIX_EMPTY || old_value == NULL) { - if (deflt) { - return Py_NewRef(deflt); + if (default_value) { + *result = Py_NewRef(default_value); + return 0; } + *result = NULL; _PyErr_SetKeyError(key); - return NULL; + return -1; } + assert(old_value != NULL); + PyInterpreterState *interp = _PyInterpreterState_GET(); uint64_t new_version = _PyDict_NotifyEvent( interp, PyDict_EVENT_DELETED, mp, key, NULL); delitem_common(mp, hash, ix, Py_NewRef(old_value), new_version); ASSERT_CONSISTENT(mp); - return old_value; + *result = old_value; + return 1; } -PyObject * -_PyDict_Pop(PyObject *dict, PyObject *key, PyObject *deflt) + +int +PyDict_Pop(PyObject *op, PyObject *key, PyObject *default_value, PyObject **result) { - Py_hash_t hash; + if (!PyDict_Check(op)) { + *result = NULL; + PyErr_BadInternalCall(); + return -1; + } + PyDictObject *dict = (PyDictObject *)op; - if (((PyDictObject *)dict)->ma_used == 0) { - if (deflt) { - return Py_NewRef(deflt); + if (dict->ma_used == 0) { + if (default_value) { + *result = Py_NewRef(default_value); + return 0; } + *result = NULL; _PyErr_SetKeyError(key); - return NULL; + return -1; } + + Py_hash_t hash; if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) { hash = PyObject_Hash(key); - if (hash == -1) - return NULL; + if (hash == -1) { + *result = NULL; + return -1; + } } - return _PyDict_Pop_KnownHash(dict, key, hash, deflt); + return _PyDict_Pop_KnownHash(dict, key, hash, default_value, result); } + +PyObject * +_PyDict_Pop(PyObject *dict, PyObject *key, PyObject *default_value) +{ + PyObject *result; + (void)PyDict_Pop(dict, key, default_value, &result); + return result; +} + + /* Internal version of dict.from_keys(). It is subclass-friendly. */ PyObject * _PyDict_FromKeys(PyObject *cls, PyObject *iterable, PyObject *value) diff --git a/Objects/odictobject.c b/Objects/odictobject.c index b99896319e0136c..3b148bbe3721596 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1049,7 +1049,7 @@ _odict_popkey_hash(PyObject *od, PyObject *key, PyObject *failobj, return NULL; } /* Now delete the value from the dict. */ - value = _PyDict_Pop_KnownHash(od, key, hash, failobj); + (void)_PyDict_Pop_KnownHash((PyDictObject *)od, key, hash, failobj, &value); } else if (value == NULL && !PyErr_Occurred()) { /* Apply the fallback value, if necessary. */ diff --git a/PC/python3dll.c b/PC/python3dll.c index 07aa84c91f9fc7f..8876f3607b863f6 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -184,6 +184,7 @@ EXPORT_FUNC(PyDict_Merge) EXPORT_FUNC(PyDict_MergeFromSeq2) EXPORT_FUNC(PyDict_New) EXPORT_FUNC(PyDict_Next) +EXPORT_FUNC(PyDict_Pop) EXPORT_FUNC(PyDict_SetItem) EXPORT_FUNC(PyDict_SetItemString) EXPORT_FUNC(PyDict_Size) From 9b2005341c8c8fc4568b29b2ae81bd7c97c6dd70 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 13 Nov 2023 00:10:13 +0100 Subject: [PATCH 2/6] Update Doc/c-api/dict.rst Co-authored-by: Antoine Pitrou --- Doc/c-api/dict.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index 5cbc6a22a19a3bf..b1ae72e332481ea 100644 --- a/Doc/c-api/dict.rst +++ b/Doc/c-api/dict.rst @@ -174,7 +174,7 @@ Dictionary Objects .. versionadded:: 3.4 -.. c:function:: int PyDict_Pop(PyObject *p, PyObject *key, PyObject *default_value, PyObject *result) +.. c:function:: int PyDict_Pop(PyObject *p, PyObject *key, PyObject *default_value, PyObject **result) Remove *key* from dictionary *p* and return the removed value. From 096ff502e766a687d0972f4bbe314a2d6e8edeb3 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 13 Nov 2023 00:17:39 +0100 Subject: [PATCH 3/6] Use _PyDict_Pop_KnownHash() return value --- Modules/_functoolsmodule.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 94745072fb7e41e..9428f5f7b9360dd 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1087,19 +1087,8 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds The cache dict holds one reference to the link. We created one other reference when the link was created. The linked list only has borrowed references. */ - (void)_PyDict_Pop_KnownHash((PyDictObject*)self->cache, link->key, - link->hash, Py_None, &popresult); - if (popresult == Py_None) { - /* Getting here means that the user function call or another - thread has already removed the old key from the dictionary. - This link is now an orphan. Since we don't want to leave the - cache in an inconsistent state, we don't restore the link. */ - Py_DECREF(popresult); - Py_DECREF(link); - Py_DECREF(key); - return result; - } - if (popresult == NULL) { + if (_PyDict_Pop_KnownHash((PyDictObject*)self->cache, link->key, + link->hash, Py_None, &popresult) < 0) { /* An error arose while trying to remove the oldest key (the one being evicted) from the cache. We restore the link to its original position as the oldest link. Then we allow the @@ -1110,6 +1099,17 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds Py_DECREF(result); return NULL; } + + if (popresult == Py_None) { + /* Getting here means that the user function call or another + thread has already removed the old key from the dictionary. + This link is now an orphan. Since we don't want to leave the + cache in an inconsistent state, we don't restore the link. */ + Py_DECREF(popresult); + Py_DECREF(link); + Py_DECREF(key); + return result; + } /* Keep a reference to the old key and old result to prevent their ref counts from going to zero during the update. That will prevent potentially arbitrary object clean-up code (i.e. __del__) From 7596d95dec10476a208ee2956e2d624edf1d7ddb Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 13 Nov 2023 00:22:04 +0100 Subject: [PATCH 4/6] Use PyDict_Pop() --- Modules/_threadmodule.c | 8 ++++---- Modules/socketmodule.c | 7 ++++--- Objects/dictobject.c | 4 +++- Objects/odictobject.c | 3 ++- Objects/structseq.c | 8 ++++---- Python/import.c | 3 ++- Python/sysmodule.c | 3 +-- 7 files changed, 20 insertions(+), 16 deletions(-) diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 88ca9032b5e6798..8c67df5126a3548 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -967,12 +967,12 @@ local_clear(localobject *self) HEAD_UNLOCK(runtime); while (tstate) { if (tstate->dict) { - PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None); - if (v != NULL) { - Py_DECREF(v); + PyObject *v; + if (PyDict_Pop(tstate->dict, self->key, Py_None, &v) < 0) { + PyErr_Clear(); } else { - PyErr_Clear(); + Py_DECREF(v); } } HEAD_LOCK(runtime); diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 97f4d1746667501..c9ad3c8d94a91c1 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -396,11 +396,12 @@ remove_unusable_flags(PyObject *m) if (flag_name == NULL) { return -1; } - PyObject *v = _PyDict_Pop(dict, flag_name, Py_None); - Py_DECREF(flag_name); - if (v == NULL) { + PyObject *v; + if (PyDict_Pop(dict, flag_name, Py_None, &v) < 0) { + Py_DECREF(flag_name); return -1; } + Py_DECREF(flag_name); Py_DECREF(v); } } diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 1c804c6cb32b923..1a14a7e56fd87ec 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -3493,7 +3493,9 @@ static PyObject * dict_pop_impl(PyDictObject *self, PyObject *key, PyObject *default_value) /*[clinic end generated code: output=3abb47b89f24c21c input=e221baa01044c44c]*/ { - return _PyDict_Pop((PyObject*)self, key, default_value); + PyObject *result; + PyDict_Pop((PyObject*)self, key, default_value, &result); + return result; } /*[clinic input] diff --git a/Objects/odictobject.c b/Objects/odictobject.c index 3b148bbe3721596..2147eb2bbbad49c 100644 --- a/Objects/odictobject.c +++ b/Objects/odictobject.c @@ -1049,7 +1049,8 @@ _odict_popkey_hash(PyObject *od, PyObject *key, PyObject *failobj, return NULL; } /* Now delete the value from the dict. */ - (void)_PyDict_Pop_KnownHash((PyDictObject *)od, key, hash, failobj, &value); + (void)_PyDict_Pop_KnownHash((PyDictObject *)od, key, hash, + failobj, &value); } else if (value == NULL && !PyErr_Occurred()) { /* Apply the fallback value, if necessary. */ diff --git a/Objects/structseq.c b/Objects/structseq.c index db4aebebdd8404e..2bcb4e62b041518 100644 --- a/Objects/structseq.c +++ b/Objects/structseq.c @@ -8,7 +8,6 @@ */ #include "Python.h" -#include "pycore_dict.h" // _PyDict_Pop() #include "pycore_initconfig.h" // _PyStatus_OK() #include "pycore_modsupport.h" // _PyArg_NoPositional() #include "pycore_object.h" // _PyObject_GC_TRACK() @@ -421,11 +420,12 @@ structseq_replace(PyStructSequence *self, PyObject *args, PyObject *kwargs) if (!key) { goto error; } - PyObject *ob = _PyDict_Pop(kwargs, key, self->ob_item[i]); - Py_DECREF(key); - if (!ob) { + PyObject *ob; + if (PyDict_Pop(kwargs, key, self->ob_item[i], &ob) < 0) { + Py_DECREF(key); goto error; } + Py_DECREF(key); result->ob_item[i] = ob; } // Check if there are any unexpected fields. diff --git a/Python/import.c b/Python/import.c index b6ffba5c5746e2d..32f37b629ea57e6 100644 --- a/Python/import.c +++ b/Python/import.c @@ -395,7 +395,8 @@ remove_module(PyThreadState *tstate, PyObject *name) PyObject *modules = MODULES(tstate->interp); if (PyDict_CheckExact(modules)) { - PyObject *mod = _PyDict_Pop(modules, name, Py_None); + PyObject *mod; + (void)PyDict_Pop(modules, name, Py_None, &mod); Py_XDECREF(mod); } else if (PyMapping_DelItem(modules, name) < 0) { diff --git a/Python/sysmodule.c b/Python/sysmodule.c index e28523284f15176..342f8fe2cba14b7 100644 --- a/Python/sysmodule.c +++ b/Python/sysmodule.c @@ -125,8 +125,7 @@ sys_set_object(PyInterpreterState *interp, PyObject *key, PyObject *v) } PyObject *sd = interp->sysdict; if (v == NULL) { - v = _PyDict_Pop(sd, key, Py_None); - if (v == NULL) { + if (PyDict_Pop(sd, key, Py_None, &v) < 0) { return -1; } Py_DECREF(v); From 9ec6aa4e4445bf18cdf2283e308c7956d90eb4d8 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 13 Nov 2023 00:30:44 +0100 Subject: [PATCH 5/6] Cleanup --- Modules/_functoolsmodule.c | 2 +- Modules/_threadmodule.c | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_functoolsmodule.c b/Modules/_functoolsmodule.c index 9428f5f7b9360dd..ac159cc8060c07e 100644 --- a/Modules/_functoolsmodule.c +++ b/Modules/_functoolsmodule.c @@ -1088,7 +1088,7 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds We created one other reference when the link was created. The linked list only has borrowed references. */ if (_PyDict_Pop_KnownHash((PyDictObject*)self->cache, link->key, - link->hash, Py_None, &popresult) < 0) { + link->hash, Py_None, &popresult) < 0) { /* An error arose while trying to remove the oldest key (the one being evicted) from the cache. We restore the link to its original position as the oldest link. Then we allow the diff --git a/Modules/_threadmodule.c b/Modules/_threadmodule.c index 8c67df5126a3548..9a842d6ed8c4a19 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -969,6 +969,7 @@ local_clear(localobject *self) if (tstate->dict) { PyObject *v; if (PyDict_Pop(tstate->dict, self->key, Py_None, &v) < 0) { + // Silently ignore error PyErr_Clear(); } else { From 787346d6a6fae0e1a26180a757f6a3da97910573 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 13 Nov 2023 00:34:25 +0100 Subject: [PATCH 6/6] Use "new reference" in the doc --- Doc/c-api/dict.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index b1ae72e332481ea..0d79ec56cf30eed 100644 --- a/Doc/c-api/dict.rst +++ b/Doc/c-api/dict.rst @@ -178,10 +178,10 @@ Dictionary Objects Remove *key* from dictionary *p* and return the removed value. - - If the key is present, set *\*result* to a :term:`strong reference` to the - removed value, and return ``1``. + - If the key is present, set *\*result* to a new reference to the removed + value, and return ``1``. - If the key is missing and *default_value* is not NULL, set *\*result* - to a :term:`strong reference` to *default_value*, and return ``0``. + to a new reference to *default_value*, and return ``0``. - If the key is missing and *default_value* is NULL, raise :exc:`KeyError`, and return -1. - On error, raise an exception and return ``-1``.