8000 gh-111262: Add PyDict_Pop() function · vstinner/cpython@148d06c · GitHub
[go: up one dir, main page]
Skip to content

Commit 148d06c

Browse files
vstinnerscoderpitrou
committed
pythongh-111262: Add PyDict_Pop() function
_PyDict_Pop_KnownHash(): remove the default value and the return type becomes an int. Co-Authored-By: Stefan Behnel <stefan_ml@behnel.de> Co-authored-by: Antoine Pitrou <pitrou@free.fr>
1 parent 1c7ed7e commit 148d06c

File tree

19 files changed

+199
-67
lines changed

19 files changed

+199
-67
lines changed

‎Doc/c-api/dict.rst

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,21 @@ Dictionary Objects
173173
174174
.. versionadded:: 3.4
175175
176+
177+
.. c:function:: int PyDict_Pop(PyObject *p, PyObject *key, PyObject **result)
178+
179+
Remove *key* from dictionary *p* and optionally return the removed value.
180+
181+
- If the key is present, set *\*result* to a new reference to the removed
182+
value (if *result* is not ``NULL``), and return ``1``.
183+
- If the key is missing, set *\*result* to ``NULL`` (if *result* is not
184+
``NULL``), and return ``0``.
185+
- On error, raise an exception and return ``-1``.
186+
187+
This is the similar to :meth:`dict.pop` without the default value.
188+
189+
.. versionadded:: 3.13
190+
176191
.. c:function:: PyObject* PyDict_Items(PyObject *p)
177192
178193
Return a :c:type:`PyListObject` containing all the items from the dictionary.

‎Doc/data/stable_abi.dat

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Doc/whatsnew/3.13.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1164,6 +1164,11 @@ New Features
11641164
:c:func:`PyErr_WriteUnraisable`, but allow to customize the warning mesage.
11651165
(Contributed by Serhiy Storchaka in :gh:`108082`.)
11661166

1167+
* Add :c:func:`PyDict_Pop` function: remove a key from a dictionary and return
1168+
the removed value. This is the similar to :meth:`dict.pop` without the
1169+
default value.
1170+
(Contributed by Victor Stinner in :gh:`111262`.)
1171+
11671172

11681173
Porting to Python 3.13
11691174
----------------------

‎Include/dictobject.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ PyAPI_FUNC(int) PyDict_DelItemString(PyObject *dp, const char *key);
6666
// - On error, raise an exception and return -1.
6767
PyAPI_FUNC(int) PyDict_GetItemRef(PyObject *mp, PyObject *key, PyObject **result);
6868
PyAPI_FUNC(int) PyDict_GetItemStringRef(PyObject *mp, const char *key, PyObject **result);
69+
70+
PyAPI_FUNC(int) PyDict_Pop(PyObject *dict, PyObject *key, PyObject **result);
6971
#endif
7072

7173
#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000

‎Include/internal/pycore_dict.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,11 @@ extern PyObject *_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObject *);
116116
extern int _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value);
117117
extern int _PyObjectDict_SetItem(PyTypeObject *tp, PyObject **dictptr, PyObject *name, PyObject *value);
118118

119-
extern PyObject *_PyDict_Pop_KnownHash(PyObject *, PyObject *, Py_hash_t, PyObject *);
119+
extern int _PyDict_Pop_KnownHash(
120+
PyDictObject *dict,
121+
PyObject *key,
122+
Py_hash_t hash,
123+
PyObject **result);
120124

121125
#define DKIX_EMPTY (-1)
122126
#define DKIX_DUMMY (-2) /* Used internally */

‎Lib/test/test_capi/test_dict.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,40 @@ def test_dict_mergefromseq2(self):
432432
# CRASHES mergefromseq2({}, NULL, 0)
433433
# CRASHES mergefromseq2(NULL, {}, 0)
434434

