8000 gh-111262: Add PyDict_Pop() function (#112028) · python/cpython@4f04172 · GitHub
[go: up one dir, main page]
Skip to content

Commit 4f04172

Browse files
vstinnerscoderpitrou
authored
gh-111262: Add PyDict_Pop() function (#112028)
_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 f44d6ff commit 4f04172

File tree

15 files changed

+335
-73
lines changed

15 files changed

+335
-73
lines changed

‎Doc/c-api/dict.rst

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,33 @@ 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+
Do not raise :exc:`KeyError` if the key missing.
181+
182+
- If the key is present, set *\*result* to a new reference to the removed
183+
value if *result* is not ``NULL``, and return ``1``.
184+
- If the key is missing, set *\*result* to ``NULL`` if *result* is not
185+
``NULL``, and return ``0``.
186+
- On error, raise an exception and return ``-1``.
187+
188+
This is similar to :meth:`dict.pop`, but without the default value and
189+
not raising :exc:`KeyError` if the key missing.
190+
191+
.. versionadded:: 3.13
192+
193+
194+
.. c:function:: int PyDict_PopString(PyObject *p, const char *key, PyObject **result)
195+
196+
Similar to :c:func:`PyDict_Pop`, but *key* is specified as a
197+
:c:expr:`const char*` UTF-8 encoded bytes string, rather than a
198+
:c:expr:`PyObject*`.
199+
200+
.. versionadded:: 3.13
201+
202+
176203
.. c:function:: PyObject* PyDict_Items(PyObject *p)
177204
178205
Return a :c:type:`PyListObject` containing all the items from the dictionary.

‎Doc/whatsnew/3.13.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,12 @@ New Features
11751175
Python ``list.extend()`` and ``list.clear()`` methods.
11761176
(Contributed by Victor Stinner in :gh:`111138`.)
11771177

1178+
* Add :c:func:`PyDict_Pop` and :c:func:`PyDict_PopString` functions: remove a
1179+
key from a dictionary and optionally return the removed value. This is
1180+
similar to :meth:`dict.pop`, but without the default value and not raising
1181+
:exc:`KeyError` if the key missing.
1182+
(Contributed by Stefan Behnel and Victor Stinner in :gh:`111262`.)
1183+
11781184

11791185
Porting to Python 3.13
11801186
----------------------

‎Include/cpython/dictobject.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ static inline Py_ssize_t PyDict_GET_SIZE(PyObject *op) {
4646

4747
PyAPI_FUNC(int) PyDict_ContainsString(PyObject *mp, const char *key);
4848

49+
PyAPI_FUNC(int) PyDict_Pop(PyObject *dict, PyObject *key, PyObject **result);
50+
PyAPI_FUNC(int) PyDict_PopString(PyObject *dict, const char *key, PyObject **result);
4951
PyAPI_FUNC(PyObject *) _PyDict_Pop(PyObject *dict, PyObject *key, PyObject *default_value);
5052

5153
/* Dictionary watchers */

‎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: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,93 @@ 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+
dict_pop_null = _testcapi.dict_pop_null
439+
440+
# key present, get removed value
441+
mydict = {"key": "value", "key2": "value2"}
442+
self.assertEqual(dict_pop(mydict, "key"), (1, "value"))
443+
self.assertEqual(mydict, {"key2": "value2"})
444+
self.assertEqual(dict_pop(mydict, "key2"), (1, "value2"))
445+
self.assertEqual(mydict, {})
446+
447+
# key present, ignore removed value
448+
mydict = {"key": "value", "key2": "value2"}
449+
self.assertEqual(dict_pop_null(mydict, "key"), 1)
450+
self.assertEqual(mydict, {"key2": "value2"})
451+
self.assertEqual(dict_pop_null(mydict, "key2"), 1)
452+
self.assertEqual(mydict, {})
453+
454+
# key missing, expect removed value; empty dict has a fast path
455+
self.assertEqual(dict_pop({}, "key"), (0, NULL))
456+
self.assertEqual(dict_pop({"a": 1}, "key"), (0, NULL))
457+
458+
# key missing, ignored removed value; empty dict has a fast path
459+
self.assertEqual(dict_pop_null({}, "key"), 0)
460+
self.assertEqual(dict_pop_null({"a": 1}, "key"), 0)
461+
462+
# dict error
463+
not_dict = UserDict({1: 2})
464+
self.assertRaises(SystemError, dict_pop, not_dict, "key")
465+
self.assertRaises(SystemError, dict_pop_null, not_dict, "key")
466+
467+
# key error; don't hash key if dict is empty
468+
not_hashable_key = ["list"]
469+
self.assertEqual(dict_pop({}, not_hashable_key), (0, NULL))
470+
with self.assertRaises(TypeError):
471+
dict_pop({'key': 1}, not_hashable_key)
472+
dict_pop({}, NULL) # key is not checked if dict is empty
473+
474+
# CRASHES dict_pop(NULL, "key")
475+
# CRASHES dict_pop({"a": 1}, NULL)
476+
477+
def test_dict_popstring(self):
478+
# Test PyDict_PopString()
479+
dict_popstring = _testcapi.dict_popstring
480+
dict_popstring_null = _testcapi.dict_popstring_null
481+
482+
# key present, get removed value
483+
mydict = {"key": "value", "key2": "value2"}
484+
self.assertEqual(dict_popstring(mydict, "key"), (1, "value"))
485+
self.assertEqual(mydict, {"key2": "value2"})
486+
self.assertEqual(dict_popstring(mydict, "key2"), (1, "value2"))
487+
self.assertEqual(mydict, {})
488+
489+
# key present, ignore removed value
490+
mydict = {"key": "value", "key2": "value2"}
491+
self.assertEqual(dict_popstring_null(mydict, "key"), 1)
492+
self.assertEqual(mydict, {"key2": "value2"})
493+
self.assertEqual(dict_popstring_null(mydict, "key2"), 1)
494+
self.assertEqual(mydict, {})
495+
496+
# key missing; empty dict has a fast path
497+
self.assertEqual(dict_popstring({}, "key"), (0, NULL))
498+
self.assertEqual(dict_popstring_null({}, "key"), 0)
499+
self.assertEqual(dict_popstring({"a": 1}, "key"), (0, NULL))
500+
self.assertEqual(dict_popstring_null({"a": 1}, "key"), 0)
501+
502+
# non-ASCII key
503+
non_ascii = '\U0001f40d'
504+
dct = {'\U0001f40d': 123}
505+
self.assertEqual(dict_popstring(dct, '\U0001f40d'.encode()), (1, 123))
506+
dct = {'\U0001f40d': 123}
507+
self.assertEqual(dict_popstring_null(dct, '\U0001f40d'.encode()), 1)
508+
509+
# dict error
510+
not_dict = UserDict({1: 2})
511+
self.assertRaises(SystemError, dict_popstring, not_dict, "key")
512+
self.assertRaises(SystemError, dict_popstring_null, not_dict, "key")
513+
514+
# key error
515+
self.assertRaises(UnicodeDecodeError, dict_popstring, {1: 2}, INVALID_UTF8)
516+
self.assertRaises(UnicodeDecodeError, dict_popstring_null, {1: 2}, INVALID_UTF8)
517+
518+
# CRASHES dict_popstring(NULL, "key")
519+
# CRASHES dict_popstring({}, NULL)
520+
# CRASHES dict_popstring({"a": 1}, NULL)
521+
435522

436523
if __name__ == "__main__":
437524
unittest.main()

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

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

‎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

‎Modules/_testcapi/dict.c

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,88 @@ dict_mergefromseq2(PyObject *self, PyObject *args)
331331
}
332332

333333

334+
static PyObject *
335+
dict_pop(PyObject *self, PyObject *args)
336+
{
337+
// Test PyDict_Pop(dict, key, &value)
338+
PyObject *dict, *key;
339+
if (!PyArg_ParseTuple(args, "OO", &dict, &key)) {
340+
return NULL;
341+
}
342+
NULLABLE(dict);
343+
NULLABLE(key);
344+
PyObject *result = UNINITIALIZED_PTR;
345+
int res = PyDict_Pop(dict, key, &result);
346+
if (res < 0) {
< 10000 code>347+
assert(result == NULL);
348+
return NULL;
349+
}
350+
if (res == 0) {
351+
assert(result == NULL);
352+
result = Py_NewRef(Py_None);
353+
}
354+
else {
355+
assert(result != NULL);
356+
}
357+
return Py_BuildValue("iN", res, result);
358+
}
359+
360+
361+
static PyObject *
362+
dict_pop_null(PyObject *self, PyObject *args)
363+
{
364+
// Test PyDict_Pop(dict, key, NULL)
365+
PyObject *dict, *key;
366+
if (!PyArg_ParseTuple(args, "OO", &dict, &key)) {
367+
return NULL;
368+
}
369+
NULLABLE(dict);
370+
NULLABLE(key);
371+
RETURN_INT(PyDict_Pop(dict, key, NULL));
372+
}
373+
374+
375+
static PyObject *
376+
dict_popstring(PyObject *self, PyObject *args)
377+
{
378+
PyObject *dict;
379+
const char *key;
380+
Py_ssize_t key_size;
381+
if (!PyArg_ParseTuple(args, "Oz#", &dict, &key, &key_size)) {
382+
return NULL;
383+
}
384+
NULLABLE(dict);
385+
PyObject *result = UNINITIALIZED_PTR;
386+
int res = PyDict_PopString(dict, key, &result);
387+
if (res < 0) {
388+
assert(result == NULL);
389+
return NULL;
390+
}
391+
if (res == 0) {
392+
assert(result == NULL);
393+
result = Py_NewRef(Py_None);
394+
}
395+
else {
396+
assert(result != NULL);
397+
}
398+
return Py_BuildValue("iN", res, result);
399+
}
400+
401+
402+
static PyObject *
403+
dict_popstring_null(PyObject *self, PyObject *args)
404+
{
405+
PyObject *dict;
406+
const char *key;
407+
Py_ssize_t key_size;
408+
if (!PyArg_ParseTuple(args, "Oz#", &dict, &key, &key_size)) {
409+
return NULL;
410+
}
411+
NULLABLE(dict);
412+
RETURN_INT(PyDict_PopString(dict, key, NULL));
413+
}
414+
415+
334416
static PyMethodDef test_methods[] = {
335417
{"dict_check", dict_check, METH_O},
336418
{"dict_checkexact", dict_checkexact, METH_O},
@@ -358,7 +440,10 @@ static PyMethodDef test_methods[] = {
358440
{"dict_merge", dict_merge, METH_VARARGS},
359441
{"dict_update", dict_update, METH_VARARGS},
360442
{"dict_mergefromseq2", dict_mergefromseq2, METH_VARARGS},
361-
443+
{"dict_pop", dict_pop, METH_VARARGS},
444+
{"dict_pop_null", dict_pop_null, METH_VARARGS},
445+
{"dict_popstring", dict_popstring, METH_VARARGS},
446+
{"dict_popstring_null", dict_popstring_null, METH_VARARGS},
362447
{NULL},
363448
};
364449

‎Modules/_threadmodule.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -967,11 +967,8 @@ local_clear(localobject *self)
967967
HEAD_UNLOCK(runtime);
968968
while (tstate) {
969969
if (tstate->dict) {
970-
PyObject *v = _PyDict_Pop(tstate->dict, self->key, Py_None);
971-
if (v != NULL) {
972-
Py_DECREF(v);
973-
}
974-
else {
970+
if (PyDict_Pop(tstate->dict, self->key, NULL) < 0) {
971+
// Silently ignore error
975972
PyErr_Clear();
976973
}
977974
}

‎Modules/socketmodule.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -392,16 +392,10 @@ remove_unusable_flags(PyObject *m)
392392
break;
393393
}
394394
else {
395-
PyObject *flag_name = PyUnicode_FromString(win_runtime_flags[i].flag_name);
396-
if (flag_name == NULL) {
395+
if (PyDict_PopString(dict, win_runtime_flags[i].flag_name,
396+
NULL) < 0) {
397397
return -1;
398398
}
399-
PyObject *v = _PyDict_Pop(dict, flag_name, Py_None);
400-
Py_DECREF(flag_name);
401-
if (v == NULL) {
402-
return -1;
403-
}
404-
Py_DECREF(v);
405399
}
406400
}
407401
return 0;

0 commit comments

Comments
 (0)
0