diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index 8ee351918006e4..0d79ec56cf30ee 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 811b1bd84d2417..6f0c3ca3fdfc9c 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 ae3c88d28d73a0..063876503895d6 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 1bbeec1ab699e7..6453265d74d0d1 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 d01ef55de51f5d..6fada82e7b00df 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 67f12a56313b6f..5c517b64e46648 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 4976ac3642bbe4..6cd9eb5794c19a 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 00000000000000..ceb20fca086aa7 --- /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 22b25dd0ec141f..e6f464986fbb1f 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 8ea493ad9ab278..ac159cc8060c07 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 5f6a1a037dcba2..cb0ca6cf480c4c 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 88ca9032b5e679..9a842d6ed8c4a1 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 97f4d174666750..c9ad3c8d94a91c 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 719d438897ca6c..1a14a7e56fd87e 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 b99896319e0136..2147eb2bbbad49 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 db4aebebdd8404..2bcb4e62b04151 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 07aa84c91f9fc7..8876f3607b863f 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 b6ffba5c5746e2..32f37b629ea57e 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 e28523284f1517..342f8fe2cba14b 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);