bpo-32571: Avoid raising unneeded AttributeError and silencing it in C code (GH-5222)

Add two new private APIs: _PyObject_LookupAttr() and _PyObject_LookupAttrId()

7 files changed, 106 insertions, 131 deletions

diff --git a/Objects/abstract.c b/Objects/abstract.c
index a68253bdb4..93eda62a27 100644
--- a/Objects/abstract.c
+++ b/Objects/abstract.c
@@ -171,16 +171,14 @@ PyObject_GetItem(PyObject *o, PyObject *key)
     if (PyType_Check(o)) {
         PyObject *meth, *result, *stack[1] = {key};
         _Py_IDENTIFIER(__class_getitem__);
-        meth = _PyObject_GetAttrId(o, &PyId___class_getitem__);
+        if (_PyObject_LookupAttrId(o, &PyId___class_getitem__, &meth) < 0) {
+            return NULL;
+        }
         if (meth) {
             result = _PyObject_FastCall(meth, stack, 1);
             Py_DECREF(meth);
             return result;
         }
-        else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            return NULL;
-        }
-        PyErr_Clear();
     }
 
     return type_error("'%.200s' object is not subscriptable", o);
@@ -2268,14 +2266,9 @@ abstract_get_bases(PyObject *cls)
     PyObject *bases;
 
     Py_ALLOW_RECURSION
-    bases = _PyObject_GetAttrId(cls, &PyId___bases__);
+    (void)_PyObject_LookupAttrId(cls, &PyId___bases__, &bases);
     Py_END_ALLOW_RECURSION