435+
def test_dict_pop(self):
436+
# Test PyDict_Pop()
437+
dict_pop = _testcapi.dict_pop
438+
439+
def expect_value(mydict, key, expected_value):
440+
self.assertEqual(dict_pop(mydict.copy(), key),
441+
(1, expected_value))
442+
443+
def expect_missing(mydict, key):
444+
self.assertEqual(dict_pop(mydict, key), (0, None))
445+
446+
# key present
447+
mydict = {"key": 2, "key2": 5}
448+
expect_value(mydict, "key", 2)
449+
expect_value(mydict, "key2", 5)
450+
451+
# key missing
452+
expect_missing({}, "key") # empty dict has a fast path
453+
expect_missing({"a": 1}, "key")
454+
455+
# dict error
456+
not_dict = "string"
457+
self.assertRaises(SystemError, dict_pop, not_dict, "key")
458+
459+
# key error
460+
not_hashable_key = ["list"]
461+
expect_missing({}, not_hashable_key) # don't hash key if dict is empty
462+
with self.assertRaises(TypeError):
463+
dict_pop({'key': 1}, not_hashable_key)
464+
dict_pop({}, NULL) # key is not checked if dict is empty
465+
466+
# CRASHES dict_pop(NULL, "key")
467+
# CRASHES dict_pop({"a": 1}, NULL, default)
468+
435469

436470
if __name__ == "__main__":
437471
unittest.main()

‎Lib/test/test_stable_abi_ctypes.py

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Misc/NEWS.d/next/C API/2023-11-10-10-21-38.gh-issue-111262.2utB5m.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Add :c:func:`PyDict_Pop` function: remove a key from a dictionary and return
2+
the removed value. This is the similar to :meth:`dict.pop` without the default
3+
value. Patch by Stefan Behnel and Victor Stinner.

‎Misc/stable_abi.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2481,3 +2481,5 @@
24812481
[function._Py_SetRefcnt]
24822482
added = '3.13'
24832483
abi_only = true
2484+
[function.PyDict_Pop]
2485+
added = '3.13'

‎Modules/_functoolsmodule.c

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,19 +1087,9 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
10871087
The cache dict holds one reference to the link.
10881088
We created one other reference when the link was created.
10891089
The linked list only has borrowed references. */
1090-
popresult = _PyDict_Pop_KnownHash(self->cache, link->key,
1091-
link->hash, Py_None);
1092-
if (popresult == Py_None) {
1093-
/* Getting here means that the user function call or another
1094-
thread has already removed the old key from the dictionary.
1095-
This link is now an orphan. Since we don't want to leave the
1096-
cache in an inconsistent state, we don't restore the link. */
1097-
Py_DECREF(popresult);
1098-
Py_DECREF(link);
1099-
Py_DECREF(key);
1100-
return result;
1101-
}
1102-
if (popresult == NULL) {
1090+
int res = _PyDict_Pop_KnownHash((PyDictObject*)self->cache, link->key,
1091+
link->hash, &popresult);
1092+
if (res < 0) {
11031093
/* An error arose while trying to remove the oldest key (the one
11041094
being evicted) from the cache. We restore the link to its
11051095
original position as the oldest link. Then we allow the
@@ -1110,10 +1100,22 @@ bounded_lru_cache_wrapper(lru_cache_object *self, PyObject *args, PyObject *kwds
11101100
Py_DECREF(result);
11111101
return NULL;
11121102
}
1103+
if (res == 0) {
1104+
/* Getting here means that the user function call or another
1105+
thread has already removed the old key from the dictionary.
1106+
This link is now an orphan. Since we don't want to leave the
1107+
cache in an inconsistent state, we don't restore the link. */
1108+
assert(popresult == NULL);
1109+
Py_DECREF(link);
1110+
Py_DECREF(key);
1111+
return result;
1112+
}
1113+
11131114
/* Keep a reference to the old key and old result to prevent their
11141115
ref counts from going to zero during the update. That will
11151116
prevent potentially arbitrary object clean-up code (i.e. __del__)
11161117
from running while we're still adjusting the links. */
1118+
assert(popresult != NULL);
11171119
oldkey = link->key;
11181120
oldresult = link->result;
11191121

0 commit comments

Comments
 (0)
0