From 0d77454c7619b24f3670e521b8a5cec7f4059b73 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Sun, 14 Jul 2019 17:38:22 +0200 Subject: [PATCH 01/11] Test METH_* functions using dedicated C API functions --- Lib/test/test_call.py | 129 ++++++++++++++++++++++++++++++++++++-- Modules/_testcapimodule.c | 59 +++++++++++++++++ 2 files changed, 184 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index 0bff7ded4670f1..e3e043178da68c 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -27,13 +27,16 @@ def fn(**kw): self.assertEqual(list(res.items()), expected) -# The test cases here cover several paths through the function calling -# code. They depend on the METH_XXX flag that is used to define a C -# function, which can't be verified from Python. If the METH_XXX decl -# for a C function changes, these tests may not cover the right paths. +# The test cases here covered several paths through the function calling +# code. They depended on the C function being defined with the METH_VARARGS +# or METH_OLDARGS flag, but all these functions have since changed these +# flags. +# Nevertheless, the tests should still pass, so we keep them. class CFunctionCalls(unittest.TestCase): + # {}.__contains__ + def test_varargs0(self): self.assertRaises(TypeError, {}.__contains__) @@ -66,6 +69,8 @@ def test_varargs1_kw(self): def test_varargs2_kw(self): self.assertRaises(TypeError, {}.__contains__, x=2, y=2) + # {}.keys + def test_oldargs0_0(self): {}.keys() @@ -108,6 +113,8 @@ def test_oldargs0_1_kw(self): def test_oldargs0_2_kw(self): self.assertRaises(TypeError, {}.keys, x=2, y=2) + # [].count + def test_oldargs1_0(self): self.assertRaises(TypeError, [].count) @@ -289,6 +296,120 @@ def test_oldargs1_2_kw(self): self.assertRaisesRegex(TypeError, msg, [].count, x=2, y=2) + +@cpython_only +class TestCallingCnventions(unittest.TestCase): + """Test calling using the various C calling conventions (METH_*)""" + + # This uses dedicated C functons whose METH_* flags shouldn't change. + + def test_varargs(self): + self.assertEqual(_testcapi.meth_varargs(1, 2, 3), (1, 2, 3)) + + def test_varargs_ext(self): + self.assertEqual(_testcapi.meth_varargs(*(1, 2, 3)), (1, 2, 3)) + + def test_varargs_error_kw(self): + msg = r"meth_varargs\(\) takes no keyword arguments" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_varargs(k=1), + ) + + def test_varargs_keywords(self): + self.assertEqual( + _testcapi.meth_varargs_keywords(1, 2, a=3, b=4), + ((1, 2), {'a': 3, 'b': 4}) + ) + + def test_o(self): + self.assertEqual(_testcapi.meth_o(1), 1) + + def test_o_ext(self): + self.assertEqual(_testcapi.meth_o(*[1]), 1) + + def test_o_error_no_arg(self): + msg = r"meth_o\(\) takes exactly one argument \(0 given\)" + self.assertRaisesRegex(TypeError, msg, _testcapi.meth_o) + + def test_o_error_two_args(self): + msg = r"meth_o\(\) takes exactly one argument \(2 given\)" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_o(1, 2), + ) + + def test_o_error_ext(self): + msg = r"meth_o\(\) takes exactly one argument \(3 given\)" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_o(*(1, 2, 3)), + ) + + def test_o_error_kw(self): + msg = r"meth_o\(\) takes no keyword arguments" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_o(k=1), + ) + + def test_o_error_arg_kw(self): + msg = r"meth_o\(\) takes no keyword arguments" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_o(k=1), + ) + + def test_noargs(self): + self.assertEqual(_testcapi.meth_noargs(), None) + + def test_noargs_ext(self): + self.assertEqual(_testcapi.meth_noargs(*[]), None) + + def test_noargs_error_arg(self): + msg = r"meth_noargs\(\) takes no arguments \(1 given\)" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_noargs(1), + ) + + def test_noargs_error_arg2(self): + msg = r"meth_noargs\(\) takes no arguments \(2 given\)" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_noargs(1, 2), + ) + + def test_noargs_error_ext(self): + msg = r"meth_noargs\(\) takes no arguments \(3 given\)" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_noargs(*(1, 2, 3)), + ) + + def test_noargs_error_kw(self): + msg = r"meth_noargs\(\) takes no keyword arguments" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_noargs(k=1), + ) + + def test_fastcall(self): + self.assertEqual(_testcapi.meth_fastcall(1, 2, 3), (1, 2, 3)) + + def test_fastcall_ext(self): + self.assertEqual(_testcapi.meth_fastcall(*(1, 2, 3)), (1, 2, 3)) + + def test_fastcall_error_kw(self): + msg = r"meth_fastcall\(\) takes no keyword arguments" + self.assertRaisesRegex( + TypeError, msg, lambda: _testcapi.meth_fastcall(k=1), + ) + + def test_fastcall_keywords(self): + self.assertEqual( + _testcapi.meth_fastcall_keywords(1, 2, a=3, b=4), + ((1, 2), {'a': 3, 'b': 4}) + ) + + def test_fastcall_keywords_ext(self): + self.assertEqual( + _testcapi.meth_fastcall_keywords(*(1, 2), a=3, b=4), + ((1, 2), {'a': 3, 'b': 4}) + ) + + def pyfunc(arg1, arg2): return [arg1, arg2] diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 147008b0a3dc4c..3d1bbbcfa91c08 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5176,6 +5176,59 @@ sequence_getitem(PyObject *self, PyObject *args) } +static PyObject* +meth_varargs(PyObject* self, PyObject* args) +{ + Py_INCREF(args); + return args; +} + +static PyObject* +meth_varargs_keywords(PyObject* self, PyObject* args, PyObject* kwargs) +{ + return Py_BuildValue("OO", args, kwargs); +} + +static PyObject* +meth_o(PyObject* self, PyObject* obj) +{ + Py_INCREF(obj); + return obj; +} + +static PyObject* +meth_noargs(PyObject* self, PyObject* ignored) +{ + Py_RETURN_NONE; +} + +static PyObject* +meth_fastcall(PyObject* self, PyObject** args, Py_ssize_t nargs) +{ + PyObject *tuple = PyTuple_New(nargs); + if (tuple == NULL) { + return NULL; + } + for (Py_ssize_t i=0; i < nargs; i++) { + Py_INCREF(args[i]); + PyTuple_SET_ITEM(tuple, i, args[i]); + } + return tuple; +} + +static PyObject* +meth_fastcall_keywords(PyObject* self, PyObject** args, + Py_ssize_t nargs, PyObject* kwargs) +{ + PyObject *pyargs = meth_fastcall(self, args, nargs); + if (pyargs == NULL) { + return NULL; + } + PyObject *pykwargs = _PyObject_Vectorcall((PyObject*)&PyDict_Type, + args + nargs, 0, kwargs); + return Py_BuildValue("NN", pyargs, pykwargs); +} + static PyMethodDef TestMethods[] = { {"raise_exception", raise_exception, METH_VARARGS}, {"raise_memoryerror", raise_memoryerror, METH_NOARGS}, @@ -5426,6 +5479,12 @@ static PyMethodDef TestMethods[] = { #endif {"write_unraisable_exc", test_write_unraisable_exc, METH_VARARGS}, {"sequence_getitem", sequence_getitem, METH_VARARGS}, + {"meth_varargs", meth_varargs, METH_VARARGS}, + {"meth_varargs_keywords", (PyCFunction)(void(*)(void))meth_varargs_keywords, METH_VARARGS|METH_KEYWORDS}, + {"meth_o", meth_o, METH_O}, + {"meth_noargs", meth_noargs, METH_NOARGS}, + {"meth_fastcall", (PyCFunction)(void(*)(void))meth_fastcall, METH_FASTCALL}, + {"meth_fastcall_keywords", (PyCFunction)(void(*)(void))meth_fastcall_keywords, METH_FASTCALL|METH_KEYWORDS}, {NULL, NULL} /* sentinel */ }; From 9534a804e99eb29317a74f1e8decd0f38f061088 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 15 Jul 2019 10:25:49 +0200 Subject: [PATCH 02/11] Test instances, class and static methods --- Lib/test/test_call.py | 123 +++++++++++++++++++++++++++++--------- Modules/_testcapimodule.c | 119 ++++++++++++++++++++++++++++++++---- 2 files changed, 204 insertions(+), 38 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index e3e043178da68c..a17ff3b4d7e779 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -298,118 +298,185 @@ def test_oldargs1_2_kw(self): @cpython_only -class TestCallingCnventions(unittest.TestCase): - """Test calling using the various C calling conventions (METH_*)""" +class TestCallingConventions(unittest.TestCase): + """Test calling using the various C calling conventions (METH_*) - # This uses dedicated C functons whose METH_* flags shouldn't change. + Subclasses test several kinds of functions (module-level, methods, + class methods static methods) using these attributes: + obj: the object that contains tested functions (as attributes) + expected_self: expected "self" argument to the C function + + The base class tests module-level functions. + """ + + def setUp(self): + self.obj = self.expected_self = _testcapi def test_varargs(self): - self.assertEqual(_testcapi.meth_varargs(1, 2, 3), (1, 2, 3)) + self.assertEqual( + self.obj.meth_varargs(1, 2, 3), + (self.expected_self, (1, 2, 3)), + ) def test_varargs_ext(self): - self.assertEqual(_testcapi.meth_varargs(*(1, 2, 3)), (1, 2, 3)) + self.assertEqual( + self.obj.meth_varargs(*(1, 2, 3)), + (self.expected_self, (1, 2, 3)), + ) def test_varargs_error_kw(self): msg = r"meth_varargs\(\) takes no keyword arguments" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_varargs(k=1), + TypeError, msg, lambda: self.obj.meth_varargs(k=1), ) def test_varargs_keywords(self): self.assertEqual( - _testcapi.meth_varargs_keywords(1, 2, a=3, b=4), - ((1, 2), {'a': 3, 'b': 4}) + self.obj.meth_varargs_keywords(1, 2, a=3, b=4), + (self.expected_self, (1, 2), {'a': 3, 'b': 4}) + ) + + def test_varargs_keywords_ext(self): + self.assertEqual( + self.obj.meth_varargs_keywords(*[1, 2], **{'a': 3, 'b': 4}), + (self.expected_self, (1, 2), {'a': 3, 'b': 4}) ) def test_o(self): - self.assertEqual(_testcapi.meth_o(1), 1) + self.assertEqual(self.obj.meth_o(1), (self.expected_self, 1)) def test_o_ext(self): - self.assertEqual(_testcapi.meth_o(*[1]), 1) + self.assertEqual(self.obj.meth_o(*[1]), (self.expected_self, 1)) def test_o_error_no_arg(self): msg = r"meth_o\(\) takes exactly one argument \(0 given\)" - self.assertRaisesRegex(TypeError, msg, _testcapi.meth_o) + self.assertRaisesRegex(TypeError, msg, self.obj.meth_o) def test_o_error_two_args(self): msg = r"meth_o\(\) takes exactly one argument \(2 given\)" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_o(1, 2), + TypeError, msg, lambda: self.obj.meth_o(1, 2), ) def test_o_error_ext(self): msg = r"meth_o\(\) takes exactly one argument \(3 given\)" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_o(*(1, 2, 3)), + TypeError, msg, lambda: self.obj.meth_o(*(1, 2, 3)), ) def test_o_error_kw(self): msg = r"meth_o\(\) takes no keyword arguments" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_o(k=1), + TypeError, msg, lambda: self.obj.meth_o(k=1), ) def test_o_error_arg_kw(self): msg = r"meth_o\(\) takes no keyword arguments" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_o(k=1), + TypeError, msg, lambda: self.obj.meth_o(k=1), ) def test_noargs(self): - self.assertEqual(_testcapi.meth_noargs(), None) + self.assertEqual(self.obj.meth_noargs(), self.expected_self) def test_noargs_ext(self): - self.assertEqual(_testcapi.meth_noargs(*[]), None) + self.assertEqual(self.obj.meth_noargs(*[]), self.expected_self) def test_noargs_error_arg(self): msg = r"meth_noargs\(\) takes no arguments \(1 given\)" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_noargs(1), + TypeError, msg, lambda: self.obj.meth_noargs(1), ) def test_noargs_error_arg2(self): msg = r"meth_noargs\(\) takes no arguments \(2 given\)" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_noargs(1, 2), + TypeError, msg, lambda: self.obj.meth_noargs(1, 2), ) def test_noargs_error_ext(self): msg = r"meth_noargs\(\) takes no arguments \(3 given\)" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_noargs(*(1, 2, 3)), + TypeError, msg, lambda: self.obj.meth_noargs(*(1, 2, 3)), ) def test_noargs_error_kw(self): msg = r"meth_noargs\(\) takes no keyword arguments" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_noargs(k=1), + TypeError, msg, lambda: self.obj.meth_noargs(k=1), ) def test_fastcall(self): - self.assertEqual(_testcapi.meth_fastcall(1, 2, 3), (1, 2, 3)) + self.assertEqual( + self.obj.meth_fastcall(1, 2, 3), + (self.expected_self, (1, 2, 3)), + ) def test_fastcall_ext(self): - self.assertEqual(_testcapi.meth_fastcall(*(1, 2, 3)), (1, 2, 3)) + self.assertEqual( + self.obj.meth_fastcall(*(1, 2, 3)), + (self.expected_self, (1, 2, 3)), + ) def test_fastcall_error_kw(self): msg = r"meth_fastcall\(\) takes no keyword arguments" self.assertRaisesRegex( - TypeError, msg, lambda: _testcapi.meth_fastcall(k=1), + TypeError, msg, lambda: self.obj.meth_fastcall(k=1), ) def test_fastcall_keywords(self): self.assertEqual( - _testcapi.meth_fastcall_keywords(1, 2, a=3, b=4), - ((1, 2), {'a': 3, 'b': 4}) + self.obj.meth_fastcall_keywords(1, 2, a=3, b=4), + (self.expected_self, (1, 2), {'a': 3, 'b': 4}) ) def test_fastcall_keywords_ext(self): self.assertEqual( - _testcapi.meth_fastcall_keywords(*(1, 2), a=3, b=4), - ((1, 2), {'a': 3, 'b': 4}) + self.obj.meth_fastcall_keywords(*(1, 2), **{'a': 3, 'b': 4}), + (self.expected_self, (1, 2), {'a': 3, 'b': 4}) ) +@cpython_only +class TestCallingConventionsFunction(TestCallingConventions): + """Test calling module-level functions using various calling conventions""" + + + +@cpython_only +class TestCallingConventionsInstance(TestCallingConventions): + """Test calling instance methods using various calling conventions""" + + def setUp(self): + self.obj = self.expected_self = _testcapi.MethInstance() + + +@cpython_only +class TestCallingConventionsClass(TestCallingConventions): + """Test calling class methods using various calling conventions""" + + def setUp(self): + self.obj = self.expected_self = _testcapi.MethClass + + +@cpython_only +class TestCallingConventionsClassInstance(TestCallingConventions): + """Test calling class methods on instance""" + + def setUp(self): + self.obj = _testcapi.MethClass() + self.expected_self = _testcapi.MethClass + + +@cpython_only +class TestCallingConventionsStatic(TestCallingConventions): + """Test calling static methods using various calling conventions""" + + def setUp(self): + self.obj = _testcapi.MethStatic() + self.expected_self = None + + def pyfunc(arg1, arg2): return [arg1, arg2] diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 3d1bbbcfa91c08..2e47e4cfd7d166 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5176,34 +5176,50 @@ sequence_getitem(PyObject *self, PyObject *args) } +/* Functions for testing C calling conventions (METH_*) are named meth_*, + * e.g. "meth_varargs" for METH_VARARGS. + * + * They all return a tuple of their C-level arguments, with None instead + * of NULL and Python tuples instead of C arrays. + */ + + +static PyObject* +_null_to_none(PyObject* obj) +{ + if (obj == NULL) { + Py_RETURN_NONE; + } + Py_INCREF(obj); + return obj; +} + static PyObject* meth_varargs(PyObject* self, PyObject* args) { - Py_INCREF(args); - return args; + return Py_BuildValue("NO", _null_to_none(self), args); } static PyObject* meth_varargs_keywords(PyObject* self, PyObject* args, PyObject* kwargs) { - return Py_BuildValue("OO", args, kwargs); + return Py_BuildValue("NOO", _null_to_none(self), args, kwargs); } static PyObject* meth_o(PyObject* self, PyObject* obj) { - Py_INCREF(obj); - return obj; + return Py_BuildValue("NO", _null_to_none(self), obj); } static PyObject* meth_noargs(PyObject* self, PyObject* ignored) { - Py_RETURN_NONE; + return _null_to_none(self); } static PyObject* -meth_fastcall(PyObject* self, PyObject** args, Py_ssize_t nargs) +_fastcall_to_tuple(PyObject* const* args, Py_ssize_t nargs) { PyObject *tuple = PyTuple_New(nargs); if (tuple == NULL) { @@ -5217,16 +5233,24 @@ meth_fastcall(PyObject* self, PyObject** args, Py_ssize_t nargs) } static PyObject* -meth_fastcall_keywords(PyObject* self, PyObject** args, +meth_fastcall(PyObject* self, PyObject* const* args, Py_ssize_t nargs) +{ + return Py_BuildValue( + "NN", _null_to_none(self), _fastcall_to_tuple(args, nargs) + ); +} + +static PyObject* +meth_fastcall_keywords(PyObject* self, PyObject* const* args, Py_ssize_t nargs, PyObject* kwargs) { - PyObject *pyargs = meth_fastcall(self, args, nargs); + PyObject *pyargs = _fastcall_to_tuple(args, nargs); if (pyargs == NULL) { return NULL; } PyObject *pykwargs = _PyObject_Vectorcall((PyObject*)&PyDict_Type, args + nargs, 0, kwargs); - return Py_BuildValue("NN", pyargs, pykwargs); + return Py_BuildValue("NNN", _null_to_none(self), pyargs, pykwargs); } static PyMethodDef TestMethods[] = { @@ -6145,6 +6169,66 @@ static PyTypeObject MethodDescriptor2_Type = { .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | _Py_TPFLAGS_HAVE_VECTORCALL, }; +static PyMethodDef meth_instance_methods[] = { + {"meth_varargs", meth_varargs, METH_VARARGS}, + {"meth_varargs_keywords", (PyCFunction)(void(*)(void))meth_varargs_keywords, METH_VARARGS|METH_KEYWORDS}, + {"meth_o", meth_o, METH_O}, + {"meth_noargs", meth_noargs, METH_NOARGS}, + {"meth_fastcall", (PyCFunction)(void(*)(void))meth_fastcall, METH_FASTCALL}, + {"meth_fastcall_keywords", (PyCFunction)(void(*)(void))meth_fastcall_keywords, METH_FASTCALL|METH_KEYWORDS}, + {NULL, NULL} /* sentinel */ +}; + + +static PyTypeObject MethInstance_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + "MethInstance", + sizeof(PyObject), + .tp_new = PyType_GenericNew, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_methods = meth_instance_methods, +}; + +static PyMethodDef meth_class_methods[] = { + {"meth_varargs", meth_varargs, METH_VARARGS|METH_CLASS}, + {"meth_varargs_keywords", (PyCFunction)(void(*)(void))meth_varargs_keywords, METH_VARARGS|METH_KEYWORDS|METH_CLASS}, + {"meth_o", meth_o, METH_O|METH_CLASS}, + {"meth_noargs", meth_noargs, METH_NOARGS|METH_CLASS}, + {"meth_fastcall", (PyCFunction)(void(*)(void))meth_fastcall, METH_FASTCALL|METH_CLASS}, + {"meth_fastcall_keywords", (PyCFunction)(void(*)(void))meth_fastcall_keywords, METH_FASTCALL|METH_KEYWORDS|METH_CLASS}, + {NULL, NULL} /* sentinel */ +}; + + +static PyTypeObject MethClass_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + "MethClass", + sizeof(PyObject), + .tp_new = PyType_GenericNew, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_methods = meth_class_methods, +}; + +static PyMethodDef meth_static_methods[] = { + {"meth_varargs", meth_varargs, METH_VARARGS|METH_STATIC}, + {"meth_varargs_keywords", (PyCFunction)(void(*)(void))meth_varargs_keywords, METH_VARARGS|METH_KEYWORDS|METH_STATIC}, + {"meth_o", meth_o, METH_O|METH_STATIC}, + {"meth_noargs", meth_noargs, METH_NOARGS|METH_STATIC}, + {"meth_fastcall", (PyCFunction)(void(*)(void))meth_fastcall, METH_FASTCALL|METH_STATIC}, + {"meth_fastcall_keywords", (PyCFunction)(void(*)(void))meth_fastcall_keywords, METH_FASTCALL|METH_KEYWORDS|METH_STATIC}, + {NULL, NULL} /* sentinel */ +}; + + +static PyTypeObject MethStatic_Type = { + PyVarObject_HEAD_INIT(NULL, 0) + "MethStatic", + sizeof(PyObject), + .tp_new = PyType_GenericNew, + .tp_flags = Py_TPFLAGS_DEFAULT, + .tp_methods = meth_static_methods, +}; + static struct PyModuleDef _testcapimodule = { PyModuleDef_HEAD_INIT, @@ -6231,6 +6315,21 @@ PyInit__testcapi(void) Py_INCREF(&Generic_Type); PyModule_AddObject(m, "Generic", (PyObject *)&Generic_Type); + if (PyType_Ready(&MethInstance_Type) < 0) + return NULL; + Py_INCREF(&MethInstance_Type); + PyModule_AddObject(m, "MethInstance", (PyObject *)&MethInstance_Type); + + if (PyType_Ready(&MethClass_Type) < 0) + return NULL; + Py_INCREF(&MethClass_Type); + PyModule_AddObject(m, "MethClass", (PyObject *)&MethClass_Type); + + if (PyType_Ready(&MethStatic_Type) < 0) + return NULL; + Py_INCREF(&MethStatic_Type); + PyModule_AddObject(m, "MethStatic", (PyObject *)&MethStatic_Type); + PyRecursingInfinitelyError_Type.tp_base = (PyTypeObject *)PyExc_Exception; if (PyType_Ready(&PyRecursingInfinitelyError_Type) < 0) { return NULL; From 381981a289a5cc40c2446d7942a06101f1b8a722 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 9 Sep 2019 14:02:25 +0100 Subject: [PATCH 03/11] Test GDB tracebacks with various C calling conventions --- Lib/test/test_gdb.py | 73 +++++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 31 deletions(-) diff --git a/Lib/test/test_gdb.py b/Lib/test/test_gdb.py index a2aa16c62b7d78..489b40bc384231 100644 --- a/Lib/test/test_gdb.py +++ b/Lib/test/test_gdb.py @@ -838,6 +838,7 @@ def test_gc(self): ) self.assertIn('Garbage-collecting', gdb_output) + @unittest.skipIf(python_is_optimized(), "Python was compiled with optimizations") # Some older versions of gdb will fail with @@ -847,38 +848,48 @@ def test_pycfunction(self): 'Verify that "py-bt" displays invocations of PyCFunction instances' # Various optimizations multiply the code paths by which these are # called, so test a variety of calling conventions. - for py_name, py_args, c_name, expected_frame_number in ( - ('gmtime', '', 'time_gmtime', 1), # METH_VARARGS - ('len', '[]', 'builtin_len', 1), # METH_O - ('locals', '', 'builtin_locals', 1), # METH_NOARGS - ('iter', '[]', 'builtin_iter', 1), # METH_FASTCALL - ('sorted', '[]', 'builtin_sorted', 1), # METH_FASTCALL|METH_KEYWORDS + for func_name, args, expected_frame in ( + ('meth_varargs', '', 1), + ('meth_varargs_keywords', '', 1), + ('meth_o', '[]', 1), + ('meth_noargs', '', 1), + ('meth_fastcall', '', 1), + ('meth_fastcall_keywords', '', 1), ): - with self.subTest(c_name): - cmd = ('from time import gmtime\n' # (not always needed) - 'def foo():\n' - f' {py_name}({py_args})\n' - 'def bar():\n' - ' foo()\n' - 'bar()\n') - # Verify with "py-bt": - gdb_output = self.get_stack_trace( - cmd, - breakpoint=c_name, - cmds_after_breakpoint=['bt', 'py-bt'], - ) - self.assertIn(f' Date: Tue, 16 Jul 2019 15:02:21 +0200 Subject: [PATCH 04/11] Add tests for calling from C Adapted by Petr Viktorin from https://github.com/python/cpython/pull/14795 All bugs are Petr's :) --- Lib/test/test_call.py | 97 +++++++++++++++++++++------------------ Modules/_testcapimodule.c | 2 +- 2 files changed, 53 insertions(+), 46 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index a17ff3b4d7e779..bbbacc5ebdd07a 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -299,7 +299,7 @@ def test_oldargs1_2_kw(self): @cpython_only class TestCallingConventions(unittest.TestCase): - """Test calling using the various C calling conventions (METH_*) + """Test calling using various C calling conventions (METH_*) from Python Subclasses test several kinds of functions (module-level, methods, class methods static methods) using these attributes: @@ -503,14 +503,15 @@ def static_method(): PYTHON_INSTANCE = PythonClass() - -IGNORE_RESULT = object() - +NULL_OR_EMPTY = object() @cpython_only class FastCallTests(unittest.TestCase): + """Test calling using various callables from C + """ + # Test calls with positional arguments - CALLS_POSARGS = ( + CALLS_POSARGS = [ # (func, args: tuple, result) # Python function with 2 arguments @@ -529,31 +530,11 @@ class FastCallTests(unittest.TestCase): (PYTHON_INSTANCE.class_method, (), "classmethod"), (PYTHON_INSTANCE.static_method, (), "staticmethod"), - # C function: METH_NOARGS - (globals, (), IGNORE_RESULT), - - # C function: METH_O - (id, ("hello",), IGNORE_RESULT), - - # C function: METH_VARARGS - (dir, (1,), IGNORE_RESULT), - - # C function: METH_VARARGS | METH_KEYWORDS - (min, (5, 9), 5), - - # C function: METH_FASTCALL - (divmod, (1000, 33), (30, 10)), - - # C type static method: METH_FASTCALL | METH_CLASS - (int.from_bytes, (b'\x01\x00', 'little'), 1), - - # bpo-30524: Test that calling a C type static method with no argument - # doesn't crash (ignore the result): METH_FASTCALL | METH_CLASS - (datetime.datetime.now, (), IGNORE_RESULT), - ) + # C callables are added later + ] # Test calls with positional and keyword arguments - CALLS_KWARGS = ( + CALLS_KWARGS = [ # (func, args: tuple, kwargs: dict, result) # Python function with 2 arguments @@ -564,17 +545,51 @@ class FastCallTests(unittest.TestCase): (PYTHON_INSTANCE.method, (1,), {'arg2': 2}, [1, 2]), (PYTHON_INSTANCE.method, (), {'arg1': 1, 'arg2': 2}, [1, 2]), - # C function: METH_VARARGS | METH_KEYWORDS - (max, ([],), {'default': 9}, 9), - - # C type static method: METH_FASTCALL | METH_CLASS - (int.from_bytes, (b'\x01\x00',), {'byteorder': 'little'}, 1), - (int.from_bytes, (), {'bytes': b'\x01\x00', 'byteorder': 'little'}, 1), - ) + # C callables are added later + ] + + # Add all the calling conventions and variants of C callables + _instance = _testcapi.MethInstance() + for obj, expected_self in ( + (_testcapi, _testcapi), # module-level function + (_instance, _instance), # bound method + (_testcapi.MethClass, _testcapi.MethClass), # class method on class + (_testcapi.MethClass(), _testcapi.MethClass), # class method on inst. + (_testcapi.MethStatic, None), # static method + ): + CALLS_POSARGS.extend([ + (obj.meth_varargs, (1, 2), (expected_self, (1, 2))), + (obj.meth_varargs_keywords, + (1, 2), (expected_self, (1, 2), NULL_OR_EMPTY)), + (obj.meth_fastcall, (1, 2), (expected_self, (1, 2))), + (obj.meth_fastcall, (), (expected_self, ())), + (obj.meth_fastcall_keywords, + (1, 2), (expected_self, (1, 2), NULL_OR_EMPTY)), + (obj.meth_fastcall_keywords, + (), (expected_self, (), NULL_OR_EMPTY)), + (obj.meth_noargs, (), expected_self), + (obj.meth_o, (123, ), (expected_self, 123)), + ]) + + CALLS_KWARGS.extend([ + (obj.meth_varargs_keywords, + (1, 2), {'x': 'y'}, (expected_self, (1, 2), {'x': 'y'})), + (obj.meth_varargs_keywords, + (), {'x': 'y'}, (expected_self, (), {'x': 'y'})), + (obj.meth_varargs_keywords, + (1, 2), {}, (expected_self, (1, 2), NULL_OR_EMPTY)), + (obj.meth_fastcall_keywords, + (1, 2), {'x': 'y'}, (expected_self, (1, 2), {'x': 'y'})), + (obj.meth_fastcall_keywords, + (), {'x': 'y'}, (expected_self, (), {'x': 'y'})), + (obj.meth_fastcall_keywords, + (1, 2), {}, (expected_self, (1, 2), NULL_OR_EMPTY)), + ]) def check_result(self, result, expected): - if expected is IGNORE_RESULT: - return + if isinstance(expected, tuple) and expected[-1] is NULL_OR_EMPTY: + if result[-1] in ({}, None): + expected = (*expected[:-1], result[-1]) self.assertEqual(result, expected) def test_fastcall(self): @@ -599,19 +614,11 @@ def test_vectorcall_dict(self): result = _testcapi.pyobject_fastcalldict(func, args, None) self.check_result(result, expected) - # kwargs={} - result = _testcapi.pyobject_fastcalldict(func, args, {}) - self.check_result(result, expected) - if not args: # args=NULL, nargs=0, kwargs=NULL result = _testcapi.pyobject_fastcalldict(func, None, None) self.check_result(result, expected) - # args=NULL, nargs=0, kwargs={} - result = _testcapi.pyobject_fastcalldict(func, None, {}) - self.check_result(result, expected) - for func, args, kwargs, expected in self.CALLS_KWARGS: with self.subTest(func=func, args=args, kwargs=kwargs): result = _testcapi.pyobject_fastcalldict(func, args, kwargs) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 2e47e4cfd7d166..ce8d3a31abd706 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5203,7 +5203,7 @@ meth_varargs(PyObject* self, PyObject* args) static PyObject* meth_varargs_keywords(PyObject* self, PyObject* args, PyObject* kwargs) { - return Py_BuildValue("NOO", _null_to_none(self), args, kwargs); + return Py_BuildValue("NOO", _null_to_none(self), args, _null_to_none(kwargs)); } static PyObject* From 2b9295cbc6c403164bc42f42311f3bdddbc35ceb Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 9 Sep 2019 16:37:53 +0100 Subject: [PATCH 05/11] Remove the old test_call.CFunctionCalls These tests are now covered by in TestCallingConventions (and subclasses) --- Lib/test/test_call.py | 126 ------------------------------------------ 1 file changed, 126 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index bbbacc5ebdd07a..ca2659ded6ed4a 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -27,132 +27,6 @@ def fn(**kw): self.assertEqual(list(res.items()), expected) -# The test cases here covered several paths through the function calling -# code. They depended on the C function being defined with the METH_VARARGS -# or METH_OLDARGS flag, but all these functions have since changed these -# flags. -# Nevertheless, the tests should still pass, so we keep them. - -class CFunctionCalls(unittest.TestCase): - - # {}.__contains__ - - def test_varargs0(self): - self.assertRaises(TypeError, {}.__contains__) - - def test_varargs1(self): - {}.__contains__(0) - - def test_varargs2(self): - self.assertRaises(TypeError, {}.__contains__, 0, 1) - - def test_varargs0_ext(self): - try: - {}.__contains__(*()) - except TypeError: - pass - - def test_varargs1_ext(self): - {}.__contains__(*(0,)) - - def test_varargs2_ext(self): - try: - {}.__contains__(*(1, 2)) - except TypeError: - pass - else: - raise RuntimeError - - def test_varargs1_kw(self): - self.assertRaises(TypeError, {}.__contains__, x=2) - - def test_varargs2_kw(self): - self.assertRaises(TypeError, {}.__contains__, x=2, y=2) - - # {}.keys - - def test_oldargs0_0(self): - {}.keys() - - def test_oldargs0_1(self): - self.assertRaises(TypeError, {}.keys, 0) - - def test_oldargs0_2(self): - self.assertRaises(TypeError, {}.keys, 0, 1) - - def test_oldargs0_0_ext(self): - {}.keys(*()) - - def test_oldargs0_1_ext(self): - try: - {}.keys(*(0,)) - except TypeError: - pass - else: - raise RuntimeError - - def test_oldargs0_2_ext(self): - try: - {}.keys(*(1, 2)) - except TypeError: - pass - else: - raise RuntimeError - - def test_oldargs0_0_kw(self): - try: - {}.keys(x=2) - except TypeError: - pass - else: - raise RuntimeError - - def test_oldargs0_1_kw(self): - self.assertRaises(TypeError, {}.keys, x=2) - - def test_oldargs0_2_kw(self): - self.assertRaises(TypeError, {}.keys, x=2, y=2) - - # [].count - - def test_oldargs1_0(self): - self.assertRaises(TypeError, [].count) - - def test_oldargs1_1(self): - [].count(1) - - def test_oldargs1_2(self): - self.assertRaises(TypeError, [].count, 1, 2) - - def test_oldargs1_0_ext(self): - try: - [].count(*()) - except TypeError: - pass - else: - raise RuntimeError - - def test_oldargs1_1_ext(self): - [].count(*(1,)) - - def test_oldargs1_2_ext(self): - try: - [].count(*(1, 2)) - except TypeError: - pass - else: - raise RuntimeError - - def test_oldargs1_0_kw(self): - self.assertRaises(TypeError, [].count, x=2) - - def test_oldargs1_1_kw(self): - self.assertRaises(TypeError, [].count, {}, x=2) - - def test_oldargs1_2_kw(self): - self.assertRaises(TypeError, [].count, x=2, y=2) - - @cpython_only class CFunctionCallsErrorMessages(unittest.TestCase): From 835d5bf895adf65d7d1b54e5d98e56f8d76103d3 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 9 Sep 2019 16:49:32 +0100 Subject: [PATCH 06/11] Remove unnecessary test class --- Lib/test/test_call.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index ca2659ded6ed4a..a4ee40704af522 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -311,12 +311,6 @@ def test_fastcall_keywords_ext(self): ) -@cpython_only -class TestCallingConventionsFunction(TestCallingConventions): - """Test calling module-level functions using various calling conventions""" - - - @cpython_only class TestCallingConventionsInstance(TestCallingConventions): """Test calling instance methods using various calling conventions""" From f57b0bd94684d6cf13b609fc9fd73ee4299f18e3 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 9 Sep 2019 16:50:14 +0100 Subject: [PATCH 07/11] Remove @cpython_only decorators (it's just C API) --- Lib/test/test_call.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Lib/test/test_call.py b/Lib/test/test_call.py index a4ee40704af522..c233ba1351fe5a 100644 --- a/Lib/test/test_call.py +++ b/Lib/test/test_call.py @@ -171,7 +171,6 @@ def test_oldargs1_2_kw(self): -@cpython_only class TestCallingConventions(unittest.TestCase): """Test calling using various C calling conventions (METH_*) from Python @@ -311,7 +310,6 @@ def test_fastcall_keywords_ext(self): ) -@cpython_only class TestCallingConventionsInstance(TestCallingConventions): """Test calling instance methods using various calling conventions""" @@ -319,7 +317,6 @@ def setUp(self): self.obj = self.expected_self = _testcapi.MethInstance() -@cpython_only class TestCallingConventionsClass(TestCallingConventions): """Test calling class methods using various calling conventions""" @@ -327,7 +324,6 @@ def setUp(self): self.obj = self.expected_self = _testcapi.MethClass -@cpython_only class TestCallingConventionsClassInstance(TestCallingConventions): """Test calling class methods on instance""" @@ -336,7 +332,6 @@ def setUp(self): self.expected_self = _testcapi.MethClass -@cpython_only class TestCallingConventionsStatic(TestCallingConventions): """Test calling static methods using various calling conventions""" @@ -373,7 +368,6 @@ def static_method(): NULL_OR_EMPTY = object() -@cpython_only class FastCallTests(unittest.TestCase): """Test calling using various callables from C """ From c2a7454eb8e937ed3ced956858b5873a56c91db2 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 9 Sep 2019 17:03:10 +0100 Subject: [PATCH 08/11] Add docstrings to _testcapi.Meth* classes --- Modules/_testcapimodule.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ce8d3a31abd706..24a71bb4206f0c 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -6187,6 +6187,8 @@ static PyTypeObject MethInstance_Type = { .tp_new = PyType_GenericNew, .tp_flags = Py_TPFLAGS_DEFAULT, .tp_methods = meth_instance_methods, + .tp_doc = PyDoc_STR( + "Class with normal (instance) methods to test calling conventions"), }; static PyMethodDef meth_class_methods[] = { @@ -6207,6 +6209,8 @@ static PyTypeObject MethClass_Type = { .tp_new = PyType_GenericNew, .tp_flags = Py_TPFLAGS_DEFAULT, .tp_methods = meth_class_methods, + .tp_doc = PyDoc_STR( + "Class with class methods to test calling conventions"), }; static PyMethodDef meth_static_methods[] = { @@ -6227,6 +6231,8 @@ static PyTypeObject MethStatic_Type = { .tp_new = PyType_GenericNew, .tp_flags = Py_TPFLAGS_DEFAULT, .tp_methods = meth_static_methods, + .tp_doc = PyDoc_STR( + "Class with static methods to test calling conventions"), }; From ec0a0b22b12b090e3cee18b015eb47ee49987460 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 9 Sep 2019 17:13:22 +0100 Subject: [PATCH 09/11] Fix a refleak --- Modules/_testcapimodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 24a71bb4206f0c..2b261b6c86dc4b 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -5203,7 +5203,7 @@ meth_varargs(PyObject* self, PyObject* args) static PyObject* meth_varargs_keywords(PyObject* self, PyObject* args, PyObject* kwargs) { - return Py_BuildValue("NOO", _null_to_none(self), args, _null_to_none(kwargs)); + return Py_BuildValue("NON", _null_to_none(self), args, _null_to_none(kwargs)); } static PyObject* From d8a28ab29e668e5d66824fdaec5bc59cf914e6a7 Mon Sep 17 00:00:00 2001 From: Paul Ganssle Date: Tue, 10 Sep 2019 11:41:40 +0100 Subject: [PATCH 10/11] Formatting and add explanatory comment --- Lib/test/test_gdb.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_gdb.py b/Lib/test/test_gdb.py index 489b40bc384231..365c290edd6fbf 100644 --- a/Lib/test/test_gdb.py +++ b/Lib/test/test_gdb.py @@ -844,6 +844,12 @@ def test_gc(self): # Some older versions of gdb will fail with # "Cannot find new threads: generic error" # unless we add LD_PRELOAD=PATH-TO-libpthread.so.1 as a workaround + # + # gdb will also generate many erroneous errors such as: + # Function "meth_varargs" not defined. + # This is because we are calling functions from an "external" module + # (_testcapimodule) rather than compiled-in functions. It seems difficult + # to suppress these. See also the comment in DebuggerTests.get_stack_trace def test_pycfunction(self): 'Verify that "py-bt" displays invocations of PyCFunction instances' # Various optimizations multiply the code paths by which these are @@ -866,12 +872,14 @@ def test_pycfunction(self): # '_testcapi.MethInstance()', ): with self.subTest(f'{obj}.{func_name}'): - cmd = ('import _testcapi\n' # (not always needed) - 'def foo():\n' - f' {obj}.{func_name}({args})\n' - 'def bar():\n' - ' foo()\n' - 'bar()\n') + cmd = textwrap.dedent(f''' + import _testcapi # (not always needed) + def foo(): + {obj}.{func_name}({args}) + def bar(): + foo() + bar() + ''') # Verify with "py-bt": gdb_output = self.get_stack_trace( cmd, From f2d71eb67a3e24a878b92dc85265d33dc2a6a744 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 10 Sep 2019 11:52:48 +0100 Subject: [PATCH 11/11] Remove outdated comment --- Lib/test/test_gdb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_gdb.py b/Lib/test/test_gdb.py index 365c290edd6fbf..e515d0d3fb513e 100644 --- a/Lib/test/test_gdb.py +++ b/Lib/test/test_gdb.py @@ -873,7 +873,7 @@ def test_pycfunction(self): ): with self.subTest(f'{obj}.{func_name}'): cmd = textwrap.dedent(f''' - import _testcapi # (not always needed) + import _testcapi def foo(): {obj}.{func_name}({args}) def bar():