-    if (bases == NULL) {
-        if (PyErr_ExceptionMatches(PyExc_AttributeError))
-            PyErr_Clear();
-        return NULL;
-    }
-    if (!PyTuple_Check(bases)) {
+    if (bases != NULL && !PyTuple_Check(bases)) {
         Py_DECREF(bases);
         return NULL;
     }
@@ -2338,26 +2331,23 @@ static int
 recursive_isinstance(PyObject *inst, PyObject *cls)
 {
     PyObject *icls;
-    int retval = 0;
+    int retval;
     _Py_IDENTIFIER(__class__);
 
     if (PyType_Check(cls)) {
         retval = PyObject_TypeCheck(inst, (PyTypeObject *)cls);
         if (retval == 0) {
-            PyObject *c = _PyObject_GetAttrId(inst, &PyId___class__);
-            if (c == NULL) {
-                if (PyErr_ExceptionMatches(PyExc_AttributeError))
-                    PyErr_Clear();
-                else
-                    retval = -1;
-            }
-            else {
-                if (c != (PyObject *)(inst->ob_type) &&
-                    PyType_Check(c))
+            retval = _PyObject_LookupAttrId(inst, &PyId___class__, &icls);
+            if (icls != NULL) {
+                if (icls != (PyObject *)(inst->ob_type) && PyType_Check(icls)) {
                     retval = PyType_IsSubtype(
-                        (PyTypeObject *)c,
+                        (PyTypeObject *)icls,
                         (PyTypeObject *)cls);
-                Py_DECREF(c);
+                }
+                else {
+                    retval = 0;
+                }
+                Py_DECREF(icls);
             }
         }
     }
@@ -2365,14 +2355,8 @@ recursive_isinstance(PyObject *inst, PyObject *cls)
         if (!check_class(cls,
             "isinstance() arg 2 must be a type or tuple of types"))
             return -1;
-        icls = _PyObject_GetAttrId(inst, &PyId___class__);
-        if (icls == NULL) {
-            if (PyErr_ExceptionMatches(PyExc_AttributeError))
-                PyErr_Clear();
-            else
-                retval = -1;
-        }
-        else {
+        retval = _PyObject_LookupAttrId(inst, &PyId___class__, &icls);
+        if (icls != NULL) {
             retval = abstract_issubclass(icls, cls);
             Py_DECREF(icls);
         }
diff --git a/Objects/classobject.c b/Objects/classobject.c
index e88c95cbfb..3dc23b796c 100644
--- a/Objects/classobject.c
+++ b/Objects/classobject.c
@@ -246,21 +246,14 @@ method_repr(PyMethodObject *a)
 {
     PyObject *self = a->im_self;
     PyObject *func = a->im_func;
-    PyObject *funcname = NULL, *result = NULL;
+    PyObject *funcname, *result;
     const char *defname = "?";
 
-    funcname = _PyObject_GetAttrId(func, &PyId___qualname__);
-    if (funcname == NULL) {
-        if (!PyErr_ExceptionMatches(PyExc_AttributeError))
-            return NULL;
-        PyErr_Clear();
-
-        funcname = _PyObject_GetAttrId(func, &PyId___name__);
-        if (funcname == NULL) {
-            if (!PyErr_ExceptionMatches(PyExc_AttributeError))
-                return NULL;
-            PyErr_Clear();
-        }
+    if (_PyObject_LookupAttrId(func, &PyId___qualname__, &funcname) < 0 ||
+        (funcname == NULL &&
+         _PyObject_LookupAttrId(func, &PyId___name__, &funcname) < 0))
+    {
+        return NULL;
     }
 
     if (funcname != NULL && !PyUnicode_Check(funcname)) {
@@ -542,7 +535,7 @@ static PyObject *
 instancemethod_repr(PyObject *self)
 {
     PyObject *func = PyInstanceMethod_Function(self);
-    PyObject *funcname = NULL , *result = NULL;
+    PyObject *funcname, *result;
     const char *defname = "?";
 
     if (func == NULL) {
@@ -550,13 +543,10 @@ instancemethod_repr(PyObject *self)
         return NULL;
     }
 
-    funcname = _PyObject_GetAttrId(func, &PyId___name__);
-    if (funcname == NULL) {
-        if (!PyErr_ExceptionMatches(PyExc_AttributeError))
-            return NULL;
-        PyErr_Clear();
+    if (_PyObject_LookupAttrId(func, &PyId___name__, &funcname) < 0) {
+        return NULL;
     }
-    else if (!PyUnicode_Check(funcname)) {
+    if (funcname != NULL && !PyUnicode_Check(funcname)) {
         Py_DECREF(funcname);
         funcname = NULL;
     }
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 259fa25565..0d25739f72 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -2234,17 +2234,16 @@ dict_update_common(PyObject *self, PyObject *args, PyObject *kwds,
     }
     else if (arg != NULL) {
         _Py_IDENTIFIER(keys);
-        PyObject *func = _PyObject_GetAttrId(arg, &PyId_keys);
-        if (func != NULL) {
+        PyObject *func;
+        if (_PyObject_LookupAttrId(arg, &PyId_keys, &func) < 0) {
+            result = -1;
+        }
+        else if (func != NULL) {
             Py_DECREF(func);
             result = PyDict_Merge(self, arg, 1);
         }
-        else if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            PyErr_Clear();
-            result = PyDict_MergeFromSeq2(self, arg, 1);
-        }
         else {
-            result = -1;
+            result = PyDict_MergeFromSeq2(self, arg, 1);
         }
     }
 
diff --git a/Objects/genobject.c b/Objects/genobject.c
index 0a34c1f6a9..7baffa7fc0 100644
--- a/Objects/genobject.c
+++ b/Objects/genobject.c
@@ -352,13 +352,11 @@ gen_close_iter(PyObject *yf)
             return -1;
     }
     else {
-        PyObject *meth = _PyObject_GetAttrId(yf, &PyId_close);
-        if (meth == NULL) {
-            if (!PyErr_ExceptionMatches(PyExc_AttributeError))
-                PyErr_WriteUnraisable(yf);
-            PyErr_Clear();
+        PyObject *meth;
+        if (_PyObject_LookupAttrId(yf, &PyId_close, &meth) < 0) {
+            PyErr_WriteUnraisable(yf);
         }
-        else {
+        if (meth) {
             retval = _PyObject_CallNoArg(meth);
             Py_DECREF(meth);
             if (retval == NULL)
@@ -471,13 +469,12 @@ _gen_throw(PyGenObject *gen, int close_on_genexit,
             gen->gi_running = 0;
         } else {
             /* `yf` is an iterator or a coroutine-like object. */
-            PyObject *meth = _PyObject_GetAttrId(yf, &PyId_throw);
+            PyObject *meth;
+            if (_PyObject_LookupAttrId(yf, &PyId_throw, &meth) < 0) {
+                Py_DECREF(yf);
+                return NULL;
+            }
             if (meth == NULL) {
-                if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
-                    Py_DECREF(yf);
-                    return NULL;
-                }
-                PyErr_Clear();
                 Py_DECREF(yf);
                 goto throw_here;
             }
diff --git a/Objects/object.c b/Objects/object.c
index 8cec6e2f12..7b2adbea1a 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -817,16 +817,11 @@ _PyObject_IsAbstract(PyObject *obj)
     if (obj == NULL)
         return 0;
 
-    isabstract = _PyObject_GetAttrId(obj, &PyId___isabstractmethod__);
-    if (isabstract == NULL) {
-        if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            PyErr_Clear();
-            return 0;
-        }
-        return -1;
+    res = _PyObject_LookupAttrId(obj, &PyId___isabstractmethod__, &isabstract);
+    if (res > 0) {
+        res = PyObject_IsTrue(isabstract);
+        Py_DECREF(isabstract);
     }
-    res = PyObject_IsTrue(isabstract);
-    Py_DECREF(isabstract);
     return res;
 }
 
@@ -888,47 +883,74 @@ PyObject_GetAttr(PyObject *v, PyObject *name)
     return NULL;
 }
 
-PyObject *
-_PyObject_GetAttrWithoutError(PyObject *v, PyObject *name)
+int
+_PyObject_LookupAttr(PyObject *v, PyObject *name, PyObject **result)
 {
     PyTypeObject *tp = Py_TYPE(v);
-    PyObject *ret = NULL;
 
     if (!PyUnicode_Check(name)) {
         PyErr_Format(PyExc_TypeError,
                      "attribute name must be string, not '%.200s'",
                      name->ob_type->tp_name);
-        return NULL;
+        *result = NULL;
+        return -1;
     }
 
     if (tp->tp_getattro == PyObject_GenericGetAttr) {
-        return _PyObject_GenericGetAttrWithDict(v, name, NULL, 1);
+        *result = _PyObject_GenericGetAttrWithDict(v, name, NULL, 1);
+        if (*result != NULL) {
+            return 1;
+        }
+        if (PyErr_Occurred()) {
+            return -1;
+        }
+        return 0;
     }
     if (tp->tp_getattro != NULL) {
-        ret = (*tp->tp_getattro)(v, name);
+        *result = (*tp->tp_getattro)(v, name);
     }
     else if (tp->tp_getattr != NULL) {
         const char *name_str = PyUnicode_AsUTF8(name);
-        if (name_str == NULL)
-            return NULL;
-        ret = (*tp->tp_getattr)(v, (char *)name_str);
+        if (name_str == NULL) {
+            *result = NULL;
+            return -1;
+        }
+        *result = (*tp->tp_getattr)(v, (char *)name_str);
     }
-    if (ret == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) {
-        PyErr_Clear();
+    if (*result != NULL) {
+        return 1;
     }
-    return ret;
+    if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
+        return -1;
+    }
+    PyErr_Clear();
+    return 0;
+}
+
+int
+_PyObject_LookupAttrId(PyObject *v, _Py_Identifier *name, PyObject **result)
+{
+    PyObject *oname = _PyUnicode_FromId(name); /* borrowed */
+    if (!oname) {
+        *result = NULL;
+        return -1;
+    }
+    return  _PyObject_LookupAttr(v, oname, result);
 }
 
 int
 PyObject_HasAttr(PyObject *v, PyObject *name)
 {
-    PyObject *res = _PyObject_GetAttrWithoutError(v, name);
-    if (res != NULL) {
-        Py_DECREF(res);
-        return 1;
+    PyObject *res;
+    if (_PyObject_LookupAttr(v, name, &res) < 0) {
+        PyErr_Clear();
+        return 0;
     }
-    PyErr_Clear();
-    return 0;
+    if (res == NULL) {
+        return 0;
+    }
+    Py_DECREF(res);
+    return 1;
 }
 
 int
diff --git a/Objects/odictobject.c b/Objects/odictobject.c
index 218aa2c5d9..bf19fedb07 100644
--- a/Objects/odictobject.c
+++ b/Objects/odictobject.c
@@ -2359,7 +2359,10 @@ mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs)
             goto handle_kwargs;
         }
 
-        func = _PyObject_GetAttrId(other, &PyId_keys);
+        if (_PyObject_LookupAttrId(other, &PyId_keys, &func) < 0) {
+            Py_DECREF(other);
+            return NULL;
+        }
         if (func != NULL) {
             PyObject *keys, *iterator, *key;
             keys = _PyObject_CallNoArg(func);
@@ -2391,15 +2394,11 @@ mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs)
                 return NULL;
             goto handle_kwargs;
         }
-        else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
+
+        if (_PyObject_LookupAttrId(other, &PyId_items, &func) < 0) {
             Py_DECREF(other);
             return NULL;
         }
-        else {
-            PyErr_Clear();
-        }
-
-        func = _PyObject_GetAttrId(other, &PyId_items);
         if (func != NULL) {
             PyObject *items;
             Py_DECREF(other);
@@ -2413,13 +2412,6 @@ mutablemapping_update(PyObject *self, PyObject *args, PyObject *kwargs)
                 return NULL;
             goto handle_kwargs;
         }
-        else if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            Py_DECREF(other);
-            return NULL;
-        }
-        else {
-            PyErr_Clear();
-        }
 
         res = mutablemapping_add_pairs(self, other);
         Py_DECREF(other);
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 0c8f0adfb7..a22173878e 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -2403,7 +2403,9 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds)
             if (PyType_Check(tmp)) {
                 continue;
             }
-            tmp = _PyObject_GetAttrId(tmp, &PyId___mro_entries__);
+            if (_PyObject_LookupAttrId(tmp, &PyId___mro_entries__, &tmp) < 0) {
+                return NULL;
+            }
             if (tmp != NULL) {
                 PyErr_SetString(PyExc_TypeError,
                                 "type() doesn't support MRO entry resolution; "
@@ -2411,12 +2413,6 @@ type_new(PyTypeObject *metatype, PyObject *args, PyObject *kwds)
                 Py_DECREF(tmp);
                 return NULL;
             }
-            else if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
-                PyErr_Clear();
-            }
-            else {
-                return NULL;
-            }
         }
         /* Search the bases for the proper metatype to deal with this: */
         winner = _PyType_CalculateMetaclass(metatype, bases);
@@ -4099,15 +4095,12 @@ _PyObject_GetState(PyObject *obj, int required)
     PyObject *getstate;
     _Py_IDENTIFIER(__getstate__);
 
-    getstate = _PyObject_GetAttrId(obj, &PyId___getstate__);
+    if (_PyObject_LookupAttrId(obj, &PyId___getstate__, &getstate) < 0) {
+        return NULL;
+    }
     if (getstate == NULL) {
         PyObject *slotnames;
 
-        if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
-            return NULL;
-        }
-        PyErr_Clear();
-
         if (required && obj->ob_type->tp_itemsize) {
             PyErr_Format(PyExc_TypeError,
                          "can't pickle %.200s objects",
@@ -4174,14 +4167,12 @@ _PyObject_GetState(PyObject *obj, int required)
 
                 name = PyList_GET_ITEM(slotnames, i);
                 Py_INCREF(name);
-                value = PyObject_GetAttr(obj, name);
+                if (_PyObject_LookupAttr(obj, name, &value) < 0) {
+                    goto error;
+                }
                 if (value == NULL) {
                     Py_DECREF(name);
-                    if (!PyErr_ExceptionMatches(PyExc_AttributeError)) {
-                        goto error;
-                    }
                     /* It is not an error if the attribute is not present. */
-                    PyErr_Clear();
                 }
                 else {
                     int err = PyDict_SetItem(slots, name, value);