serhiy-storchaka · Nov 7, 2023 · Jul 12, 2023 · Jul 12, 2023 · Jul 12, 2023 · Jul 12, 2023
@@ -700,6 +700,15 @@ that may require changes to your code.
   Note that ``Py_TRASHCAN_BEGIN`` has a second argument which
   should be the deallocation function it is in.
 
+* Functions :c:func:`PyDict_GetItem`, :c:func:`PyDict_GetItemString`,
+  :c:func:`PyMapping_HasKey`, :c:func:`PyMapping_HasKeyString`,
+  :c:func:`PyObject_HasAttr`, :c:func:`PyObject_HasAttrString`, and
+  :c:func:`PySys_GetObject`, which clear all errors occurred during calling
+  the function, report now them using :func:`sys.unraisablehook`.
+  You can consider to replace these functions with other functions as
+  recomended in the documentation.
+  (Contributed by Serhiy Storchaka in :gh:`106672`.)
+
 
 Build Changes
 =============

diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py
@@ -51,6 +51,8 @@
 import _testinternalcapi
 
 
+NULL = None
+
 def decode_stderr(err):
     return err.decode('utf-8', 'replace').replace('\r', '')
 
@@ -338,16 +340,83 @@ def items(self):
         self.assertRaises(TypeError, _testcapi.get_mapping_items, bad_mapping)
 
     def test_mapping_has_key(self):
-        dct = {'a': 1}
-        self.assertTrue(_testcapi.mapping_has_key(dct, 'a'))
-        self.assertFalse(_testcapi.mapping_has_key(dct, 'b'))
-
-        class SubDict(dict):
-            pass
-
-        dct2 = SubDict({'a': 1})
-        self.assertTrue(_testcapi.mapping_has_key(dct2, 'a'))
-        self.assertFalse(_testcapi.mapping_has_key(dct2, 'b'))
+        has_key = _testcapi.mapping_has_key
+        dct = {'a': 1, '\U0001f40d': 2}
+        self.assertTrue(has_key(dct, 'a'))
+        self.assertFalse(has_key(dct, 'b'))
+        self.assertTrue(has_key(dct, '\U0001f40d'))
+
+        class M:
+            def __getitem__(self, key):
+                return dct[key]
+
+        dct2 = M()
+        self.assertTrue(has_key(dct2, 'a'))
+        self.assertFalse(has_key(dct2, 'b'))
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key(42, 'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "'int' object is not subscriptable")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key({}, []))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "unhashable type: 'list'")
+
+        self.assertTrue(has_key(['a', 'b'], 1))
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key([], 1))
+            self.assertEqual(cm.unraisable.exc_type, IndexError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'list index out of range')
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key([], 'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'list indices must be integers or slices, not str')
+
+    def test_mapping_has_key_string(self):
+        has_key_string = _testcapi.mapping_has_key_string
+        dct = {'a': 1, '\U0001f40d': 2}
+        self.assertTrue(has_key_string(dct, b'a'))
+        self.assertFalse(has_key_string(dct, b'b'))
+        self.assertTrue(has_key_string(dct, '\U0001f40d'.encode()))
+
+        class M:
+            def __getitem__(self, key):
+                return dct[key]
+
+        dct2 = M()
+        self.assertTrue(has_key_string(dct2, b'a'))
+        self.assertFalse(has_key_string(dct2, b'b'))
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key_string(42, b'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "'int' object is not subscriptable")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key_string({}, b'\xff'))
+            self.assertEqual(cm.unraisable.exc_type, UnicodeDecodeError)
+            self.assertRegex(str(cm.unraisable.exc_value),
+                             "'utf-8' codec can't decode")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key_string({}, NULL))
+            self.assertEqual(cm.unraisable.exc_type, SystemError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             "null argument to internal routine")
+
+        with support.catch_unraisable_exception() as cm:
+            self.assertFalse(has_key_string([], b'a'))
+            self.assertEqual(cm.unraisable.exc_type, TypeError)
+            self.assertEqual(str(cm.unraisable.exc_value),
+                             'list indices must be integers or slices, not str')
 
     def test_sequence_set_slice(self):
         # Correct case:

diff --git a/Misc/NEWS.d/next/C API/2023-07-12-12-14-52.gh-issue-106672.fkRjmi.rst b/Misc/NEWS.d/next/C API/2023-07-12-12-14-52.gh-issue-106672.fkRjmi.rst
@@ -0,0 +1,5 @@
+Functions :c:func:`PyDict_GetItem`, :c:func:`PyDict_GetItemString`,
+:c:func:`PyMapping_HasKey`, :c:func:`PyMapping_HasKeyString`,
+:c:func:`PyObject_HasAttr`, :c:func:`PyObject_HasAttrString`, and
+:c:func:`PySys_GetObject`, which clear all errors occurred during calling
+the function, report now them using :func:`sys.unraisablehook`.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -2078,37 +2078,25 @@ get_mapping_items(PyObject* self, PyObject *obj)
 }
 
 static PyObject *
