From bf17d41826a8bb4bc1e34ba6345da98aac779e41 Mon Sep 17 00:00:00 2001 From: Jeroen Demeyer Date: Tue, 5 Nov 2019 16:48:04 +0100 Subject: bpo-37645: add new function _PyObject_FunctionStr() (GH-14890) Additional note: the `method_check_args` function in `Objects/descrobject.c` is written in such a way that it applies to all kinds of descriptors. In particular, a future re-implementation of `wrapper_descriptor` could use that code. CC @vstinner @encukou https://bugs.python.org/issue37645 Automerge-Triggered-By: @encukou --- Doc/c-api/object.rst | 1 + Include/cpython/object.h | 1 + Lib/test/test_call.py | 10 ++-- Lib/test/test_descr.py | 2 +- Lib/test/test_extcall.py | 38 +++++++------- Lib/test/test_unpack_ex.py | 10 ++-- .../C API/2019-07-21-21-08-47.bpo-37645.4DcUaI.rst | 2 + Objects/descrobject.c | 57 ++++++++++----------- Objects/methodobject.c | 37 +++++++------- Objects/object.c | 58 ++++++++++++++++++++++ Python/ceval.c | 49 +++++++++++------- 11 files changed, 171 insertions(+), 94 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2019-07-21-21-08-47.bpo-37645.4DcUaI.rst diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 9d11551..7d7a3be 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -196,6 +196,7 @@ Object Protocol This function now includes a debug assertion to help ensure that it does not silently discard an active exception. + .. c:function:: PyObject* PyObject_Bytes(PyObject *o) .. index:: builtin: bytes diff --git a/Include/cpython/object.h b/Include/cpython/object.h index fd4e771..75e5599 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -348,6 +348,7 @@ static inline void _Py_Dealloc_inline(PyObject *op) } #define _Py_Dealloc(op) _Py_Dealloc_inline(op) +PyAPI_FUNC(PyObject *) _PyObject_FunctionStr(PyObject *); /* Safely decref `op` and set `op` to `op2`. * diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index c233ba1..d178aa4 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -74,7 +74,7 @@ class CFunctionCallsErrorMessages(unittest.TestCase): self.assertRaisesRegex(TypeError, msg, bool, x=2) def test_varargs4_kw(self): - msg = r"^index\(\) takes no keyword arguments$" + msg = r"^list[.]index\(\) takes no keyword arguments$" self.assertRaisesRegex(TypeError, msg, [].index, x=2) def test_varargs5_kw(self): @@ -90,19 +90,19 @@ class CFunctionCallsErrorMessages(unittest.TestCase): 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): - msg = r"^index\(\) takes no keyword arguments$" + msg = r"^deque[.]index\(\) takes no keyword arguments$" self.assertRaisesRegex(TypeError, msg, collections.deque().index, x=2) def test_varargs11_kw(self): - msg = r"^pack\(\) takes no keyword arguments$" + msg = r"^Struct[.]pack\(\) takes no keyword arguments$" self.assertRaisesRegex(TypeError, msg, struct.Struct.pack, struct.Struct(""), x=2) def test_varargs12_kw(self): diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index 796e60a..d2e1218 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -1967,7 +1967,7 @@ order (MRO) for bases """ # different error messages. set_add = set.add - expected_errmsg = "descriptor 'add' of 'set' object 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 index d9dcb70..4edb668 100644 --- a/Lib/test/test_extcall.py +++ b/Lib/test/test_extcall.py @@ -52,15 +52,15 @@ Here we add keyword arguments >>> f(1, 2, **{'a': -1, 'b': 5}, **{'a': 4, 'c': 6}) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'a' + TypeError: test.test_extcall.f() got multiple values for keyword argument 'a' >>> f(1, 2, **{'a': -1, 'b': 5}, a=4, c=6) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'a' + TypeError: test.test_extcall.f() got multiple values for keyword argument 'a' >>> f(1, 2, a=3, **{'a': 4}, **{'a': 5}) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'a' + TypeError: test.test_extcall.f() got multiple values for keyword argument 'a' >>> f(1, 2, 3, *[4, 5], **{'a':6, 'b':7}) (1, 2, 3, 4, 5) {'a': 6, 'b': 7} >>> f(1, 2, 3, x=4, y=5, *(6, 7), **{'a':8, 'b': 9}) @@ -118,7 +118,7 @@ Verify clearing of SF bug #733667 >>> 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 @@ Verify clearing of SF bug #733667 >>> 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 @@ What about willful misconduct? >>> 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): @@ -268,38 +268,38 @@ What about willful misconduct? >>> nothing(*h) Traceback (most recent call last): ... - TypeError: NoneType object argument after * must be an iterable, \ + TypeError: None argument after * must be an iterable, \ not function >>> h(**h) Traceback (most recent call last): ... - TypeError: h() argument after ** must be a mapping, not function + TypeError: test.test_extcall.h() argument after ** must be a mapping, not function >>> h(**[]) Traceback (most recent call last): ... - TypeError: h() argument after ** must be a mapping, not list + TypeError: test.test_extcall.h() argument after ** must be a mapping, not list >>> h(a=1, **h) Traceback (most recent call last): ... - TypeError: h() argument after ** must be a mapping, not function + TypeError: test.test_extcall.h() argument after ** must be a mapping, not function >>> h(a=1, **[]) Traceback (most recent call last): ... - TypeError: h() argument after ** must be a mapping, not list + TypeError: test.test_extcall.h() argument after ** must be a mapping, not list >>> h(**{'a': 1}, **h) Traceback (most recent call last): ... - TypeError: h() argument after ** must be a mapping, not function + TypeError: test.test_extcall.h() argument after ** must be a mapping, not function >>> h(**{'a': 1}, **[]) Traceback (most recent call last): ... - TypeError: h() argument after ** must be a mapping, not list + TypeError: test.test_extcall.h() argument after ** must be a mapping, not list >>> dir(**h) Traceback (most recent call last): @@ -309,7 +309,7 @@ not function >>> nothing(**h) Traceback (most recent call last): ... - TypeError: NoneType object argument after ** must be a mapping, \ + TypeError: None argument after ** must be a mapping, \ not function >>> dir(b=1, **{'b': 1}) @@ -351,17 +351,17 @@ Test a kwargs mapping with duplicated keys. >>> g(**MultiDict([('x', 1), ('x', 2)])) Traceback (most recent call last): ... - TypeError: g() got multiple values for keyword argument 'x' + TypeError: test.test_extcall.g() got multiple values for keyword argument 'x' >>> g(a=3, **MultiDict([('x', 1), ('x', 2)])) Traceback (most recent call last): ... - TypeError: g() got multiple values for keyword argument 'x' + TypeError: test.test_extcall.g() got multiple values for keyword argument 'x' >>> g(**MultiDict([('a', 3)]), **MultiDict([('x', 1), ('x', 2)])) Traceback (most recent call last): ... - TypeError: g() got multiple values for keyword argument 'x' + TypeError: test.test_extcall.g() got multiple values for keyword argument 'x' Another helper function diff --git a/Lib/test/test_unpack_ex.py b/Lib/test/test_unpack_ex.py index 87fea59..46f70c2 100644 --- a/Lib/test/test_unpack_ex.py +++ b/Lib/test/test_unpack_ex.py @@ -236,27 +236,27 @@ Overridden parameters >>> f(x=5, **{'x': 3}, y=2) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'x' + TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' >>> f(**{'x': 3}, x=5, y=2) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'x' + TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' >>> f(**{'x': 3}, **{'x': 5}, y=2) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'x' + TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' >>> f(x=5, **{'x': 3}, **{'x': 2}) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument 'x' + TypeError: test.test_unpack_ex.f() got multiple values for keyword argument 'x' >>> f(**{1: 3}, **{1: 5}) Traceback (most recent call last): ... - TypeError: f() got multiple values for keyword argument '1' + TypeError: test.test_unpack_ex.f() got multiple values for keyword argument '1' Unpacking non-sequence diff --git a/Misc/NEWS.d/next/C API/2019-07-21-21-08-47.bpo-37645.4DcUaI.rst b/Misc/NEWS.d/next/C API/2019-07-21-21-08-47.bpo-37645.4DcUaI.rst new file mode 100644 index 0000000..2a6efaa --- /dev/null +++ b/Misc/NEWS.d/next/C API/2019-07-21-21-08-47.bpo-37645.4DcUaI.rst @@ -0,0 +1,2 @@ +Add :c:func:`_PyObject_FunctionStr` to get a user-friendly string representation +of a function-like object. Patch by Jeroen Demeyer. diff --git a/Objects/descrobject.c b/Objects/descrobject.c index dbab4cd..342b993 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -231,45 +231,38 @@ getset_set(PyGetSetDescrObject *descr, PyObject *obj, PyObject *value) * * First, common helpers */ -static const char * -get_name(PyObject *func) { - assert(PyObject_TypeCheck(func, &PyMethodDescr_Type)); - return ((PyMethodDescrObject *)func)->d_method->ml_name; -} - -typedef void (*funcptr)(void); - static inline int method_check_args(PyObject *func, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { assert(!PyErr_Occurred()); - assert(PyObject_TypeCheck(func, &PyMethodDescr_Type)); if (nargs < 1) { - PyErr_Format(PyExc_TypeError, - "descriptor '%.200s' of '%.100s' " - "object needs an argument", - get_name(func), PyDescr_TYPE(func)->tp_name); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + PyErr_Format(PyExc_TypeError, + "unbound method %U needs an argument", funcstr); + Py_DECREF(funcstr); + } return -1; } PyObject *self = args[0]; - if (!_PyObject_RealIsSubclass((PyObject *)Py_TYPE(self), - (PyObject *)PyDescr_TYPE(func))) - { - PyErr_Format(PyExc_TypeError, - "descriptor '%.200s' for '%.100s' objects " - "doesn't apply to a '%.100s' object", - get_name(func), PyDescr_TYPE(func)->tp_name, - Py_TYPE(self)->tp_name); + PyObject *dummy; + if (descr_check((PyDescrObject *)func, self, &dummy)) { return -1; } if (kwnames && PyTuple_GET_SIZE(kwnames)) { - PyErr_Format(PyExc_TypeError, - "%.200s() takes no keyword arguments", get_name(func)); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + PyErr_Format(PyExc_TypeError, + "%U takes no keyword arguments", funcstr); + Py_DECREF(funcstr); + } return -1; } return 0; } +typedef void (*funcptr)(void); + static inline funcptr method_enter_call(PyThreadState *tstate, PyObject *func) { @@ -387,8 +380,12 @@ method_vectorcall_NOARGS( return NULL; } if (nargs != 1) { - PyErr_Format(PyExc_TypeError, - "%.200s() takes no arguments (%zd given)", get_name(func), nargs-1); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + PyErr_Format(PyExc_TypeError, + "%U takes no arguments (%zd given)", funcstr, nargs-1); + Py_DECREF(funcstr); + } return NULL; } PyCFunction meth = (PyCFunction)method_enter_call(tstate, func); @@ -410,9 +407,13 @@ method_vectorcall_O( return NULL; } if (nargs != 2) { - PyErr_Format(PyExc_TypeError, - "%.200s() takes exactly one argument (%zd given)", - get_name(func), nargs-1); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + PyErr_Format(PyExc_TypeError, + "%U takes exactly one argument (%zd given)", + funcstr, nargs-1); + Py_DECREF(funcstr); + } return NULL; } PyCFunction meth = (PyCFunction)method_enter_call(tstate, func); diff --git a/Objects/methodobject.c b/Objects/methodobject.c index c780904..8f752c6 100644 --- a/Objects/methodobject.c +++ b/Objects/methodobject.c @@ -334,15 +334,6 @@ _PyCFunction_Fini(void) * * First, common helpers */ -static const char * -get_name(PyObject *func) -{ - assert(PyCFunction_Check(func)); - PyMethodDef *method = ((PyCFunctionObject *)func)->m_ml; - return method->ml_name; -} - -typedef void (*funcptr)(void); static inline int cfunction_check_kwargs(PyThreadState *tstate, PyObject *func, PyObject *kwnames) @@ -350,13 +341,19 @@ cfunction_check_kwargs(PyThreadState *tstate, PyObject *func, PyObject *kwnames) assert(!_PyErr_Occurred(tstate)); assert(PyCFunction_Check(func)); if (kwnames && PyTuple_GET_SIZE(kwnames)) { - _PyErr_Format(tstate, PyExc_TypeError, - "%.200s() takes no keyword arguments", get_name(func)); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + _PyErr_Format(tstate, PyExc_TypeError, + "%U takes no keyword arguments", funcstr); + Py_DECREF(funcstr); + } return -1; } return 0; } +typedef void (*funcptr)(void); + static inline funcptr cfunction_enter_call(PyThreadState *tstate, PyObject *func) { @@ -412,9 +409,12 @@ cfunction_vectorcall_NOARGS( } Py_ssize_t nargs = PyVectorcall_NARGS(nargsf); if (nargs != 0) { - _PyErr_Format(tstate, PyExc_TypeError, - "%.200s() takes no arguments (%zd given)", - get_name(func), nargs); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + _PyErr_Format(tstate, PyExc_TypeError, + "%U takes no arguments (%zd given)", funcstr, nargs); + Py_DECREF(funcstr); + } return NULL; } PyCFunction meth = (PyCFunction)cfunction_enter_call(tstate, func); @@ -436,9 +436,12 @@ cfunction_vectorcall_O( } Py_ssize_t nargs = PyVectorcall_NARGS(nargsf); if (nargs != 1) { - _PyErr_Format(tstate, PyExc_TypeError, - "%.200s() takes exactly one argument (%zd given)", - get_name(func), nargs); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + _PyErr_Format(tstate, PyExc_TypeError, + "%U takes exactly one argument (%zd given)", funcstr, nargs); + Py_DECREF(funcstr); + } return NULL; } PyCFunction meth = (PyCFunction)cfunction_enter_call(tstate, func); diff --git a/Objects/object.c b/Objects/object.c index 9536d46..3e61282 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -681,6 +681,64 @@ PyObject_Bytes(PyObject *v) return PyBytes_FromObject(v); } + +/* +def _PyObject_FunctionStr(x): + try: + 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 *x) +{ + _Py_IDENTIFIER(__module__); + _Py_IDENTIFIER(__qualname__); + _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) { + // error + goto done; + } + if (ret > 0) { + result = PyUnicode_FromFormat("%S.%S()", module, qualname); + goto done; + } + } + else if (ret < 0) { + goto done; + } + 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 completely removed in favour of rich comparisons. PyObject_Compare() and PyObject_Cmp() are gone, and the builtin cmp function no longer exists. diff --git a/Python/ceval.c b/Python/ceval.c index 9019c78..4d8f1b9 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -5351,12 +5351,17 @@ static int check_args_iterable(PyThreadState *tstate, PyObject *func, PyObject *args) { if (args->ob_type->tp_iter == NULL && !PySequence_Check(args)) { - _PyErr_Format(tstate, PyExc_TypeError, - "%.200s%.200s argument after * " - "must be an iterable, not %.200s", - PyEval_GetFuncName(func), - PyEval_GetFuncDesc(func), - args->ob_type->tp_name); + /* 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) { + _PyErr_Format(tstate, PyExc_TypeError, + "%U argument after * must be an iterable, not %.200s", + funcstr, Py_TYPE(args)->tp_name); + Py_DECREF(funcstr); + } return -1; } return 0; @@ -5372,24 +5377,30 @@ format_kwargs_error(PyThreadState *tstate, PyObject *func, PyObject *kwargs) * is not a mapping. */ if (_PyErr_ExceptionMatches(tstate, PyExc_AttributeError)) { - _PyErr_Format(tstate, PyExc_TypeError, - "%.200s%.200s argument after ** " - "must be a mapping, not %.200s", - PyEval_GetFuncName(func), - PyEval_GetFuncDesc(func), - kwargs->ob_type->tp_name); + PyErr_Clear(); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + _PyErr_Format( + tstate, PyExc_TypeError, + "%U argument after ** must be a mapping, not %.200s", + funcstr, Py_TYPE(kwargs)->tp_name); + Py_DECREF(funcstr); + } } else if (_PyErr_ExceptionMatches(tstate, PyExc_KeyError)) { PyObject *exc, *val, *tb; _PyErr_Fetch(tstate, &exc, &val, &tb); if (val && PyTuple_Check(val) && PyTuple_GET_SIZE(val) == 1) { - PyObject *key = PyTuple_GET_ITEM(val, 0); - _PyErr_Format(tstate, PyExc_TypeError, - "%.200s%.200s got multiple " - "values for keyword argument '%S'", - PyEval_GetFuncName(func), - PyEval_GetFuncDesc(func), - key); + PyErr_Clear(); + PyObject *funcstr = _PyObject_FunctionStr(func); + if (funcstr != NULL) { + PyObject *key = PyTuple_GET_ITEM(val, 0); + _PyErr_Format( + tstate, PyExc_TypeError, + "%U got multiple values for keyword argument '%S'", + funcstr, key); + Py_DECREF(funcstr); + } Py_XDECREF(exc); Py_XDECREF(val); Py_XDECREF(tb); -- cgit v0.12