bpo-37645: review fixes

miss-islington · Nov 5, 2019 · Jul 21, 2019 · Jul 21, 2019 · Aug 4, 2019 · Aug 18, 2019
commit fa2055690b052a0cf15c83722cd420e2e9adb38f
diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst
@@ -197,16 +197,6 @@ Object Protocol
       does not silently discard an active exception.
 
 
-.. c:function:: PyObject* _PyObject_FunctionStr(PyObject *func)
-
-   Return a user-friendly string representation of the function-like object
-   *func*. This returns ``func.__qualname__ + "()"`` if there is a
-   ``__qualname__`` attribute and ``str(func)`` otherwise.
-   Note that there is no check that *func* is actually callable.
-
-   .. versionadded:: 3.9
-
-
 .. c:function:: PyObject* PyObject_Bytes(PyObject *o)
 
    .. index:: builtin: bytes

diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py
@@ -209,11 +209,11 @@ def test_varargs7_kw(self):
         self.assertRaisesRegex(TypeError, msg, next, x=2)
 
     def test_varargs8_kw(self):
-        msg = r"^pack\(\) takes no keyword arguments$"
+        msg = r"^_struct[.]pack\(\) takes no keyword arguments$"
         self.assertRaisesRegex(TypeError, msg, struct.pack, x=2)
 
     def test_varargs9_kw(self):
-        msg = r"^pack_into\(\) takes no keyword arguments$"
+        msg = r"^_struct[.]pack_into\(\) takes no keyword arguments$"
         self.assertRaisesRegex(TypeError, msg, struct.pack_into, x=2)
 
     def test_varargs10_kw(self):

diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
@@ -1967,7 +1967,7 @@ def test_methods_in_c(self):
         # different error messages.
         set_add = set.add
 
-        expected_errmsg = "set.add() needs an argument"
+        expected_errmsg = "unbound method set.add() needs an argument"
 
         with self.assertRaises(TypeError) as cm:
             set_add()

diff --git a/Lib/test/test_extcall.py b/Lib/test/test_extcall.py
@@ -118,7 +118,7 @@
     >>> g(*Nothing())
     Traceback (most recent call last):
       ...
-    TypeError: g() argument after * must be an iterable, not Nothing
+    TypeError: test.test_extcall.g() argument after * must be an iterable, not Nothing
 
     >>> class Nothing:
     ...     def __len__(self): return 5
@@ -127,7 +127,7 @@
     >>> g(*Nothing())
     Traceback (most recent call last):
       ...
-    TypeError: g() argument after * must be an iterable, not Nothing
+    TypeError: test.test_extcall.g() argument after * must be an iterable, not Nothing
 
     >>> class Nothing():
     ...     def __len__(self): return 5
@@ -247,17 +247,17 @@
     >>> h(*h)
     Traceback (most recent call last):
       ...
-    TypeError: h() argument after * must be an iterable, not function
+    TypeError: test.test_extcall.h() argument after * must be an iterable, not function
 
     >>> h(1, *h)
     Traceback (most recent call last):
       ...
-    TypeError: h() argument after * must be an iterable, not function
+    TypeError: test.test_extcall.h() argument after * must be an iterable, not function
 
     >>> h(*[1], *h)
     Traceback (most recent call last):
       ...
-    TypeError: h() argument after * must be an iterable, not function
+    TypeError: test.test_extcall.h() argument after * must be an iterable, not function
 
     >>> dir(*h)
     Traceback (most recent call last):

diff --git a/Objects/descrobject.c b/Objects/descrobject.c
@@ -239,7 +239,7 @@ method_check_args(PyObject *func, PyObject *const *args, Py_ssize_t nargs, PyObj
         PyObject *funcstr = _PyObject_FunctionStr(func);
         if (funcstr != NULL) {
             PyErr_Format(PyExc_TypeError,
-                         "%U needs an argument", funcstr);
+                         "unbound method %U needs an argument", funcstr);
             Py_DECREF(funcstr);
         }
         return -1;

diff --git a/Objects/object.c b/Objects/object.c
@@ -665,28 +665,56 @@ PyObject_Bytes(PyObject *v)
 
 
 /*
-def _PyObject_FunctionStr(f):
+def _PyObject_FunctionStr(x):
     try:
-        return f.__qualname__ + "()"
-    except Exception:
-        return str(f)
+        qualname = x.__qualname__
+    except AttributeError:
+        return str(x)
+    try:
+        mod = x.__module__
+        if mod is not None and mod != 'builtins':
+            return f"{x.__module__}.{qualname}()"
+    except AttributeError:
+        pass
+    return qualname
 */
 PyObject *
-_PyObject_FunctionStr(PyObject *f)
+_PyObject_FunctionStr(PyObject *x)
 {
+    _Py_IDENTIFIER(__module__);
     _Py_IDENTIFIER(__qualname__);
-    PyObject *name = _PyObject_GetAttrId(f, &PyId___qualname__);
-    if (name != NULL) {
-        PyObject *res = PyUnicode_FromFormat("%S()", name);
-        Py_DECREF(name);
-        return res;
+    _Py_IDENTIFIER(builtins);
+    assert(!PyErr_Occurred());
+    PyObject *qualname;
+    int ret = _PyObject_LookupAttrId(x, &PyId___qualname__, &qualname);
+    if (qualname == NULL) {
+        if (ret < 0) {
+            return NULL;
+        }
+        return PyObject_Str(x);
+    }
+    PyObject *module;
+    PyObject *result = NULL;
+    ret = _PyObject_LookupAttrId(x, &PyId___module__, &module);
+    if (module != NULL && module != Py_None) {
+        PyObject *builtinsname = _PyUnicode_FromId(&PyId_builtins);
+        if (builtinsname == NULL) {
+            goto done;
+        }
+        ret = PyObject_RichCompareBool(module, builtinsname, Py_NE);
+        if (ret > 0) {
+            result = PyUnicode_FromFormat("%S.%S()", module, qualname);
+            goto done;
+        }
     }
-    /* __qualname__ lookup failed */
-    if (!PyErr_ExceptionMatches(PyExc_Exception)) {
-        return NULL;
+    else if (ret < 0) {
+        goto done;
     }
-    PyErr_Clear();
-    return PyObject_Str(f);
+    result = PyUnicode_FromFormat("%S()", qualname);
+done:
+    Py_DECREF(qualname);
+    Py_XDECREF(module);
+    return result;
 }
 
 /* For Python 3.0.1 and later, the old three-way comparison has been

diff --git a/Python/ceval.c b/Python/ceval.c
@@ -5341,8 +5341,9 @@ static int
 check_args_iterable(PyThreadState *tstate, PyObject *func, PyObject *args)
 {
     if (args->ob_type->tp_iter == NULL && !PySequence_Check(args)) {
-        /* check_args_iterable() may be called with a live exception,
-         * clear it. */
+        /* check_args_iterable() may be called with a live exception:
+         * clear it to prevent calling _PyObject_FunctionStr() with an
+         * exception set. */
         PyErr_Clear();
         PyObject *funcstr = _PyObject_FunctionStr(func);
         if (funcstr != NULL) {