From 5131071ced7d7f695274f7717b2580f9dc2a1221 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Fri, 23 Jun 2023 01:58:50 +0200 Subject: [PATCH] gh-106004: Add PyDict_GetItemRef() function * Add PyDict_GetItemRef() and PyDict_GetItemStringRef() functions. * Add unit tests on the PyDict C API in test_capi. --- Doc/c-api/dict.rst | 28 ++- Doc/whatsnew/3.13.rst | 7 + Include/dictobject.h | 2 + ...-06-23-02-57-15.gh-issue-106004.-OToh6.rst | 4 + Modules/_testcapimodule.c | 190 ++++++++++++++++++ Objects/dictobject.c | 68 +++++-- 6 files changed, 285 insertions(+), 14 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 0ca8ad624b2034a..f2a505b44667ca8 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 **value) + + Return a new :term:`strong reference` to the object from dictionary *p* + which has a key *key*: + + * If the key is present, set *\*pvalue* to a new :term:`strong reference` + to the value and return ``0``. + * If the key is missing, set *\*pvalue* 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 that exceptions which occur while calling :meth:`__hash__` and :meth:`__eq__` methods will get suppressed. @@ -126,6 +142,14 @@ Dictionary Objects To get error reporting use :c:func:`PyDict_GetItemWithError()` instead. +.. c:function:: int PyDict_GetItemStringRef(PyObject *p, const char *key, PyObject **pvalue) + + 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/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index d1f13a50335b5bf..c35c80257534c46 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -440,6 +440,13 @@ New Features ``NULL`` if the referent is no longer live. (Contributed by Victor Stinner in :gh:`105927`.) +* Add :c:func:`PyDict_GetItemRef` and :c:func:`PyDict_GetItemStringRef` + functions: similar than :c:func:`PyDict_GetItemWithError` but return 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 e7fcb44d0cf9a98..f8c02e083023449 100644 --- a/Include/dictobject.h +++ b/Include/dictobject.h @@ -21,6 +21,7 @@ PyAPI_DATA(PyTypeObject) PyDict_Type; PyAPI_FUNC(PyObject *) PyDict_New(void); PyAPI_FUNC(PyObject *) PyDict_GetItem(PyObject *mp, PyObject *key); PyAPI_FUNC(PyObject *) PyDict_GetItemWithError(PyObject *mp, PyObject *key); +PyAPI_FUNC(int) PyDict_GetItemRef(PyObject *mp, PyObject *key, PyObject **pvalue); PyAPI_FUNC(int) PyDict_SetItem(PyObject *mp, PyObject *key, PyObject *item); PyAPI_FUNC(int) PyDict_DelItem(PyObject *mp, PyObject *key); PyAPI_FUNC(void) PyDict_Clear(PyObject *mp); @@ -55,6 +56,7 @@ PyAPI_FUNC(int) PyDict_MergeFromSeq2(PyObject *d, int override); PyAPI_FUNC(PyObject *) PyDict_GetItemString(PyObject *dp, const char *key); +PyAPI_FUNC(int) PyDict_GetItemStringRef(PyObject *mp, const char *key, PyObject **pvalue); PyAPI_FUNC(int) PyDict_SetItemString(PyObject *dp, const char *key, PyObject *item); PyAPI_FUNC(int) PyDict_DelItemString(PyObject *dp, const char *key); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030A0000 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 000000000000000..342a71efe5fad0c --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-06-23-02-57-15.gh-issue-106004.-OToh6.rst @@ -0,0 +1,4 @@ +Add :c:func:`PyDict_GetItemRef` and :c:func:`PyDict_GetItemStringRef` +functions: similar than :c:func:`PyDict_GetItemWithError` but return a +:term:`strong reference` instead of a :term:`borrowed reference`. Patch by +Victor Stinner. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index d847539f6608dd2..4942b12235aa72b 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -3466,6 +3466,195 @@ 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; + + // 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); + if (PyDict_SetItem(dict, key, value) < 0) { + goto error; + } + assert(Py_REFCNT(key) == (key_refcnt + 1)); + assert(Py_REFCNT(value) == (value_refcnt + 1)); + + // test PyDict_SetItemString() + if (PyDict_SetItemString(dict, "key", value) < 0) { + goto error; + } + 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 + if (PyDict_GetItemRef(dict, key, &get_value) < 0) { + goto error; + } + assert(get_value == value); + Py_DECREF(get_value); + + // test PyDict_GetItemStringRef(), key is present + get_value = Py_Ellipsis; // marker value + if (PyDict_GetItemStringRef(dict, "key", &get_value) < 0) { + goto error; + } + 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}, @@ -3611,6 +3800,7 @@ static PyMethodDef TestMethods[] = { {"test_atexit", test_atexit, METH_NOARGS}, {"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 194081db2f290a4..35b2d344a702e3e 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,7 +1736,40 @@ _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 +} + +// Return the object from dictionary *op* which has a key *key*. +// - If the key is present, set *pvalue to a new strong reference to the value +// and return 0. +// - If the key is missing, set *pvalue to NULL and return 0 . +// - On error, raise an exception and return -1. +int +PyDict_GetItemRef(PyObject *op, PyObject *key, PyObject **pvalue) +{ + Py_ssize_t ix; (void)ix; + Py_hash_t hash; + PyDictObject*mp = (PyDictObject *)op; + PyObject *value; + + if (!PyDict_Check(op)) { + PyErr_BadInternalCall(); + *pvalue = NULL; + return -1; + } + if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) + { + hash = PyObject_Hash(key); + if (hash == -1) { + *pvalue = NULL; + return -1; + } + } + + ix = _Py_dict_lookup(mp, key, hash, &value); + assert(ix >= 0 || value == NULL); + *pvalue = Py_XNewRef(value); + return 0; } /* Variant of PyDict_GetItem() that doesn't suppress exceptions. @@ -1766,7 +1798,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 +1809,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 +1821,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 +1834,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 +3926,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 **pvalue) +{ + PyObject *key_obj = PyUnicode_FromString(key); + if (key_obj == NULL) { + *pvalue = NULL; + return -1; + } + int res = PyDict_GetItemRef(v, key_obj, pvalue); + Py_DECREF(key_obj); + return res; } int @@ -5146,13 +5191,12 @@ 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 (PyDict_GetItemRef((PyObject *)dv->dv_dict, key, &found) < 0) { + return -1; + } if (found == NULL) { - if (PyErr_Occurred()) - return -1; return 0; } - Py_INCREF(found); result = PyObject_RichCompareBool(found, value, Py_EQ); Py_DECREF(found); return result;