From 41ca16455188db806bfc7037058e8ecff2755e6c Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 21 Jul 2023 23:10:51 +0200 Subject: [PATCH] gh-106004: Add PyDict_GetItemRef() function (#106005) * Add PyDict_GetItemRef() and PyDict_GetItemStringRef() functions. Add these functions to the stable ABI version 3.13. * Add unit tests on the PyDict C API in test_capi. --- Doc/c-api/dict.rst | 28 ++- Doc/data/stable_abi.dat | 2 + Doc/whatsnew/3.13.rst | 7 + Include/dictobject.h | 9 + Lib/test/test_stable_abi_ctypes.py | 2 + ...-06-23-02-57-15.gh-issue-106004.-OToh6.rst | 4 + Misc/stable_abi.toml | 4 + Modules/_testcapimodule.c | 191 ++++++++++++++++++ Objects/dictobject.c | 77 +++++-- PC/python3dll.c | 2 + 10 files changed, 308 insertions(+), 18 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-06-23-02-57-15.gh-issue-106004.-OToh6.rst diff --git a/Doc/c-api/dict.rst b/Doc/c-api/dict.rst index bd0c36a217e..e4c1d71a413 100644 --- a/Doc/c-api/dict.rst +++ b/Doc/c-api/dict.rst @@ -93,10 +93,26 @@ Dictionary Objects Return ``0`` on success or ``-1`` on failure. +.. c:function:: int PyDict_GetItemRef(PyObject *p, PyObject *key, PyObject **result) + + Return a new :term:`strong reference` to the object from dictionary *p* + which has a key *key*: + + * If the key is present, set *\*result* to a new :term:`strong reference` + to the value and return ``1``. + * If the key is missing, set *\*result* to ``NULL`` and return ``0``. + * On error, raise an exception and return ``-1``. + + .. versionadded:: 3.13 + + See also the :c:func:`PyObject_GetItem` function. + + .. c:function:: PyObject* PyDict_GetItem(PyObject *p, PyObject *key) - Return the object from dictionary *p* which has a key *key*. Return ``NULL`` - if the key *key* is not present, but *without* setting an exception. + Return a :term:`borrowed reference` to the object from dictionary *p* which + has a key *key*. Return ``NULL`` if the key *key* is missing *without* + setting an exception. .. note:: @@ -131,6 +147,14 @@ Dictionary Objects :c:func:`PyUnicode_FromString` *key* instead. +.. c:function:: int PyDict_GetItemStringRef(PyObject *p, const char *key, PyObject **result) + + Similar than :c:func:`PyDict_GetItemRef`, but *key* is specified as a + :c:expr:`const char*`, rather than a :c:expr:`PyObject*`. + + .. versionadded:: 3.13 + + .. c:function:: PyObject* PyDict_SetDefault(PyObject *p, PyObject *key, PyObject *defaultobj) This is the same as the Python-level :meth:`dict.setdefault`. If present, it diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index aa1edf54637..ed415a4dc64 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -111,7 +111,9 @@ function,PyDict_Copy,3.2,, function,PyDict_DelItem,3.2,, function,PyDict_DelItemString,3.2,, function,PyDict_GetItem,3.2,, +function,PyDict_GetItemRef,3.13,, function,PyDict_GetItemString,3.2,, +function,PyDict_GetItemStringRef,3.13,, function,PyDict_GetItemWithError,3.2,, function,PyDict_Items,3.2,, function,PyDict_Keys,3.2,, diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index 1d5e34dac28..0203e3d1873 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -786,6 +786,13 @@ New Features always steals a reference to the value. (Contributed by Serhiy Storchaka in :gh:`86493`.) +* Added :c:func:`PyDict_GetItemRef` and :c:func:`PyDict_GetItemStringRef` + functions: similar to :c:func:`PyDict_GetItemWithError` but returning a + :term:`strong reference` instead of a :term:`borrowed reference`. Moreover, + these functions return -1 on error and so checking ``PyErr_Occurred()`` is + not needed. + (Contributed by Victor Stinner in :gh:`106004`.) + Porting to Python 3.13 ---------------------- diff --git a/Include/dictobject.h b/Include/dictobject.h index e7fcb44d0cf..26434e49f91 100644 --- a/Include/dictobject.h +++ b/Include/dictobject.h @@ -57,6 +57,15 @@ PyAPI_FUNC(int) PyDict_MergeFromSeq2(PyObject *d, PyAPI_FUNC(PyObject *) PyDict_GetItemString(PyObject *dp, const char *key); PyAPI_FUNC(int) PyDict_SetItemString(PyObject *dp, const char *key, PyObject *item); PyAPI_FUNC(int) PyDict_DelItemString(PyObject *dp, const char *key); + +// Return the object from dictionary *op* which has a key *key*. +// - If the key is present, set *result to a new strong reference to the value +// and return 1. +// - If the key is missing, set *result to NULL and return 0 . +// - 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); + #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 PyAPI_FUNC(PyObject *) PyObject_GenericGetDict(PyObject *, void *); #endif diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 4e74bb374c9..566d36a3f5b 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -144,7 +144,9 @@ SYMBOL_NAMES = ( "PyDict_DelItem", "PyDict_DelItemString", "PyDict_GetItem", + "PyDict_GetItemRef", "PyDict_GetItemString", + "PyDict_GetItemStringRef", "PyDict_GetItemWithError", "PyDict_Items", "PyDict_Keys", diff --git a/Misc/NEWS.d/next/C API/2023-06-23-02-57-15.gh-issue-106004.-OToh6.rst b/Misc/NEWS.d/next/C API/2023-06-23-02-57-15.gh-issue-106004.-OToh6.rst new file mode 100644 index 00000000000..c7a006b2bc0 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-06-23-02-57-15.gh-issue-106004.-OToh6.rst @@ -0,0 +1,4 @@ +Adds :c:func:`PyDict_GetItemRef` and :c:func:`PyDict_GetItemStringRef` +functions: similar to :c:func:`PyDict_GetItemWithError` but returning a +:term:`strong reference` instead of a :term:`borrowed reference`. Patch by +Victor Stinner. diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index dd2c9910b83..16d5c1a07ae 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2446,3 +2446,7 @@ added = '3.13' [function.PyModule_Add] added = '3.13' +[function.PyDict_GetItemRef] + added = '3.13' +[function.PyDict_GetItemStringRef] + added = '3.13' diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index dd2c9c72e53..065a7fb733d 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3464,6 +3464,196 @@ test_weakref_capi(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) } +static PyObject * +test_dict_capi(PyObject *Py_UNUSED(module), PyObject *Py_UNUSED(args)) +{ + assert(!PyErr_Occurred()); + + PyObject *dict= NULL, *key = NULL, *missing_key = NULL, *value = NULL; + PyObject *invalid_key = NULL; + int res; + + // test PyDict_New() + dict = PyDict_New(); + if (dict == NULL) { + goto error; + } + + key = PyUnicode_FromString("key"); + if (key == NULL) { + goto error; + } + + missing_key = PyUnicode_FromString("missing_key"); + if (missing_key == NULL) { + goto error; + } + + value = PyUnicode_FromString("value"); + if (value == NULL) { + goto error; + } + + // test PyDict_SetItem() + Py_ssize_t key_refcnt = Py_REFCNT(key); + Py_ssize_t value_refcnt = Py_REFCNT(value); + res = PyDict_SetItem(dict, key, value); + if (res < 0) { + goto error; + } + assert(res == 0); + assert(Py_REFCNT(key) == (key_refcnt + 1)); + assert(Py_REFCNT(value) == (value_refcnt + 1)); + + // test PyDict_SetItemString() + res = PyDict_SetItemString(dict, "key", value); + if (res < 0) { + goto error; + } + assert(res == 0); + assert(Py_REFCNT(key) == (key_refcnt + 1)); + assert(Py_REFCNT(value) == (value_refcnt + 1)); + + // test PyDict_Size() + assert(PyDict_Size(dict) == 1); + + // test PyDict_Contains(), key is present + assert(PyDict_Contains(dict, key) == 1); + + // test PyDict_GetItem(), key is present + assert(PyDict_GetItem(dict, key) == value); + + // test PyDict_GetItemString(), key is present + assert(PyDict_GetItemString(dict, "key") == value); + + // test PyDict_GetItemWithError(), key is present + assert(PyDict_GetItemWithError(dict, key) == value); + assert(!PyErr_Occurred()); + + // test PyDict_GetItemRef(), key is present + PyObject *get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemRef(dict, key, &get_value) == 1); + assert(get_value == value); + Py_DECREF(get_value); + + // test PyDict_GetItemStringRef(), key is present + get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemStringRef(dict, "key", &get_value) == 1); + assert(get_value == value); + Py_DECREF(get_value); + + // test PyDict_Contains(), missing key + assert(PyDict_Contains(dict, missing_key) == 0); + + // test PyDict_GetItem(), missing key + assert(PyDict_GetItem(dict, missing_key) == NULL); + assert(!PyErr_Occurred()); + + // test PyDict_GetItemString(), missing key + assert(PyDict_GetItemString(dict, "missing_key") == NULL); + assert(!PyErr_Occurred()); + + // test PyDict_GetItemWithError(), missing key + assert(PyDict_GetItem(dict, missing_key) == NULL); + assert(!PyErr_Occurred()); + + // test PyDict_GetItemRef(), missing key + get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemRef(dict, missing_key, &get_value) == 0); + assert(!PyErr_Occurred()); + assert(get_value == NULL); + + // test PyDict_GetItemStringRef(), missing key + get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemStringRef(dict, "missing_key", &get_value) == 0); + assert(!PyErr_Occurred()); + assert(get_value == NULL); + + // test PyDict_GetItem(), invalid dict + PyObject *invalid_dict = key; // borrowed reference + assert(PyDict_GetItem(invalid_dict, key) == NULL); + assert(!PyErr_Occurred()); + + // test PyDict_GetItemWithError(), invalid dict + assert(PyDict_GetItemWithError(invalid_dict, key) == NULL); + assert(PyErr_ExceptionMatches(PyExc_SystemError)); + PyErr_Clear(); + + // test PyDict_GetItemRef(), invalid dict + get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemRef(invalid_dict, key, &get_value) == -1); + assert(PyErr_ExceptionMatches(PyExc_SystemError)); + PyErr_Clear(); + assert(get_value == NULL); + + // test PyDict_GetItemStringRef(), invalid dict + get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemStringRef(invalid_dict, "key", &get_value) == -1); + assert(PyErr_ExceptionMatches(PyExc_SystemError)); + PyErr_Clear(); + assert(get_value == NULL); + + invalid_key = PyList_New(0); + if (invalid_key == NULL) { + goto error; + } + + // test PyDict_Contains(), invalid key + assert(PyDict_Contains(dict, invalid_key) == -1); + assert(PyErr_ExceptionMatches(PyExc_TypeError)); + PyErr_Clear(); + + // test PyDict_GetItem(), invalid key + assert(PyDict_GetItem(dict, invalid_key) == NULL); + assert(!PyErr_Occurred()); + + // test PyDict_GetItemWithError(), invalid key + assert(PyDict_GetItemWithError(dict, invalid_key) == NULL); + assert(PyErr_ExceptionMatches(PyExc_TypeError)); + PyErr_Clear(); + + // test PyDict_GetItemRef(), invalid key + get_value = Py_Ellipsis; // marker value + assert(PyDict_GetItemRef(dict, invalid_key, &get_value) == -1); + assert(PyErr_ExceptionMatches(PyExc_TypeError)); + PyErr_Clear(); + assert(get_value == NULL); + + // test PyDict_DelItem(), key is present + assert(PyDict_DelItem(dict, key) == 0); + assert(PyDict_Size(dict) == 0); + + // test PyDict_DelItem(), missing key + assert(PyDict_DelItem(dict, missing_key) == -1); + assert(PyErr_ExceptionMatches(PyExc_KeyError)); + PyErr_Clear(); + + // test PyDict_DelItem(), invalid key + assert(PyDict_DelItem(dict, invalid_key) == -1); + assert(PyErr_ExceptionMatches(PyExc_TypeError)); + PyErr_Clear(); + + // test PyDict_Clear() + PyDict_Clear(dict); + + Py_DECREF(dict); + Py_DECREF(key); + Py_DECREF(missing_key); + Py_DECREF(value); + Py_DECREF(invalid_key); + + Py_RETURN_NONE; + +error: + Py_XDECREF(dict); + Py_XDECREF(key); + Py_XDECREF(missing_key); + Py_XDECREF(value); + Py_XDECREF(invalid_key); + return NULL; +} + + static PyMethodDef TestMethods[] = { {"set_errno", set_errno, METH_VARARGS}, {"test_config", test_config, METH_NOARGS}, @@ -3609,6 +3799,7 @@ static PyMethodDef TestMethods[] = { {"function_set_kw_defaults", function_set_kw_defaults, METH_VARARGS, NULL}, {"check_pyimport_addmodule", check_pyimport_addmodule, METH_VARARGS}, {"test_weakref_capi", test_weakref_capi, METH_NOARGS}, + {"test_dict_capi", test_dict_capi, METH_NOARGS}, {NULL, NULL} /* sentinel */ }; diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 013c2188403..3f9297c8a41 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -1696,9 +1696,8 @@ PyDict_GetItem(PyObject *op, PyObject *key) /* Ignore any exception raised by the lookup */ _PyErr_SetRaisedException(tstate, exc); - assert(ix >= 0 || value == NULL); - return value; + return value; // borrowed reference } Py_ssize_t @@ -1737,9 +1736,46 @@ _PyDict_GetItem_KnownHash(PyObject *op, PyObject *key, Py_hash_t hash) ix = _Py_dict_lookup(mp, key, hash, &value); assert(ix >= 0 || value == NULL); - return value; + return value; // borrowed reference } + +int +PyDict_GetItemRef(PyObject *op, PyObject *key, PyObject **result) +{ + if (!PyDict_Check(op)) { + PyErr_BadInternalCall(); + *result = NULL; + return -1; + } + PyDictObject*mp = (PyDictObject *)op; + + Py_hash_t hash; + if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) + { + hash = PyObject_Hash(key); + if (hash == -1) { + *result = NULL; + return -1; + } + } + + PyObject *value; + Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &value); + assert(ix >= 0 || value == NULL); + if (ix == DKIX_ERROR) { + *result = NULL; + return -1; + } + if (value == NULL) { + *result = NULL; + return 0; // missing key + } + *result = Py_NewRef(value); + return 1; // key is present +} + + /* Variant of PyDict_GetItem() that doesn't suppress exceptions. This returns NULL *with* an exception set if an exception occurred. It returns NULL *without* an exception set if the key wasn't present. @@ -1766,7 +1802,7 @@ PyDict_GetItemWithError(PyObject *op, PyObject *key) ix = _Py_dict_lookup(mp, key, hash, &value); assert(ix >= 0 || value == NULL); - return value; + return value; // borrowed reference } PyObject * @@ -1777,7 +1813,7 @@ _PyDict_GetItemWithError(PyObject *dp, PyObject *kv) if (hash == -1) { return NULL; } - return _PyDict_GetItem_KnownHash(dp, kv, hash); + return _PyDict_GetItem_KnownHash(dp, kv, hash); // borrowed reference } PyObject * @@ -1789,7 +1825,7 @@ _PyDict_GetItemIdWithError(PyObject *dp, _Py_Identifier *key) return NULL; Py_hash_t hash = unicode_get_hash(kv); assert (hash != -1); /* interned strings have their hash value initialised */ - return _PyDict_GetItem_KnownHash(dp, kv, hash); + return _PyDict_GetItem_KnownHash(dp, kv, hash); // borrowed reference } PyObject * @@ -1802,7 +1838,7 @@ _PyDict_GetItemStringWithError(PyObject *v, const char *key) } rv = PyDict_GetItemWithError(v, kv); Py_DECREF(kv); - return rv; + return rv; // borrowed reference } /* Fast version of global value lookup (LOAD_GLOBAL). @@ -3894,7 +3930,20 @@ PyDict_GetItemString(PyObject *v, const char *key) } rv = PyDict_GetItem(v, kv); Py_DECREF(kv); - return rv; + return rv; // borrowed reference +} + +int +PyDict_GetItemStringRef(PyObject *v, const char *key, PyObject **result) +{ + PyObject *key_obj = PyUnicode_FromString(key); + if (key_obj == NULL) { + *result = NULL; + return -1; + } + int res = PyDict_GetItemRef(v, key_obj, result); + Py_DECREF(key_obj); + return res; } int @@ -5146,15 +5195,11 @@ dictitems_contains(_PyDictViewObject *dv, PyObject *obj) return 0; key = PyTuple_GET_ITEM(obj, 0); value = PyTuple_GET_ITEM(obj, 1); - found = PyDict_GetItemWithError((PyObject *)dv->dv_dict, key); - if (found == NULL) { - if (PyErr_Occurred()) - return -1; - return 0; + result = PyDict_GetItemRef((PyObject *)dv->dv_dict, key, &found); + if (result == 1) { + result = PyObject_RichCompareBool(found, value, Py_EQ); + Py_DECREF(found); } - Py_INCREF(found); - result = PyObject_RichCompareBool(found, value, Py_EQ); - Py_DECREF(found); return result; } diff --git a/PC/python3dll.c b/PC/python3dll.c index 0b54c5a7072..64dfbba3e42 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -172,7 +172,9 @@ EXPORT_FUNC(PyDict_Copy) EXPORT_FUNC(PyDict_DelItem) EXPORT_FUNC(PyDict_DelItemString) EXPORT_FUNC(PyDict_GetItem) +EXPORT_FUNC(PyDict_GetItemRef) EXPORT_FUNC(PyDict_GetItemString) +EXPORT_FUNC(PyDict_GetItemStringRef) EXPORT_FUNC(PyDict_GetItemWithError) EXPORT_FUNC(PyDict_Items) EXPORT_FUNC(PyDict_Keys)