diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index f3fd610414cd8a..35b8ac6c148534 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -3,6 +3,7 @@ # Licensed to the PSF under a contributor agreement. from functools import partial +from itertools import permutations from test import support, test_tools from test.support import os_helper from test.support.os_helper import TESTFN, unlink, rmtree @@ -3333,12 +3334,106 @@ def test_vararg(self): ac_tester.vararg(1, b=2) self.assertEqual(ac_tester.vararg(1, 2, 3, 4), (1, (2, 3, 4))) + def test_vararg_with_multiple_pos(self): + # vararg_with_multiple_pos(a, b, *args) + func = ac_tester.vararg_with_multiple_pos + with self.assertRaises(TypeError): + func() + + self.assertTupleEqual(func(1, 2), (1, 2, ())) + self.assertTupleEqual(func(1, b=2), (1, 2, ())) + self.assertTupleEqual(func(a=1, b=2), (1, 2, ())) + self.assertTupleEqual(func(1, 2, 'c'), (1, 2, ('c',))) + self.assertTupleEqual(func(1, 2, 'c', 'd'), (1, 2, ('c', 'd'))) + def test_vararg_with_default(self): with self.assertRaises(TypeError): ac_tester.vararg_with_default() self.assertEqual(ac_tester.vararg_with_default(1, b=False), (1, (), False)) self.assertEqual(ac_tester.vararg_with_default(1, 2, 3, 4), (1, (2, 3, 4), False)) self.assertEqual(ac_tester.vararg_with_default(1, 2, 3, 4, b=True), (1, (2, 3, 4), True)) + self.assertEqual(ac_tester.vararg_with_default(a=1, b=False), (1, (), False)) + self.assertEqual(ac_tester.vararg_with_default(b=False, a=1), (1, (), False)) + + def test_vararg_with_more_defaults(self): + # vararg_with_more_defaults(a, *args, kw1=False, kw2=False) + with self.assertRaises(TypeError): + ac_tester.vararg_with_more_defaults() + + check = self.assertTupleEqual + fn = ac_tester.vararg_with_more_defaults + + check(fn(1), (1, (), False, False)) + + check(fn(1, 'x'), (1, ('x',), False, False)) + check(fn(1, 'x', kw1=True), (1, ('x',), True, False)) + check(fn(1, 'x', kw2=True), (1, ('x',), False, True)) + check(fn(1, 'x', kw1=True, kw2=True), (1, ('x',), True, True)) + check(fn(1, 'x', kw2=True, kw1=True), (1, ('x',), True, True)) + + check(fn(1, 'x', 'y'), (1, ('x', 'y'), False, False)) + check(fn(1, 'x', 'y', kw1=True), (1, ('x', 'y'), True, False)) + check(fn(1, 'x', 'y', kw2=True), (1, ('x', 'y'), False, True)) + check(fn(1, 'x', 'y', kw1=True, kw2=True), (1, ('x', 'y'), True, True)) + check(fn(1, 'x', 'y', kw2=True, kw1=True), (1, ('x', 'y'), True, True)) + + check(fn(1, kw1=True), (1, (), True, False)) + check(fn(1, kw2=True), (1, (), False, True)) + check(fn(1, kw1=True, kw2=True), (1, (), True, True)) + check(fn(1, kw2=True, kw1=True), (1, (), True, True)) + + check(fn(a=1), (1, (), False, False)) + check(fn(a=1, kw1=True), (1, (), True, False)) + check(fn(kw1=True, a=1), (1, (), True, False)) + check(fn(a=1, kw2=True), (1, (), False, True)) + check(fn(kw2=True, a=1), (1, (), False, True)) + + for kwds in permutations(dict(a=1, kw1=True, kw2=True).items()): + check(fn(**dict(kwds)), (1, (), True, True)) + + def test_vararg_with_more_defaults_and_pos(self): + # vararg_with_more_defaults_and_pos(a, b, *args, kw1=False, kw2=False) + with self.assertRaises(TypeError): + ac_tester.vararg_with_more_defaults_and_pos() + + check = self.assertTupleEqual + fn = ac_tester.vararg_with_more_defaults_and_pos + + check(fn(1, 2), (1, 2, (), False, False)) + + check(fn(1, 2, kw1=True), (1, 2, (), True, False)) + check(fn(1, 2, kw2=True), (1, 2, (), False, True)) + check(fn(1, 2, kw1=True, kw2=True), (1, 2, (), True, True)) + check(fn(1, 2, kw2=True, kw1=True), (1, 2, (), True, True)) + + check(fn(1, b=1), (1, 1, (), False, False)) + check(fn(1, b=1, kw1=True), (1, 1, (), True, False)) + check(fn(1, kw1=True, b=1), (1, 1, (), True, False)) + check(fn(1, b=1, kw2=True), (1, 1, (), False, True)) + check(fn(1, kw2=True, b=1), (1, 1, (), False, True)) + + check(fn(1, 2, 'x'), (1, 2, ('x',), False, False)) + check(fn(1, 2, 'x', 'y'), (1, 2, ('x', 'y'), False, False)) + check(fn(1, 2, 'x', kw1=True), (1, 2, ('x',), True, False)) + check(fn(1, 2, 'x', 'y', kw1=True), (1, 2, ('x', 'y'), True, False)) + check(fn(1, 2, 'x', kw2=True), (1, 2, ('x',), False, True)) + check(fn(1, 2, 'x', 'y', kw2=True), (1, 2, ('x', 'y'), False, True)) + check(fn(1, 2, 'x', kw1=True, kw2=True), (1, 2, ('x',), True, True)) + check(fn(1, 2, 'x', 'y', kw1=True, kw2=True), (1, 2, ('x', 'y'), True, True)) + check(fn(1, 2, 'x', kw2=True, kw1=True), (1, 2, ('x',), True, True)) + check(fn(1, 2, 'x', 'y', kw2=True, kw1=True), (1, 2, ('x', 'y'), True, True)) + + check(fn(a=1, b=2), (1, 2, (), False, False)) + check(fn(b=2, a=1), (1, 2, (), False, False)) + + for kwds in permutations(dict(a=1, b=2, kw1=True).items()): + check(fn(**dict(kwds)), (1, 2, (), True, False)) + for kwds in permutations(dict(a=1, b=2, kw2=True).items()): + check(fn(**dict(kwds)), (1, 2, (), False, True)) + for kwds in permutations(dict(b=2, kw1=True, kw2=True).items()): + check(fn(1, **dict(kwds)), (1, 2, (), True, True)) + for kwds in permutations(dict(a=1, b=2, kw1=True, kw2=True).items()): + check(fn(**dict(kwds)), (1, 2, (), True, True)) def test_vararg_with_only_defaults(self): self.assertEqual(ac_tester.vararg_with_only_defaults(), ((), None)) diff --git a/Misc/NEWS.d/next/Core and Builtins/2024-08-01-14-04-18.gh-issue-118814.m3pHDY.rst b/Misc/NEWS.d/next/Core and Builtins/2024-08-01-14-04-18.gh-issue-118814.m3pHDY.rst new file mode 100644 index 00000000000000..377ee911bb2d3b --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2024-08-01-14-04-18.gh-issue-118814.m3pHDY.rst @@ -0,0 +1,3 @@ +Fix argument parsing by ``_PyArg_UnpackKeywordsWithVararg`` when passing +positional as keyword arguments before variadic arguments. Patch by Bénédikt +Tran. diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index 4187e13231dc69..6e94027a977934 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -1014,6 +1014,24 @@ vararg_impl(PyObject *module, PyObject *a, PyObject *args) } +/*[clinic input] +vararg_with_multiple_pos + + a: object + b: object + *args: object + +[clinic start generated code]*/ + +static PyObject * +vararg_with_multiple_pos_impl(PyObject *module, PyObject *a, PyObject *b, + PyObject *args) +/*[clinic end generated code: output=7fe8cbc4165d8592 input=49b49a877d24f459]*/ +{ + return pack_arguments_newref(3, a, b, args); +} + + /*[clinic input] vararg_with_default @@ -1033,6 +1051,51 @@ vararg_with_default_impl(PyObject *module, PyObject *a, PyObject *args, } +/*[clinic input] +vararg_with_more_defaults + + a: object + *args: object + kw1: bool = False + kw2: bool = False + +[clinic start generated code]*/ + +static PyObject * +vararg_with_more_defaults_impl(PyObject *module, PyObject *a, PyObject *args, + int kw1, int kw2) +/*[clinic end generated code: output=efb60dafc084a301 input=d84a0e8641b30838]*/ +{ + PyObject *obj_kw1 = kw1 ? Py_True : Py_False; + PyObject *obj_kw2 = kw2 ? Py_True : Py_False; + return pack_arguments_newref(4, a, args, obj_kw1, obj_kw2); +} + + +/*[clinic input] +vararg_with_more_defaults_and_pos + + a: object + b: object + *args: object + kw1: bool = False + kw2: bool = False + +[clinic start generated code]*/ + +static PyObject * +vararg_with_more_defaults_and_pos_impl(PyObject *module, PyObject *a, + PyObject *b, PyObject *args, int kw1, + int kw2) +/*[clinic end generated code: output=9828348aee06bd21 input=447044ec4b59bb45]*/ + +{ + PyObject *obj_kw1 = kw1 ? Py_True : Py_False; + PyObject *obj_kw2 = kw2 ? Py_True : Py_False; + return pack_arguments_newref(5, a, b, args, obj_kw1, obj_kw2); +} + + /*[clinic input] vararg_with_only_defaults @@ -1906,7 +1969,10 @@ static PyMethodDef tester_methods[] = { POSONLY_VARARG_METHODDEF VARARG_AND_POSONLY_METHODDEF VARARG_METHODDEF + VARARG_WITH_MULTIPLE_POS_METHODDEF VARARG_WITH_DEFAULT_METHODDEF + VARARG_WITH_MORE_DEFAULTS_METHODDEF + VARARG_WITH_MORE_DEFAULTS_AND_POS_METHODDEF VARARG_WITH_ONLY_DEFAULTS_METHODDEF GH_32092_OOB_METHODDEF GH_32092_KW_PASS_METHODDEF diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index e02f39d15cce0f..acc506e4cde4a5 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2674,6 +2674,66 @@ vararg(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwna return return_value; } +PyDoc_STRVAR(vararg_with_multiple_pos__doc__, +"vararg_with_multiple_pos($module, /, a, b, *args)\n" +"--\n" +"\n"); + +#define VARARG_WITH_MULTIPLE_POS_METHODDEF \ + {"vararg_with_multiple_pos", _PyCFunction_CAST(vararg_with_multiple_pos), METH_FASTCALL|METH_KEYWORDS, vararg_with_multiple_pos__doc__}, + +static PyObject * +vararg_with_multiple_pos_impl(PyObject *module, PyObject *a, PyObject *b, + PyObject *args); + +static PyObject * +vararg_with_multiple_pos(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 2 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { _Py_LATIN1_CHR('a'), _Py_LATIN1_CHR('b'), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"a", "b", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "vararg_with_multiple_pos", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[3]; + PyObject *a; + PyObject *b; + PyObject *__clinic_args = NULL; + + args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 2, 2, 0, 2, argsbuf); + if (!args) { + goto exit; + } + a = args[0]; + b = args[1]; + __clinic_args = args[2]; + return_value = vararg_with_multiple_pos_impl(module, a, b, __clinic_args); + +exit: + Py_XDECREF(__clinic_args); + return return_value; +} + PyDoc_STRVAR(vararg_with_default__doc__, "vararg_with_default($module, /, a, *args, b=False)\n" "--\n" @@ -2742,6 +2802,166 @@ vararg_with_default(PyObject *module, PyObject *const *args, Py_ssize_t nargs, P return return_value; } +PyDoc_STRVAR(vararg_with_more_defaults__doc__, +"vararg_with_more_defaults($module, /, a, *args, kw1=False, kw2=False)\n" +"--\n" +"\n"); + +#define VARARG_WITH_MORE_DEFAULTS_METHODDEF \ + {"vararg_with_more_defaults", _PyCFunction_CAST(vararg_with_more_defaults), METH_FASTCALL|METH_KEYWORDS, vararg_with_more_defaults__doc__}, + +static PyObject * +vararg_with_more_defaults_impl(PyObject *module, PyObject *a, PyObject *args, + int kw1, int kw2); + +static PyObject * +vararg_with_more_defaults(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 3 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { _Py_LATIN1_CHR('a'), &_Py_ID(kw1), &_Py_ID(kw2), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"a", "kw1", "kw2", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "vararg_with_more_defaults", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[4]; + Py_ssize_t noptargs = Py_MIN(nargs, 1) + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1; + PyObject *a; + PyObject *__clinic_args = NULL; + int kw1 = 0; + int kw2 = 0; + + args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 1, 1, 0, 1, argsbuf); + if (!args) { + goto exit; + } + a = args[0]; + __clinic_args = args[1]; + if (!noptargs) { + goto skip_optional_kwonly; + } + if (args[2]) { + kw1 = PyObject_IsTrue(args[2]); + if (kw1 < 0) { + goto exit; + } + if (!--noptargs) { + goto skip_optional_kwonly; + } + } + kw2 = PyObject_IsTrue(args[3]); + if (kw2 < 0) { + goto exit; + } +skip_optional_kwonly: + return_value = vararg_with_more_defaults_impl(module, a, __clinic_args, kw1, kw2); + +exit: + Py_XDECREF(__clinic_args); + return return_value; +} + +PyDoc_STRVAR(vararg_with_more_defaults_and_pos__doc__, +"vararg_with_more_defaults_and_pos($module, /, a, b, *args, kw1=False,\n" +" kw2=False)\n" +"--\n" +"\n"); + +#define VARARG_WITH_MORE_DEFAULTS_AND_POS_METHODDEF \ + {"vararg_with_more_defaults_and_pos", _PyCFunction_CAST(vararg_with_more_defaults_and_pos), METH_FASTCALL|METH_KEYWORDS, vararg_with_more_defaults_and_pos__doc__}, + +static PyObject * +vararg_with_more_defaults_and_pos_impl(PyObject *module, PyObject *a, + PyObject *b, PyObject *args, int kw1, + int kw2); + +static PyObject * +vararg_with_more_defaults_and_pos(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) + + #define NUM_KEYWORDS 4 + static struct { + PyGC_Head _this_is_not_used; + PyObject_VAR_HEAD + PyObject *ob_item[NUM_KEYWORDS]; + } _kwtuple = { + .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) + .ob_item = { _Py_LATIN1_CHR('a'), _Py_LATIN1_CHR('b'), &_Py_ID(kw1), &_Py_ID(kw2), }, + }; + #undef NUM_KEYWORDS + #define KWTUPLE (&_kwtuple.ob_base.ob_base) + + #else // !Py_BUILD_CORE + # define KWTUPLE NULL + #endif // !Py_BUILD_CORE + + static const char * const _keywords[] = {"a", "b", "kw1", "kw2", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "vararg_with_more_defaults_and_pos", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[5]; + Py_ssize_t noptargs = Py_MIN(nargs, 2) + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 2; + PyObject *a; + PyObject *b; + PyObject *__clinic_args = NULL; + int kw1 = 0; + int kw2 = 0; + + args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 2, 2, 0, 2, argsbuf); + if (!args) { + goto exit; + } + a = args[0]; + b = args[1]; + __clinic_args = args[2]; + if (!noptargs) { + goto skip_optional_kwonly; + } + if (args[3]) { + kw1 = PyObject_IsTrue(args[3]); + if (kw1 < 0) { + goto exit; + } + if (!--noptargs) { + goto skip_optional_kwonly; + } + } + kw2 = PyObject_IsTrue(args[4]); + if (kw2 < 0) { + goto exit; + } +skip_optional_kwonly: + return_value = vararg_with_more_defaults_and_pos_impl(module, a, b, __clinic_args, kw1, kw2); + +exit: + Py_XDECREF(__clinic_args); + return return_value; +} + PyDoc_STRVAR(vararg_with_only_defaults__doc__, "vararg_with_only_defaults($module, /, *args, b=None)\n" "--\n" @@ -3418,4 +3638,4 @@ _testclinic_TestClass_get_defining_class_arg(PyObject *self, PyTypeObject *cls, exit: return return_value; } -/*[clinic end generated code: output=0d0ceed6c46547bb input=a9049054013a1b77]*/ +/*[clinic end generated code: output=fbc17fb342e02e52 input=a9049054013a1b77]*/ diff --git a/Python/getargs.c b/Python/getargs.c index ec2eeb15c832c3..14a4bb3b0012ba 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -2549,28 +2549,35 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs, return NULL; } - /* create varargs tuple */ + /* + * Create the tuple of variadic arguments and place it at 'buf[vararg]'. + * The function has at most 'maxpos' positional arguments, possibly with + * default values or not and possibly positional-only. Since 'args' is + * an array of 'nargs' arguments representing the positional arguments + * of the call, there are only 'nargs - maxpos' arguments that will + * be considered as variadic and gathered into a single tuple. + */ varargssize = nargs - maxpos; if (varargssize < 0) { varargssize = 0; } - buf[vararg] = PyTuple_New(varargssize); - if (!buf[vararg]) { - return NULL; - } - /* copy tuple args */ - for (i = 0; i < nargs; i++) { - if (i >= vararg) { - PyTuple_SET_ITEM(buf[vararg], i - vararg, Py_NewRef(args[i])); - continue; - } - else { - buf[i] = args[i]; - } - } + // copy the first arguments until the 'vararg' index + memcpy(buf, args, vararg * sizeof(PyObject *)); + // remaining arguments are all considered as variadic ones + buf[vararg] = _PyTuple_FromArray(&args[vararg], varargssize); - /* copy keyword args using kwtuple to drive process */ + /* + * We need to place the keyword arguments correctly in "buf". + * + * The buffer is always of the following form: + * + * buf = [x1, ..., xN] (*args) [k1, ..., kM] + * + * where x1, ..., xN are the positional arguments, '*args' is a tuple + * containing the variadic arguments (it will be untouched now) and + * k1, ..., kM are the keyword arguments that are not positionals. + */ for (i = Py_MAX((int)nargs, posonly) - Py_SAFE_DOWNCAST(varargssize, Py_ssize_t, int); i < maxargs; i++) { PyObject *current_arg; if (nkwargs) { @@ -2587,22 +2594,38 @@ _PyArg_UnpackKeywordsWithVararg(PyObject *const *args, Py_ssize_t nargs, else { current_arg = NULL; } - - /* If an arguments is passed in as a keyword argument, - * it should be placed before `buf[vararg]`. + /* + * Assume that an argument at index `i` is passed in + * as a keyword argument. Depending on `i`, it should + * be placed before `buf[vararg]` or at `buf[i + 1]`. + * + * It is placed at `buf[vararg]` if `nargs < vararg` + * and `i < vararg`, e.g.: + * + * def f(a, /, b, *args): pass * - * For example: - * def f(a, /, b, *args): - * pass - * f(1, b=2) + * The `buf` array for "f(1, b=2)" is: * - * This `buf` array should be: [1, 2, NULL]. - * In this case, nargs < vararg. + * buf = [1, 2, NULL]. * - * Otherwise, we leave a place at `buf[vararg]` for vararg tuple - * so the index is `i + 1`. */ - if (nargs < vararg && i != vararg) { - buf[i] = current_arg; + * On the other hand, if the argument is a real + * keyword argument (namely `i > vararg`), then + * it is placed at `buf[i + 1]` since `buf[vararg]` + * is already taken, e.g.: + * + * def g(a, b, *args, c, d): ... + * + * The `buf` array for "f(a=1, b=2, c=3, d=4)" is: + * + * buf = [1, 2, NULL, 3, 4]. + */ + if (nargs < vararg && i < vararg) { + if (current_arg != NULL) { + // It might happen that in the previous iteration, + // we did "buf[i + 1] = current_arg" and that in + // this current loop iteration, current_arg == NULL. + buf[i] = current_arg; + } } else { buf[i + 1] = current_arg;