diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index 8ee351918006e4b..0d79ec56cf30eed 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 new reference to the removed + value, and return ``1``. + - If the key is missing and *default_value* is not NULL, set *\*result* + 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``. + + 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..ac159cc8060c07e 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. */ - popresult = _PyDict_Pop_KnownHash(self->cache, link->key, - link->hash, Py_None); - 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__) 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/Modules/_threadmodule.c b/Modules/_threadmodule.c index 88ca9032b5e6798..9a842d6ed8c4a19 100644 --- a/Modules/_threadmodule.c +++ b/Modules/_threadmodule.c @@ -967,12 +967,13 @@ 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) { + // Silently ignore error + 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 719d438897ca6cf..1a14a7e56fd87ec 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) @@ -3463,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 b99896319e0136c..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. */ - 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/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/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) 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);