From ae6e78904ce014135837ed134d6451127d443e74 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 1 Sep 2023 09:12:38 +0300 Subject: [PATCH 1/2] gh-108511: Add C API functions which do not silently ignore errors Add the following functions: * PyObject_HasAttrWithError() * PyObject_HasAttrStringWithError() * PyMapping_HasKeyWithError() * PyMapping_HasKeyStringWithError() --- Doc/c-api/mapping.rst | 25 ++++++- Doc/c-api/object.rst | 25 ++++++- Doc/whatsnew/3.13.rst | 12 ++++ Include/abstract.h | 31 +++++++++ Include/object.h | 4 ++ Lib/test/test_capi/test_abstract.py | 66 +++++++++++++++++++ ...-09-01-16-28-09.gh-issue-108511.gg-QDG.rst | 4 ++ Modules/_ctypes/stgdict.c | 6 +- Modules/_elementtree.c | 3 +- Modules/_io/iobase.c | 6 +- Modules/_io/textio.c | 3 +- Modules/_pickle.c | 7 +- Modules/_testcapi/abstract.c | 54 +++++++++++++++ Modules/_xxinterpchannelsmodule.c | 2 +- Modules/_xxsubinterpretersmodule.c | 2 +- Objects/abstract.c | 18 +++++ Objects/dictobject.c | 7 +- Objects/genericaliasobject.c | 35 ++++------ Objects/object.c | 47 ++++++------- Objects/typeobject.c | 8 +-- Objects/unionobject.c | 27 +++----- Python/errors.c | 16 ++--- Python/import.c | 7 +- Python/suggestions.c | 6 +- 24 files changed, 310 insertions(+), 111 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2023-09-01-16-28-09.gh-issue-108511.gg-QDG.rst diff --git a/Doc/c-api/mapping.rst b/Doc/c-api/mapping.rst index 5b909ef8f5e2ce..1f55c0aa955c75 100644 --- a/Doc/c-api/mapping.rst +++ b/Doc/c-api/mapping.rst @@ -76,6 +76,24 @@ See also :c:func:`PyObject_GetItem`, :c:func:`PyObject_SetItem` and rather than a :c:expr:`PyObject*`. +.. c:function:: int PyMapping_HasKeyWithError(PyObject *o, PyObject *key) + + Return ``1`` if the mapping object has the key *key* and ``0`` otherwise. + This is equivalent to the Python expression ``key in o``. + On failure, return ``-1``. + + .. versionadded:: 3.13 + + +.. c:function:: int PyMapping_HasKeyStringWithError(PyObject *o, const char *key) + + This is the same as :c:func:`PyMapping_HasKeyWithError`, but *key* is + specified as a :c:expr:`const char*` UTF-8 encoded bytes string, + rather than a :c:expr:`PyObject*`. + + .. versionadded:: 3.13 + + .. c:function:: int PyMapping_HasKey(PyObject *o, PyObject *key) Return ``1`` if the mapping object has the key *key* and ``0`` otherwise. @@ -86,8 +104,8 @@ See also :c:func:`PyObject_GetItem`, :c:func:`PyObject_SetItem` and Exceptions which occur when this calls :meth:`~object.__getitem__` method are silently ignored. - For proper error handling, use :c:func:`PyMapping_GetOptionalItem` or - :c:func:`PyObject_GetItem()` instead. + For proper error handling, use :c:func:`PyMapping_HasKeyWithError`, + :c:func:`PyMapping_GetOptionalItem` or :c:func:`PyObject_GetItem()` instead. .. c:function:: int PyMapping_HasKeyString(PyObject *o, const char *key) @@ -101,7 +119,8 @@ See also :c:func:`PyObject_GetItem`, :c:func:`PyObject_SetItem` and Exceptions that occur when this calls :meth:`~object.__getitem__` method or while creating the temporary :class:`str` object are silently ignored. - For proper error handling, use :c:func:`PyMapping_GetOptionalItemString` or + For proper error handling, use :c:func:`PyMapping_HasKeyStringWithError`, + :c:func:`PyMapping_GetOptionalItemString` or :c:func:`PyMapping_GetItemString` instead. diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 2572c087738fe3..bf55b5788efa47 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -27,6 +27,24 @@ Object Protocol instead of the :func:`repr`. +.. c:function:: int PyObject_HasAttrWithError(PyObject *o, const char *attr_name) + + Returns ``1`` if *o* has the attribute *attr_name*, and ``0`` otherwise. + This is equivalent to the Python expression ``hasattr(o, attr_name)``. + On failure, return ``-1``. + + .. versionadded:: 3.13 + + +.. c:function:: int PyObject_HasAttrStringWithError(PyObject *o, const char *attr_name) + + This is the same as :c:func:`PyObject_HasAttrWithError`, but *attr_name* is + specified as a :c:expr:`const char*` UTF-8 encoded bytes string, + rather than a :c:expr:`PyObject*`. + + .. versionadded:: 3.13 + + .. c:function:: int PyObject_HasAttr(PyObject *o, PyObject *attr_name) Returns ``1`` if *o* has the attribute *attr_name*, and ``0`` otherwise. This @@ -37,8 +55,8 @@ Object Protocol Exceptions that occur when this calls :meth:`~object.__getattr__` and :meth:`~object.__getattribute__` methods are silently ignored. - For proper error handling, use :c:func:`PyObject_GetOptionalAttr` or - :c:func:`PyObject_GetAttr` instead. + For proper error handling, use :c:func:`PyObject_HasAttrWithError`, + :c:func:`PyObject_GetOptionalAttr` or :c:func:`PyObject_GetAttr` instead. .. c:function:: int PyObject_HasAttrString(PyObject *o, const char *attr_name) @@ -52,7 +70,8 @@ Object Protocol Exceptions that occur when this calls :meth:`~object.__getattr__` and :meth:`~object.__getattribute__` methods or while creating the temporary :class:`str` object are silently ignored. - For proper error handling, use :c:func:`PyObject_GetOptionalAttrString` + For proper error handling, use :c:func:`PyObject_HasAttrStringWithError`, + :c:func:`PyObject_GetOptionalAttrString` or :c:func:`PyObject_GetAttrString` instead. diff --git a/Doc/whatsnew/3.13.rst b/Doc/whatsnew/3.13.rst index de23172ac7a43b..c43427c27185d4 100644 --- a/Doc/whatsnew/3.13.rst +++ b/Doc/whatsnew/3.13.rst @@ -887,6 +887,18 @@ New Features be treated as a failure. (Contributed by Serhiy Storchaka in :gh:`106307`.) +* Add fixed variants of functions which silently ignore errors: + + - :c:func:`PyObject_HasAttrWithError` replaces :c:func:`PyObject_HasAttr`. + - :c:func:`PyObject_HasAttrStringWithError` replaces :c:func:`PyObject_HasAttrString`. + - :c:func:`PyMapping_HasKeyWithError` replaces :c:func:`PyMapping_HasKey`. + - :c:func:`PyMapping_HasKeyStringWithError` replaces :c:func:`PyMapping_HasKeyString`. + + New functions return not only ``1`` for true and ``0`` for false, but also + ``-1`` for error. + + (Contributed by Serhiy Storchaka in :gh:`108511`.) + * If Python is built in :ref:`debug mode ` or :option:`with assertions <--with-assertions>`, :c:func:`PyTuple_SET_ITEM` and :c:func:`PyList_SET_ITEM` now check the index argument with an assertion. diff --git a/Include/abstract.h b/Include/abstract.h index dd915004e7834e..bd12a54963c13f 100644 --- a/Include/abstract.h +++ b/Include/abstract.h @@ -50,6 +50,25 @@ extern "C" { This function always succeeds. */ + +/* Implemented elsewhere: + + int PyObject_HasAttrStringWithError(PyObject *o, const char *attr_name); + + Returns 1 if object 'o' has the attribute attr_name, and 0 otherwise. + This is equivalent to the Python expression: hasattr(o,attr_name). + Returns -1 on failure. */ + + +/* Implemented elsewhere: + + int PyObject_HasAttrWithError(PyObject *o, PyObject *attr_name); + + Returns 1 if o has the attribute attr_name, and 0 otherwise. + This is equivalent to the Python expression: hasattr(o,attr_name). + Returns -1 on failure. */ + + /* Implemented elsewhere: PyObject* PyObject_GetAttr(PyObject *o, PyObject *attr_name); @@ -821,6 +840,18 @@ PyAPI_FUNC(int) PyMapping_HasKeyString(PyObject *o, const char *key); This function always succeeds. */ PyAPI_FUNC(int) PyMapping_HasKey(PyObject *o, PyObject *key); +/* Return 1 if the mapping object has the key 'key', and 0 otherwise. + This is equivalent to the Python expression: key in o. + On failure, return -1. */ + +PyAPI_FUNC(int) PyMapping_HasKeyWithError(PyObject *o, PyObject *key); + +/* Return 1 if the mapping object has the key 'key', and 0 otherwise. + This is equivalent to the Python expression: key in o. + On failure, return -1. */ + +PyAPI_FUNC(int) PyMapping_HasKeyStringWithError(PyObject *o, const char *key); + /* On success, return a list or tuple of the keys in mapping object 'o'. On failure, return NULL. */ PyAPI_FUNC(PyObject *) PyMapping_Keys(PyObject *o); diff --git a/Include/object.h b/Include/object.h index b94b2907e4f163..5bcb3937b8c896 100644 --- a/Include/object.h +++ b/Include/object.h @@ -394,6 +394,10 @@ PyAPI_FUNC(int) PyObject_GetOptionalAttrString(PyObject *, const char *, PyObjec PyAPI_FUNC(int) PyObject_SetAttr(PyObject *, PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_DelAttr(PyObject *v, PyObject *name); PyAPI_FUNC(int) PyObject_HasAttr(PyObject *, PyObject *); +#if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030d0000 +PyAPI_FUNC(int) PyObject_HasAttrWithError(PyObject *, PyObject *); +PyAPI_FUNC(int) PyObject_HasAttrStringWithError(PyObject *, const char *); +#endif PyAPI_FUNC(PyObject *) PyObject_SelfIter(PyObject *); PyAPI_FUNC(PyObject *) PyObject_GenericGetAttr(PyObject *, PyObject *); PyAPI_FUNC(int) PyObject_GenericSetAttr(PyObject *, PyObject *, PyObject *); diff --git a/Lib/test/test_capi/test_abstract.py b/Lib/test/test_capi/test_abstract.py index 671f62b977d2aa..7fad853ff54fe3 100644 --- a/Lib/test/test_capi/test_abstract.py +++ b/Lib/test/test_capi/test_abstract.py @@ -129,6 +129,34 @@ def test_object_hasattrstring(self): # CRASHES hasattrstring(obj, NULL) # CRASHES hasattrstring(NULL, b'a') + def test_object_hasattrwitherror(self): + xhasattr = _testcapi.object_hasattrwitherror + obj = TestObject() + obj.a = 1 + setattr(obj, '\U0001f40d', 2) + self.assertTrue(xhasattr(obj, 'a')) + self.assertFalse(xhasattr(obj, 'b')) + self.assertTrue(xhasattr(obj, '\U0001f40d')) + + self.assertRaises(RuntimeError, xhasattr, obj, 'evil') + self.assertRaises(TypeError, xhasattr, obj, 1) + # CRASHES xhasattr(obj, NULL) + # CRASHES xhasattr(NULL, 'a') + + def test_object_hasattrstringwitherror(self): + hasattrstring = _testcapi.object_hasattrstringwitherror + obj = TestObject() + obj.a = 1 + setattr(obj, '\U0001f40d', 2) + self.assertTrue(hasattrstring(obj, b'a')) + self.assertFalse(hasattrstring(obj, b'b')) + self.assertTrue(hasattrstring(obj, '\U0001f40d'.encode())) + + self.assertRaises(RuntimeError, hasattrstring, obj, b'evil') + self.assertRaises(UnicodeDecodeError, hasattrstring, obj, b'\xff') + # CRASHES hasattrstring(obj, NULL) + # CRASHES hasattrstring(NULL, b'a') + def test_object_setattr(self): xsetattr = _testcapi.object_setattr obj = TestObject() @@ -339,6 +367,44 @@ def test_mapping_haskeystring(self): self.assertFalse(haskeystring([], b'a')) self.assertFalse(haskeystring(NULL, b'a')) + def test_mapping_haskeywitherror(self): + haskey = _testcapi.mapping_haskeywitherror + dct = {'a': 1, '\U0001f40d': 2} + self.assertTrue(haskey(dct, 'a')) + self.assertFalse(haskey(dct, 'b')) + self.assertTrue(haskey(dct, '\U0001f40d')) + + dct2 = ProxyGetItem(dct) + self.assertTrue(haskey(dct2, 'a')) + self.assertFalse(haskey(dct2, 'b')) + + self.assertTrue(haskey(['a', 'b', 'c'], 1)) + + self.assertRaises(TypeError, haskey, 42, 'a') + self.assertRaises(TypeError, haskey, {}, []) # unhashable + self.assertRaises(IndexError, haskey, [], 1) + self.assertRaises(TypeError, haskey, [], 'a') + + # CRASHES haskey({}, NULL)) + # CRASHES haskey(NULL, 'a')) + + def test_mapping_haskeystringwitherror(self): + haskeystring = _testcapi.mapping_haskeystringwitherror + dct = {'a': 1, '\U0001f40d': 2} + self.assertTrue(haskeystring(dct, b'a')) + self.assertFalse(haskeystring(dct, b'b')) + self.assertTrue(haskeystring(dct, '\U0001f40d'.encode())) + + dct2 = ProxyGetItem(dct) + self.assertTrue(haskeystring(dct2, b'a')) + self.assertFalse(haskeystring(dct2, b'b')) + + self.assertRaises(TypeError, haskeystring, 42, b'a') + self.assertRaises(UnicodeDecodeError, haskeystring, {}, b'\xff') + self.assertRaises(SystemError, haskeystring, {}, NULL) + self.assertRaises(TypeError, haskeystring, [], b'a') + # CRASHES haskeystring(NULL, b'a') + def test_object_setitem(self): setitem = _testcapi.object_setitem dct = {} diff --git a/Misc/NEWS.d/next/C API/2023-09-01-16-28-09.gh-issue-108511.gg-QDG.rst b/Misc/NEWS.d/next/C API/2023-09-01-16-28-09.gh-issue-108511.gg-QDG.rst new file mode 100644 index 00000000000000..1e5f32905aa24d --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-09-01-16-28-09.gh-issue-108511.gg-QDG.rst @@ -0,0 +1,4 @@ +Add functions :c:func:`PyObject_HasAttrWithError`, +:c:func:`PyObject_HasAttrStringWithError`, +:c:func:`PyMapping_HasKeyWithError` and +:c:func:`PyMapping_HasKeyStringWithError`. diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index 9b0ca73a8b1751..6fbcf77a115371 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -386,11 +386,11 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct if (fields == NULL) return 0; - if (PyObject_GetOptionalAttr(type, &_Py_ID(_swappedbytes_), &tmp) < 0) { + int rc = PyObject_HasAttrWithError(type, &_Py_ID(_swappedbytes_)); + if (rc < 0) { return -1; } - if (tmp) { - Py_DECREF(tmp); + if (rc) { big_endian = !PY_BIG_ENDIAN; } else { diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index 8cb57e693d81d7..f9d5793f9b6497 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -3532,12 +3532,11 @@ expat_start_doctype_handler(XMLParserObject *self, sysid_obj, NULL); Py_XDECREF(res); } - else if (PyObject_GetOptionalAttr((PyObject *)self, st->str_doctype, &res) > 0) { + else if (PyObject_HasAttrWithError((PyObject *)self, st->str_doctype) > 0) { (void)PyErr_WarnEx(PyExc_RuntimeWarning, "The doctype() method of XMLParser is ignored. " "Define doctype() method on the TreeBuilder target.", 1); - Py_DECREF(res); } Py_DECREF(doctype_name_obj); diff --git a/Modules/_io/iobase.c b/Modules/_io/iobase.c index 34fcd702391f32..78f0f949b68c06 100644 --- a/Modules/_io/iobase.c +++ b/Modules/_io/iobase.c @@ -148,13 +148,9 @@ _io__IOBase_truncate_impl(PyObject *self, PyTypeObject *cls, static int iobase_is_closed(PyObject *self) { - PyObject *res; - int ret; /* This gets the derived attribute, which is *not* __IOBase_closed in most cases! */ - ret = PyObject_GetOptionalAttr(self, &_Py_ID(__IOBase_closed), &res); - Py_XDECREF(res); - return ret; + return PyObject_HasAttrWithError(self, &_Py_ID(__IOBase_closed)); } /* Flush and close methods */ diff --git a/Modules/_io/textio.c b/Modules/_io/textio.c index 0a727a6e0ecd8a..91b677bde77dde 100644 --- a/Modules/_io/textio.c +++ b/Modules/_io/textio.c @@ -1223,11 +1223,10 @@ _io_TextIOWrapper___init___impl(textio *self, PyObject *buffer, goto error; self->seekable = self->telling = r; - r = PyObject_GetOptionalAttr(buffer, &_Py_ID(read1), &res); + r = PyObject_HasAttrWithError(buffer, &_Py_ID(read1)); if (r < 0) { goto error; } - Py_XDECREF(res); self->has_read1 = r; self->encoding_start_of_stream = 0; diff --git a/Modules/_pickle.c b/Modules/_pickle.c index b97524856eeca8..a3cf34699ba509 100644 --- a/Modules/_pickle.c +++ b/Modules/_pickle.c @@ -5799,14 +5799,13 @@ instantiate(PyObject *cls, PyObject *args) into a newly created tuple. */ assert(PyTuple_Check(args)); if (!PyTuple_GET_SIZE(args) && PyType_Check(cls)) { - PyObject *func; - if (PyObject_GetOptionalAttr(cls, &_Py_ID(__getinitargs__), &func) < 0) { + int rc = PyObject_HasAttrWithError(cls, &_Py_ID(__getinitargs__)); + if (rc < 0) { return NULL; } - if (func == NULL) { + if (!rc) { return PyObject_CallMethodOneArg(cls, &_Py_ID(__new__), cls); } - Py_DECREF(func); } return PyObject_CallObject(cls, args); } diff --git a/Modules/_testcapi/abstract.c b/Modules/_testcapi/abstract.c index bde0d2848954ed..81a3dea4c1dfde 100644 --- a/Modules/_testcapi/abstract.c +++ b/Modules/_testcapi/abstract.c @@ -105,6 +105,31 @@ object_hasattrstring(PyObject *self, PyObject *args) return PyLong_FromLong(PyObject_HasAttrString(obj, attr_name)); } +static PyObject * +object_hasattrwitherror(PyObject *self, PyObject *args) +{ + PyObject *obj, *attr_name; + if (!PyArg_ParseTuple(args, "OO", &obj, &attr_name)) { + return NULL; + } + NULLABLE(obj); + NULLABLE(attr_name); + RETURN_INT(PyObject_HasAttrWithError(obj, attr_name)); +} + +static PyObject * +object_hasattrstringwitherror(PyObject *self, PyObject *args) +{ + PyObject *obj; + const char *attr_name; + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "Oz#", &obj, &attr_name, &size)) { + return NULL; + } + NULLABLE(obj); + RETURN_INT(PyObject_HasAttrStringWithError(obj, attr_name)); +} + static PyObject * object_setattr(PyObject *self, PyObject *args) { @@ -280,6 +305,31 @@ mapping_haskeystring(PyObject *self, PyObject *args) return PyLong_FromLong(PyMapping_HasKeyString(mapping, key)); } +static PyObject * +mapping_haskeywitherror(PyObject *self, PyObject *args) +{ + PyObject *mapping, *key; + if (!PyArg_ParseTuple(args, "OO", &mapping, &key)) { + return NULL; + } + NULLABLE(mapping); + NULLABLE(key); + RETURN_INT(PyMapping_HasKeyWithError(mapping, key)); +} + +static PyObject * +mapping_haskeystringwitherror(PyObject *self, PyObject *args) +{ + PyObject *mapping; + const char *key; + Py_ssize_t size; + if (!PyArg_ParseTuple(args, "Oz#", &mapping, &key, &size)) { + return NULL; + } + NULLABLE(mapping); + RETURN_INT(PyMapping_HasKeyStringWithError(mapping, key)); +} + static PyObject * object_setitem(PyObject *self, PyObject *args) { @@ -568,6 +618,8 @@ static PyMethodDef test_methods[] = { {"object_getoptionalattrstring", object_getoptionalattrstring, METH_VARARGS}, {"object_hasattr", object_hasattr, METH_VARARGS}, {"object_hasattrstring", object_hasattrstring, METH_VARARGS}, + {"object_hasattrwitherror", object_hasattrwitherror, METH_VARARGS}, + {"object_hasattrstringwitherror", object_hasattrstringwitherror, METH_VARARGS}, {"object_setattr", object_setattr, METH_VARARGS}, {"object_setattrstring", object_setattrstring, METH_VARARGS}, {"object_delattr", object_delattr, METH_VARARGS}, @@ -582,6 +634,8 @@ static PyMethodDef test_methods[] = { {"mapping_getoptionalitemstring", mapping_getoptionalitemstring, METH_VARARGS}, {"mapping_haskey", mapping_haskey, METH_VARARGS}, {"mapping_haskeystring", mapping_haskeystring, METH_VARARGS}, + {"mapping_haskeywitherror", mapping_haskeywitherror, METH_VARARGS}, + {"mapping_haskeystringwitherror", mapping_haskeystringwitherror, METH_VARARGS}, {"object_setitem", object_setitem, METH_VARARGS}, {"mapping_setitemstring", mapping_setitemstring, METH_VARARGS}, {"object_delitem", object_delitem, METH_VARARGS}, diff --git a/Modules/_xxinterpchannelsmodule.c b/Modules/_xxinterpchannelsmodule.c index 1e418414767db8..60ac8ed1b38ff2 100644 --- a/Modules/_xxinterpchannelsmodule.c +++ b/Modules/_xxinterpchannelsmodule.c @@ -123,7 +123,7 @@ get_module_from_type(PyTypeObject *cls) static PyObject * add_new_exception(PyObject *mod, const char *name, PyObject *base) { - assert(!PyObject_HasAttrString(mod, name)); + assert(!PyObject_HasAttrStringWithError(mod, name)); PyObject *exctype = PyErr_NewException(name, base, NULL); if (exctype == NULL) { return NULL; diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c index 6638c2c8e0636b..2dd8d9a6f3eb7d 100644 --- a/Modules/_xxsubinterpretersmodule.c +++ b/Modules/_xxsubinterpretersmodule.c @@ -41,7 +41,7 @@ _get_current_interp(void) static PyObject * add_new_exception(PyObject *mod, const char *name, PyObject *base) { - assert(!PyObject_HasAttrString(mod, name)); + assert(!PyObject_HasAttrStringWithError(mod, name)); PyObject *exctype = PyErr_NewException(name, base, NULL); if (exctype == NULL) { return NULL; diff --git a/Objects/abstract.c b/Objects/abstract.c index b57190d2521e11..55d3b3ada296be 100644 --- a/Objects/abstract.c +++ b/Objects/abstract.c @@ -2426,6 +2426,24 @@ PyMapping_SetItemString(PyObject *o, const char *key, PyObject *value) return r; } +int +PyMapping_HasKeyStringWithError(PyObject *obj, const char *key) +{ + PyObject *res; + int rc = PyMapping_GetOptionalItemString(obj, key, &res); + Py_XDECREF(res); + return rc; +} + +int +PyMapping_HasKeyWithError(PyObject *obj, PyObject *key) +{ + PyObject *res; + int rc = PyMapping_GetOptionalItem(obj, key, &res); + Py_XDECREF(res); + return rc; +} + int PyMapping_HasKeyString(PyObject *o, const char *key) { diff --git a/Objects/dictobject.c b/Objects/dictobject.c index 7cbc49f96994f7..f953a0fa793296 100644 --- a/Objects/dictobject.c +++ b/Objects/dictobject.c @@ -2688,12 +2688,11 @@ dict_update_arg(PyObject *self, PyObject *arg) if (PyDict_CheckExact(arg)) { return PyDict_Merge(self, arg, 1); } - PyObject *func; - if (PyObject_GetOptionalAttr(arg, &_Py_ID(keys), &func) < 0) { + int has_keys = PyObject_HasAttrWithError(arg, &_Py_ID(keys)); + if (has_keys < 0) { return -1; } - if (func != NULL) { - Py_DECREF(func); + if (has_keys) { return PyDict_Merge(self, arg, 1); } return PyDict_MergeFromSeq2(self, arg, 1); diff --git a/Objects/genericaliasobject.c b/Objects/genericaliasobject.c index bb50537461040b..e0f0107252d769 100644 --- a/Objects/genericaliasobject.c +++ b/Objects/genericaliasobject.c @@ -55,8 +55,7 @@ ga_repr_item(_PyUnicodeWriter *writer, PyObject *p) PyObject *qualname = NULL; PyObject *module = NULL; PyObject *r = NULL; - PyObject *tmp; - int err; + int rc; if (p == Py_Ellipsis) { // The Ellipsis object @@ -64,19 +63,14 @@ ga_repr_item(_PyUnicodeWriter *writer, PyObject *p) goto done; } - if (PyObject_GetOptionalAttr(p, &_Py_ID(__origin__), &tmp) < 0) { - goto done; + if ((rc = PyObject_HasAttrWithError(p, &_Py_ID(__origin__))) > 0 && + (rc = PyObject_HasAttrWithError(p, &_Py_ID(__args__))) > 0) + { + // It looks like a GenericAlias + goto use_repr; } - if (tmp != NULL) { - Py_DECREF(tmp); - if (PyObject_GetOptionalAttr(p, &_Py_ID(__args__), &tmp) < 0) { - goto done; - } - if (tmp != NULL) { - Py_DECREF(tmp); - // It looks like a GenericAlias - goto use_repr; - } + if (rc < 0) { + goto done; } if (PyObject_GetOptionalAttr(p, &_Py_ID(__qualname__), &qualname) < 0) { @@ -113,13 +107,13 @@ ga_repr_item(_PyUnicodeWriter *writer, PyObject *p) Py_XDECREF(module); if (r == NULL) { // error if any of the above PyObject_Repr/PyUnicode_From* fail - err = -1; + rc = -1; } else { - err = _PyUnicodeWriter_WriteStr(writer, r); + rc = _PyUnicodeWriter_WriteStr(writer, r); Py_DECREF(r); } - return err; + return rc; } static int @@ -253,18 +247,17 @@ _Py_make_parameters(PyObject *args) Py_ssize_t iparam = 0; for (Py_ssize_t iarg = 0; iarg < nargs; iarg++) { PyObject *t = PyTuple_GET_ITEM(args, iarg); - PyObject *subst; // We don't want __parameters__ descriptor of a bare Python class. if (PyType_Check(t)) { continue; } - if (PyObject_GetOptionalAttr(t, &_Py_ID(__typing_subst__), &subst) < 0) { + int rc = PyObject_HasAttrWithError(t, &_Py_ID(__typing_subst__)); + if (rc < 0) { Py_DECREF(parameters); return NULL; } - if (subst) { + if (rc) { iparam += tuple_add(parameters, iparam, t); - Py_DECREF(subst); } else { PyObject *subparams; diff --git a/Objects/object.c b/Objects/object.c index 7aeda50e9b2757..15c2bf65de6acf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -911,26 +911,24 @@ PyObject_GetAttrString(PyObject *v, const char *name) } int -PyObject_HasAttrString(PyObject *v, const char *name) +PyObject_HasAttrStringWithError(PyObject *obj, const char *name) { - if (Py_TYPE(v)->tp_getattr != NULL) { - PyObject *res = (*Py_TYPE(v)->tp_getattr)(v, (char*)name); - if (res != NULL) { - Py_DECREF(res); - return 1; - } - PyErr_Clear(); - return 0; - } + PyObject *res; + int rc = PyObject_GetOptionalAttrString(obj, name, &res); + Py_XDECREF(res); + return rc; +} + - PyObject *attr_name = PyUnicode_FromString(name); - if (attr_name == NULL) { +int +PyObject_HasAttrString(PyObject *obj, const char *name) +{ + int rc = PyObject_HasAttrStringWithError(obj, name); + if (rc < 0) { PyErr_Clear(); return 0; } - int ok = PyObject_HasAttr(v, attr_name); - Py_DECREF(attr_name); - return ok; + return rc; } int @@ -1149,18 +1147,23 @@ PyObject_GetOptionalAttrString(PyObject *obj, const char *name, PyObject **resul } int -PyObject_HasAttr(PyObject *v, PyObject *name) +PyObject_HasAttrWithError(PyObject *obj, PyObject *name) { PyObject *res; - if (PyObject_GetOptionalAttr(v, name, &res) < 0) { + int rc = PyObject_GetOptionalAttr(obj, name, &res); + Py_XDECREF(res); + return rc; +} + +int +PyObject_HasAttr(PyObject *obj, PyObject *name) +{ + int rc = PyObject_HasAttrWithError(obj, name); + if (rc < 0) { PyErr_Clear(); return 0; } - if (res == NULL) { - return 0; - } - Py_DECREF(res); - return 1; + return rc; } int diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 84c50507691cbe..893d8420bba4c4 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3879,16 +3879,14 @@ type_new_get_bases(type_new_ctx *ctx, PyObject **type) if (PyType_Check(base)) { continue; } - PyObject *mro_entries; - if (PyObject_GetOptionalAttr(base, &_Py_ID(__mro_entries__), - &mro_entries) < 0) { + int rc = PyObject_HasAttrWithError(base, &_Py_ID(__mro_entries__)); + if (rc < 0) { return -1; } - if (mro_entries != NULL) { + if (rc) { PyErr_SetString(PyExc_TypeError, "type() doesn't support MRO entry resolution; " "use types.new_class()"); - Py_DECREF(mro_entries); return -1; } } diff --git a/Objects/unionobject.c b/Objects/unionobject.c index 347945a4c45972..36bbddb55e2ced 100644 --- a/Objects/unionobject.c +++ b/Objects/unionobject.c @@ -186,28 +186,21 @@ union_repr_item(_PyUnicodeWriter *writer, PyObject *p) { PyObject *qualname = NULL; PyObject *module = NULL; - PyObject *tmp; PyObject *r = NULL; - int err; + int rc; if (p == (PyObject *)&_PyNone_Type) { return _PyUnicodeWriter_WriteASCIIString(writer, "None", 4); } - if (PyObject_GetOptionalAttr(p, &_Py_ID(__origin__), &tmp) < 0) { - goto exit; + if ((rc = PyObject_HasAttrWithError(p, &_Py_ID(__origin__))) > 0 && + (rc = PyObject_HasAttrWithError(p, &_Py_ID(__args__))) > 0) + { + // It looks like a GenericAlias + goto use_repr; } - - if (tmp) { - Py_DECREF(tmp); - if (PyObject_GetOptionalAttr(p, &_Py_ID(__args__), &tmp) < 0) { - goto exit; - } - if (tmp) { - // It looks like a GenericAlias - Py_DECREF(tmp); - goto use_repr; - } + if (rc < 0) { + goto exit; } if (PyObject_GetOptionalAttr(p, &_Py_ID(__qualname__), &qualname) < 0) { @@ -244,9 +237,9 @@ union_repr_item(_PyUnicodeWriter *writer, PyObject *p) if (r == NULL) { return -1; } - err = _PyUnicodeWriter_WriteStr(writer, r); + rc = _PyUnicodeWriter_WriteStr(writer, r); Py_DECREF(r); - return err; + return rc; } static PyObject * diff --git a/Python/errors.c b/Python/errors.c index f670b78c1f14ef..e6fa15f92b5315 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -1776,13 +1776,11 @@ PyErr_SyntaxLocationObjectEx(PyObject *filename, int lineno, int col_offset, } } if ((PyObject *)Py_TYPE(exc) != PyExc_SyntaxError) { - if (PyObject_GetOptionalAttr(exc, &_Py_ID(msg), &tmp) < 0) { + int rc = PyObject_HasAttrWithError(exc, &_Py_ID(msg)); + if (rc < 0) { _PyErr_Clear(tstate); } - else if (tmp) { - Py_DECREF(tmp); - } - else { + else if (!rc) { tmp = PyObject_Str(exc); if (tmp) { if (PyObject_SetAttr(exc, &_Py_ID(msg), tmp)) { @@ -1795,13 +1793,11 @@ PyErr_SyntaxLocationObjectEx(PyObject *filename, int lineno, int col_offset, } } - if (PyObject_GetOptionalAttr(exc, &_Py_ID(print_file_and_line), &tmp) < 0) { + rc = PyObject_HasAttrWithError(exc, &_Py_ID(print_file_and_line)); + if (rc < 0) { _PyErr_Clear(tstate); } - else if (tmp) { - Py_DECREF(tmp); - } - else { + else if (!rc) { if (PyObject_SetAttr(exc, &_Py_ID(print_file_and_line), Py_None)) { _PyErr_Clear(tstate); } diff --git a/Python/import.c b/Python/import.c index 48090d05240188..488d5af24eee9c 100644 --- a/Python/import.c +++ b/Python/import.c @@ -2898,12 +2898,11 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *globals, } } else { - PyObject *path; - if (PyObject_GetOptionalAttr(mod, &_Py_ID(__path__), &path) < 0) { + int has_path = PyObject_HasAttrWithError(mod, &_Py_ID(__path__)); + if (has_path < 0) { goto error; } - if (path) { - Py_DECREF(path); + if (has_path) { final_mod = PyObject_CallMethodObjArgs( IMPORTLIB(interp), &_Py_ID(_handle_fromlist), mod, fromlist, IMPORT_FUNC(interp), NULL); diff --git a/Python/suggestions.c b/Python/suggestions.c index 9247da4c7037a2..1ad359b18923f3 100644 --- a/Python/suggestions.c +++ b/Python/suggestions.c @@ -245,14 +245,12 @@ get_suggestions_for_name_error(PyObject* name, PyFrameObject* frame) goto error; } - PyObject *value; - res = PyObject_GetOptionalAttr(self, name, &value); + res = PyObject_HasAttrWithError(self, name); Py_DECREF(locals); if (res < 0) { goto error; } - if (value) { - Py_DECREF(value); + if (res) { Py_DECREF(dir); return PyUnicode_FromFormat("self.%U", name); } From 7bcdfad04b2cdfa11b0bb2bd846734b8b8b841c7 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 6 Sep 2023 23:45:14 +0300 Subject: [PATCH 2/2] regen-limited-abi --- Doc/data/stable_abi.dat | 4 ++++ Lib/test/test_stable_abi_ctypes.py | 4 ++++ Misc/stable_abi.toml | 8 ++++++++ PC/python3dll.c | 4 ++++ 4 files changed, 20 insertions(+) diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index cc6349a0330e8c..c189c78238f40f 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -377,6 +377,8 @@ function,PyMapping_GetOptionalItem,3.13,, function,PyMapping_GetOptionalItemString,3.13,, function,PyMapping_HasKey,3.2,, function,PyMapping_HasKeyString,3.2,, +function,PyMapping_HasKeyStringWithError,3.13,, +function,PyMapping_HasKeyWithError,3.13,, function,PyMapping_Items,3.2,, function,PyMapping_Keys,3.2,, function,PyMapping_Length,3.2,, @@ -523,6 +525,8 @@ function,PyObject_GetOptionalAttrString,3.13,, function,PyObject_GetTypeData,3.12,, function,PyObject_HasAttr,3.2,, function,PyObject_HasAttrString,3.2,, +function,PyObject_HasAttrStringWithError,3.13,, +function,PyObject_HasAttrWithError,3.13,, function,PyObject_Hash,3.2,, function,PyObject_HashNotImplemented,3.2,, function,PyObject_Init,3.2,, diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 1f3cf612c18f6d..94f817f8e1d159 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -405,6 +405,8 @@ def test_windows_feature_macros(self): "PyMapping_GetOptionalItemString", "PyMapping_HasKey", "PyMapping_HasKeyString", + "PyMapping_HasKeyStringWithError", + "PyMapping_HasKeyWithError", "PyMapping_Items", "PyMapping_Keys", "PyMapping_Length", @@ -542,6 +544,8 @@ def test_windows_feature_macros(self): "PyObject_GetTypeData", "PyObject_HasAttr", "PyObject_HasAttrString", + "PyObject_HasAttrStringWithError", + "PyObject_HasAttrWithError", "PyObject_Hash", "PyObject_HashNotImplemented", "PyObject_Init", diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 2030a085abf27c..8df3f85e61eec6 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2452,3 +2452,11 @@ added = '3.13' [function.PyLong_AsInt] added = '3.13' +[function.PyObject_HasAttrWithError] + added = '3.13' +[function.PyObject_HasAttrStringWithError] + added = '3.13' +[function.PyMapping_HasKeyWithError] + added = '3.13' +[function.PyMapping_HasKeyStringWithError] + added = '3.13' diff --git a/PC/python3dll.c b/PC/python3dll.c index ee3a7d7b4e5be1..2c1cc8098ce856 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -359,6 +359,8 @@ EXPORT_FUNC(PyMapping_GetOptionalItem) EXPORT_FUNC(PyMapping_GetOptionalItemString) EXPORT_FUNC(PyMapping_HasKey) EXPORT_FUNC(PyMapping_HasKeyString) +EXPORT_FUNC(PyMapping_HasKeyStringWithError) +EXPORT_FUNC(PyMapping_HasKeyWithError) EXPORT_FUNC(PyMapping_Items) EXPORT_FUNC(PyMapping_Keys) EXPORT_FUNC(PyMapping_Length) @@ -480,6 +482,8 @@ EXPORT_FUNC(PyObject_GetOptionalAttrString) EXPORT_FUNC(PyObject_GetTypeData) EXPORT_FUNC(PyObject_HasAttr) EXPORT_FUNC(PyObject_HasAttrString) +EXPORT_FUNC(PyObject_HasAttrStringWithError) +EXPORT_FUNC(PyObject_HasAttrWithError) EXPORT_FUNC(PyObject_Hash) EXPORT_FUNC(PyObject_HashNotImplemented) EXPORT_FUNC(PyObject_Init)