-test_mapping_has_key_string(PyObject *self, PyObject *Py_UNUSED(args))
+mapping_has_key(PyObject* self, PyObject *args)
 {
-    PyObject *context = PyDict_New();
-    PyObject *val = PyLong_FromLong(1);
-
-    // Since this uses `const char*` it is easier to test this in C:
-    PyDict_SetItemString(context, "a", val);
-    if (!PyMapping_HasKeyString(context, "a")) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "Existing mapping key does not exist");
-        return NULL;
-    }
-    if (PyMapping_HasKeyString(context, "b")) {
-        PyErr_SetString(PyExc_RuntimeError,
-                        "Missing mapping key exists");
+    PyObject *context, *key;
+    if (!PyArg_ParseTuple(args, "OO", &context, &key)) {
         return NULL;
     }
-
-    Py_DECREF(val);
-    Py_DECREF(context);
-    Py_RETURN_NONE;
+    return PyLong_FromLong(PyMapping_HasKey(context, key));
 }
 
 static PyObject *
-mapping_has_key(PyObject* self, PyObject *args)
+mapping_has_key_string(PyObject* self, PyObject *args)
 {
-    PyObject *context, *key;
-    if (!PyArg_ParseTuple(args, "OO", &context, &key)) {
+    PyObject *context;
+    const char *key;
+    Py_ssize_t size;
+    if (!PyArg_ParseTuple(args, "Oz#", &context, &key, &size)) {
         return NULL;
     }
-    return PyLong_FromLong(PyMapping_HasKey(context, key));
+    return PyLong_FromLong(PyMapping_HasKeyString(context, key));
 }
 
 static PyObject *
@@ -3548,8 +3536,8 @@ static PyMethodDef TestMethods[] = {
     {"get_mapping_keys", get_mapping_keys, METH_O},
     {"get_mapping_values", get_mapping_values, METH_O},
     {"get_mapping_items", get_mapping_items, METH_O},
-    {"test_mapping_has_key_string", test_mapping_has_key_string, METH_NOARGS},
     {"mapping_has_key", mapping_has_key, METH_VARARGS},
+    {"mapping_has_key_string", mapping_has_key_string, METH_VARARGS},
     {"sequence_set_slice", sequence_set_slice, METH_VARARGS},
     {"sequence_del_slice", sequence_del_slice, METH_VARARGS},
     {"test_pythread_tss_key_state", test_pythread_tss_key_state, METH_VARARGS},

diff --git a/Objects/abstract.c b/Objects/abstract.c
@@ -2426,31 +2426,51 @@ PyMapping_SetItemString(PyObject *o, const char *key, PyObject *value)
 }
 
 int
-PyMapping_HasKeyString(PyObject *o, const char *key)
-{
-    PyObject *v;
-
-    v = PyMapping_GetItemString(o, key);
-    if (v) {
-        Py_DECREF(v);
-        return 1;
+PyMapping_HasKeyString(PyObject *obj, const char *key)
+{
+    PyObject *dummy;
+    int rc = PyMapping_GetOptionalItemString(obj, key, &dummy);
+    if (rc < 0) {
+        _PyErr_WriteUnraisableMsg(
+            "in PyMapping_HasKeyString(); consider using "
+            "PyMapping_GetOptionalItemString() or PyMapping_GetItemString()",
+            NULL);
+        return 0;
     }
-    PyErr_Clear();
-    return 0;
+    // PyMapping_HasKeyString() also clears the error set before it's call
+    // if the key is not found.
+    if (rc == 0 && PyErr_Occurred()) {
+        _PyErr_WriteUnraisableMsg(
+            "in PyMapping_HasKeyString(); consider using "
+            "PyMapping_GetOptionalItemString() or PyMapping_GetItemString()",
+            NULL);
+        return 0;
+    }
+    Py_XDECREF(dummy);
+    return rc;
 }
 
 int
-PyMapping_HasKey(PyObject *o, PyObject *key)
-{
-    PyObject *v;
-
-    v = PyObject_GetItem(o, key);
-    if (v) {
-        Py_DECREF(v);
-        return 1;
+PyMapping_HasKey(PyObject *obj, PyObject *key)
+{
+    PyObject *dummy;
+    int rc = PyMapping_GetOptionalItem(obj, key, &dummy);
+    if (rc < 0) {
+        _PyErr_WriteUnraisableMsg(
+            "in PyMapping_HasKey(); consider using "
+            "PyMapping_GetOptionalItem() or PyObject_GetItem()", NULL);
+        return 0;
     }
-    PyErr_Clear();
-    return 0;
+    // PyMapping_HasKey() also clears the error set before it's call
+    // if the key is not found.
+    if (rc == 0 && PyErr_Occurred()) {
+        _PyErr_WriteUnraisableMsg(
+            "in PyMapping_HasKey(); consider using "
+            "PyMapping_GetOptionalItem() or PyObject_GetItem()", NULL);
+        return 0;
+    }
+    Py_XDECREF(dummy);
+    return rc;
 }
 
 /* This function is quite similar to PySequence_Fast(), but specialized to be

@@ -1662,7 +1662,7 @@ _PyDict_FromItems(PyObject *const *keys, Py_ssize_t keys_offset,
  * even if the key is present.
  */
 PyObject *
-PyDict_GetItem(PyObject *op, PyObject *key)
+dict_getitem(PyObject *op, PyObject *key, const char *warnmsg)
 {
     if (!PyDict_Check(op)) {
         return NULL;
@@ -1673,7 +1673,7 @@ PyDict_GetItem(PyObject *op, PyObject *key)
     if (!PyUnicode_CheckExact(key) || (hash = unicode_get_hash(key)) == -1) {
         hash = PyObject_Hash(key);
         if (hash == -1) {
-            PyErr_Clear();
+            _PyErr_WriteUnraisableMsg(warnmsg, NULL);
             return NULL;
         }
     }
@@ -1694,13 +1694,25 @@ PyDict_GetItem(PyObject *op, PyObject *key)
     ix = _Py_dict_lookup(mp, key, hash, &value);
 
     /* Ignore any exception raised by the lookup */
+    PyObject *exc2 = _PyErr_Occurred(tstate);
+    if (exc2 && !PyErr_GivenExceptionMatches(exc2, PyExc_KeyError)) {
+        _PyErr_WriteUnraisableMsg(warnmsg, NULL);
+    }
     _PyErr_SetRaisedException(tstate, exc);
 
 
     assert(ix >= 0 || value == NULL);
     return value;
 }
 
+PyObject *
+PyDict_GetItem(PyObject *op, PyObject *key)
+{
+    return dict_getitem(op, key,
+            "in PyDict_GetItem(); consider using "
+            "PyDict_GetItemWithError()");
+}
+
 Py_ssize_t
 _PyDict_LookupIndex(PyDictObject *mp, PyObject *key)
 {
@@ -3889,10 +3901,12 @@ PyDict_GetItemString(PyObject *v, const char *key)
     PyObject *kv, *rv;
     kv = PyUnicode_FromString(key);
     if (kv == NULL) {
-        PyErr_Clear();
+        _PyErr_WriteUnraisableMsg(
+            "in PyDict_GetItemString()", NULL);
         return NULL;
     }
     rv = PyDict_GetItem(v, kv);
+    rv = dict_getitem(v, kv, "in PyDict_GetItemString()");
     Py_DECREF(kv);
     return rv;
 }

@@ -904,26 +904,19 @@ PyObject_GetAttrString(PyObject *v, const char *name)
 }
 
 int
-PyObject_HasAttrString(PyObject *v, 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 *attr_name = PyUnicode_FromString(name);
-    if (attr_name == NULL) {
-        PyErr_Clear();
+PyObject_HasAttrString(PyObject *obj, const char *name)
+{
+    PyObject *dummy;
+    int rc = PyObject_GetOptionalAttrString(obj, name, &dummy);
+    if (rc < 0) {
+        _PyErr_WriteUnraisableMsg(
+            "in PyObject_HasAttrString(); consider using "
+            "PyObject_GetOptionalAttrString() or PyObject_GetAttrString()",
+            NULL);
         return 0;
     }
-    int ok = PyObject_HasAttr(v, attr_name);
-    Py_DECREF(attr_name);
-    return ok;
+    Py_XDECREF(dummy);
+    return rc;
 }
 
 int
@@ -1142,18 +1135,18 @@ PyObject_GetOptionalAttrString(PyObject *obj, const char *name, PyObject **resul
 }
 
 int
-PyObject_HasAttr(PyObject *v, PyObject *name)
-{
-    PyObject *res;
-    if (PyObject_GetOptionalAttr(v, name, &res) < 0) {
-        PyErr_Clear();
-        return 0;
-    }
-    if (res == NULL) {
+PyObject_HasAttr(PyObject *obj, PyObject *name)
+{
+    PyObject *dummy;
+    int rc = PyObject_GetOptionalAttr(obj, name, &dummy);
+    if (rc < 0) {
+        _PyErr_WriteUnraisableMsg(
+            "in PyObject_HasAttr(); consider using "
+            "PyObject_GetOptionalAttr() or PyObject_GetAttr()", NULL);
         return 0;
     }
-    Py_DECREF(res);
-    return 1;
+    Py_XDECREF(dummy);
+    return rc;
 }
 
 int

@@ -98,6 +98,9 @@ PySys_GetObject(const char *name)
     PyObject *value = _PySys_GetObject(tstate->interp, name);
     /* XXX Suppress a new exception if it was raised and restore
      * the old one. */
+    if (_PyErr_Occurred(tstate)) {
+        _PyErr_WriteUnraisableMsg("in PySys_GetObject()", NULL);
+    }
     _PyErr_SetRaisedException(tstate, exc);
     return value;
 }