gh-108511: Add C API functions which do not silently ignore errors

Add the following functions: * PyObject_HasAttrWithError() * PyObject_HasAttrStringWithError() * PyMapping_HasKeyWithError() * PyMapping_HasKeyStringWithError()
serhiy-storchaka · Sep 17, 2023 · Sep 1, 2023 · Sep 6, 2023 · Sep 6, 2023 · ae6e78904ce014135837ed134d6451127d443e74
commit ae6e78904ce014135837ed134d6451127d443e74
@@ -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``.
-   On failure, return ``-1``.
+   On failure, return ``-1`` with an exception set.
-   On failure, return ``-1``.
+   On failure, return ``-1`` with an exception set.
+
+   .. 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.
 
 

@@ -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``.
-   On failure, return ``-1``.
+   On failure, return ``-1`` with an exception set.
-   On failure, return ``-1``.
+   On failure, return ``-1`` with an exception set.
+
+   .. 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.
 
 

@@ -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 <debug-build>` 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
 }