From 8393608dd9f157ae25ee44777541e62fa80a6d82 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 9 Aug 2024 12:22:41 +0300 Subject: [PATCH] gh-122688: Fix support of var-positional parameter in Argument Clinic (GH-122689) * Parameters after the var-positional parameter are now keyword-only instead of positional-or-keyword. * Correctly calculate min_kw_only. * Raise errors for invalid combinations of the var-positional parameter with "*", "/" and deprecation markers. --- Lib/test/clinic.test.c | 12 +-- Lib/test/test_clinic.py | 96 ++++++++++++++++++++--- Modules/_testclinic.c | 41 +++++----- Modules/clinic/_testclinic.c.h | 110 ++++++++++++++++++--------- Objects/setobject.c | 18 ++--- Tools/clinic/libclinic/dsl_parser.py | 20 ++--- Tools/clinic/libclinic/parse_args.py | 2 +- 7 files changed, 203 insertions(+), 96 deletions(-) diff --git a/Lib/test/clinic.test.c b/Lib/test/clinic.test.c index 76214e6cda9..2a071f8485a 100644 --- a/Lib/test/clinic.test.c +++ b/Lib/test/clinic.test.c @@ -4134,8 +4134,8 @@ test_vararg_and_posonly a: object - *args: object / + *args: object [clinic start generated code]*/ @@ -4177,7 +4177,7 @@ exit: static PyObject * test_vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args) -/*[clinic end generated code: output=79b75dc07decc8d6 input=08dc2bf7afbf1613]*/ +/*[clinic end generated code: output=79b75dc07decc8d6 input=9cfa748bbff09877]*/ /*[clinic input] test_vararg @@ -4920,7 +4920,7 @@ Test_an_metho_arg_named_arg_impl(TestObj *self, int arg) /*[clinic input] Test.__init__ *args: object - / + Varargs init method. For example, nargs is translated to PyTuple_GET_SIZE. [clinic start generated code]*/ @@ -4958,14 +4958,14 @@ exit: static int Test___init___impl(TestObj *self, PyObject *args) -/*[clinic end generated code: output=0ed1009fe0dcf98d input=96c3ddc0cd38fc0c]*/ +/*[clinic end generated code: output=0ed1009fe0dcf98d input=2a8bd0033c9ac772]*/ /*[clinic input] @classmethod Test.__new__ *args: object - / + Varargs new method. For example, nargs is translated to PyTuple_GET_SIZE. [clinic start generated code]*/ @@ -5002,7 +5002,7 @@ exit: static PyObject * Test_impl(PyTypeObject *type, PyObject *args) -/*[clinic end generated code: output=8b219f6633e2a2e9 input=26a672e2e9750120]*/ +/*[clinic end generated code: output=8b219f6633e2a2e9 input=70ad829df3dd9b84]*/ /*[clinic input] diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 8d511abac5a..a7ba7f3d998 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -322,7 +322,7 @@ class ClinicWholeFileTest(TestCase): """ self.expect_failure(block, err, lineno=8) - def test_multiple_star_in_args(self): + def test_star_after_vararg(self): err = "'my_test_func' uses '*' more than once." block = """ /*[clinic input] @@ -336,6 +336,20 @@ class ClinicWholeFileTest(TestCase): """ self.expect_failure(block, err, lineno=6) + def test_vararg_after_star(self): + err = "'my_test_func' uses '*' more than once." + block = """ + /*[clinic input] + my_test_func + + pos_arg: object + * + *args: object + kw_arg: object + [clinic start generated code]*/ + """ + self.expect_failure(block, err, lineno=6) + def test_module_already_got_one(self): err = "Already defined module 'm'!" block = """ @@ -1787,13 +1801,43 @@ class ClinicParserTest(TestCase): ) self.expect_failure(block, err, lineno=4) + def test_parameters_required_after_depr_star3(self): + block = """ + module foo + foo.bar + a: int + * [from 3.14] + *args: object + b: int + Docstring. + """ + err = ( + "Function 'bar' specifies '* [from ...]' without " + "following parameters." + ) + self.expect_failure(block, err, lineno=4) + def test_depr_star_must_come_before_star(self): block = """ module foo foo.bar - this: int + a: int * * [from 3.14] + b: int + Docstring. + """ + err = "Function 'bar': '* [from ...]' must precede '*'" + self.expect_failure(block, err, lineno=4) + + def test_depr_star_must_come_before_vararg(self): + block = """ + module foo + foo.bar + a: int + *args: object + * [from 3.14] + b: int Docstring. """ err = "Function 'bar': '* [from ...]' must precede '*'" @@ -1908,7 +1952,7 @@ class ClinicParserTest(TestCase): err = "Function 'bar' uses '/' more than once." self.expect_failure(block, err) - def test_mix_star_and_slash(self): + def test_slash_after_star(self): block = """ module foo foo.bar @@ -1921,6 +1965,19 @@ class ClinicParserTest(TestCase): err = "Function 'bar': '/' must precede '*'" self.expect_failure(block, err) + def test_slash_after_vararg(self): + block = """ + module foo + foo.bar + x: int + y: int + *args: object + z: int + / + """ + err = "Function 'bar': '/' must precede '*'" + self.expect_failure(block, err) + def test_depr_star_must_come_after_slash(self): block = """ module foo @@ -1960,6 +2017,19 @@ class ClinicParserTest(TestCase): err = "Function 'bar': '/ [from ...]' must precede '*'" self.expect_failure(block, err, lineno=4) + def test_vararg_must_come_after_depr_slash(self): + block = """ + module foo + foo.bar + a: int + *args: object + / [from 3.14] + b: int + Docstring. + """ + err = "Function 'bar': '/ [from ...]' must precede '*'" + self.expect_failure(block, err, lineno=4) + def test_depr_slash_must_come_after_slash(self): block = """ module foo @@ -1987,7 +2057,7 @@ class ClinicParserTest(TestCase): self.expect_failure(block, err) def test_parameters_no_more_than_one_vararg(self): - err = "Too many var args" + err = "Function 'bar' uses '*' more than once." block = """ module foo foo.bar @@ -3319,13 +3389,6 @@ class ClinicFunctionalTest(unittest.TestCase): with self.assertRaises(TypeError): ac_tester.posonly_vararg(1, 2, 3, b=4) - def test_vararg_and_posonly(self): - with self.assertRaises(TypeError): - ac_tester.vararg_and_posonly() - with self.assertRaises(TypeError): - ac_tester.vararg_and_posonly(1, b=2) - self.assertEqual(ac_tester.vararg_and_posonly(1, 2, 3, 4), (1, (2, 3, 4))) - def test_vararg(self): with self.assertRaises(TypeError): ac_tester.vararg() @@ -3363,6 +3426,17 @@ class ClinicFunctionalTest(unittest.TestCase): self.assertEqual(ac_tester.vararg_with_only_defaults(1, 2, 3, 4), ((1, 2, 3, 4), None)) self.assertEqual(ac_tester.vararg_with_only_defaults(1, 2, 3, 4, b=5), ((1, 2, 3, 4), 5)) + def test_vararg_kwonly_req_opt(self): + fn = ac_tester.vararg_kwonly_req_opt + self.assertRaises(TypeError, fn) + self.assertEqual(fn(a=1), ((), 1, None, None)) + self.assertEqual(fn(a=1, b=2), ((), 1, 2, None)) + self.assertEqual(fn(a=1, b=2, c=3), ((), 1, 2, 3)) + self.assertRaises(TypeError, fn, 1) + self.assertEqual(fn(1, a=2), ((1,), 2, None, None)) + self.assertEqual(fn(1, a=2, b=3), ((1,), 2, 3, None)) + self.assertEqual(fn(1, a=2, b=3, c=4), ((1,), 2, 3, 4)) + def test_gh_32092_oob(self): ac_tester.gh_32092_oob(1, 2, 3, 4, kw1=5, kw2=6) diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index f6a2ac66f0f..2dae8accf01 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -981,23 +981,6 @@ posonly_vararg_impl(PyObject *module, PyObject *a, PyObject *b, } -/*[clinic input] -vararg_and_posonly - - a: object - *args: object - / - -[clinic start generated code]*/ - -static PyObject * -vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args) -/*[clinic end generated code: output=42792f799465a14d input=defe017b19ba52e8]*/ -{ - return pack_arguments_newref(2, a, args); -} - - /*[clinic input] vararg @@ -1068,6 +1051,25 @@ vararg_with_only_defaults_impl(PyObject *module, PyObject *args, PyObject *b) } +/*[clinic input] +vararg_kwonly_req_opt + + *args: object + a: object + b: object = None + c: object = None + +[clinic start generated code]*/ + +static PyObject * +vararg_kwonly_req_opt_impl(PyObject *module, PyObject *args, PyObject *a, + PyObject *b, PyObject *c) +/*[clinic end generated code: output=54694a99c3da370a input=b0d8bf09e540d400]*/ +{ + return pack_arguments_newref(4, args, a, b, c); +} + + /*[clinic input] gh_32092_oob @@ -1115,7 +1117,6 @@ gh_32092_kw_pass_impl(PyObject *module, PyObject *pos, PyObject *args, gh_99233_refcount *args: object - / Proof-of-concept of GH-99233 refcount error bug. @@ -1123,7 +1124,7 @@ Proof-of-concept of GH-99233 refcount error bug. static PyObject * gh_99233_refcount_impl(PyObject *module, PyObject *args) -/*[clinic end generated code: output=585855abfbca9a7f input=85f5fb47ac91a626]*/ +/*[clinic end generated code: output=585855abfbca9a7f input=eecfdc2092d90dc3]*/ { Py_RETURN_NONE; } @@ -1923,11 +1924,11 @@ static PyMethodDef tester_methods[] = { POSONLY_OPT_KEYWORDS_OPT_KWONLY_OPT_METHODDEF KEYWORD_ONLY_PARAMETER_METHODDEF POSONLY_VARARG_METHODDEF - VARARG_AND_POSONLY_METHODDEF VARARG_METHODDEF VARARG_WITH_DEFAULT_METHODDEF VARARG_WITH_DEFAULT2_METHODDEF VARARG_WITH_ONLY_DEFAULTS_METHODDEF + VARARG_KWONLY_REQ_OPT_METHODDEF GH_32092_OOB_METHODDEF GH_32092_KW_PASS_METHODDEF GH_99233_REFCOUNT_METHODDEF diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index 10c80daf75f..240342b438a 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2581,42 +2581,6 @@ exit: return return_value; } -PyDoc_STRVAR(vararg_and_posonly__doc__, -"vararg_and_posonly($module, a, /, *args)\n" -"--\n" -"\n"); - -#define VARARG_AND_POSONLY_METHODDEF \ - {"vararg_and_posonly", _PyCFunction_CAST(vararg_and_posonly), METH_FASTCALL, vararg_and_posonly__doc__}, - -static PyObject * -vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args); - -static PyObject * -vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t nargs) -{ - PyObject *return_value = NULL; - PyObject *a; - PyObject *__clinic_args = NULL; - - if (!_PyArg_CheckPositional("vararg_and_posonly", nargs, 1, PY_SSIZE_T_MAX)) { - goto exit; - } - a = args[0]; - __clinic_args = PyTuple_New(nargs - 1); - if (!__clinic_args) { - goto exit; - } - for (Py_ssize_t i = 0; i < nargs - 1; ++i) { - PyTuple_SET_ITEM(__clinic_args, i, Py_NewRef(args[1 + i])); - } - return_value = vararg_and_posonly_impl(module, a, __clinic_args); - -exit: - Py_XDECREF(__clinic_args); - return return_value; -} - PyDoc_STRVAR(vararg__doc__, "vararg($module, /, a, *args)\n" "--\n" @@ -2876,6 +2840,78 @@ exit: return return_value; } +PyDoc_STRVAR(vararg_kwonly_req_opt__doc__, +"vararg_kwonly_req_opt($module, /, *args, a, b=None, c=None)\n" +"--\n" +"\n"); + +#define VARARG_KWONLY_REQ_OPT_METHODDEF \ + {"vararg_kwonly_req_opt", _PyCFunction_CAST(vararg_kwonly_req_opt), METH_FASTCALL|METH_KEYWORDS, vararg_kwonly_req_opt__doc__}, + +static PyObject * +vararg_kwonly_req_opt_impl(PyObject *module, PyObject *args, PyObject *a, + PyObject *b, PyObject *c); + +static PyObject * +vararg_kwonly_req_opt(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_LATIN1_CHR('b'), _Py_LATIN1_CHR('c'), }, + }; + #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", "c", NULL}; + static _PyArg_Parser _parser = { + .keywords = _keywords, + .fname = "vararg_kwonly_req_opt", + .kwtuple = KWTUPLE, + }; + #undef KWTUPLE + PyObject *argsbuf[4]; + Py_ssize_t noptargs = 0 + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - 1; + PyObject *__clinic_args = NULL; + PyObject *a; + PyObject *b = Py_None; + PyObject *c = Py_None; + + args = _PyArg_UnpackKeywordsWithVararg(args, nargs, NULL, kwnames, &_parser, 0, 0, 1, 0, argsbuf); + if (!args) { + goto exit; + } + __clinic_args = args[0]; + a = args[1]; + if (!noptargs) { + goto skip_optional_kwonly; + } + if (args[2]) { + b = args[2]; + if (!--noptargs) { + goto skip_optional_kwonly; + } + } + c = args[3]; +skip_optional_kwonly: + return_value = vararg_kwonly_req_opt_impl(module, __clinic_args, a, b, c); + +exit: + Py_XDECREF(__clinic_args); + return return_value; +} + PyDoc_STRVAR(gh_32092_oob__doc__, "gh_32092_oob($module, /, pos1, pos2, *varargs, kw1=None, kw2=None)\n" "--\n" @@ -3490,4 +3526,4 @@ _testclinic_TestClass_get_defining_class_arg(PyObject *self, PyTypeObject *cls, exit: return return_value; } -/*[clinic end generated code: output=cf3b04bd2ecf6e26 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=cacfbed4b2e50ba6 input=a9049054013a1b77]*/ diff --git a/Objects/setobject.c b/Objects/setobject.c index 9c17e3eef2f..c5f96d25585 100644 --- a/Objects/setobject.c +++ b/Objects/setobject.c @@ -1049,14 +1049,13 @@ set_update_internal(PySetObject *so, PyObject *other) set.update so: setobject *others as args: object - / Update the set, adding elements from all others. [clinic start generated code]*/ static PyObject * set_update_impl(PySetObject *so, PyObject *args) -/*[clinic end generated code: output=34f6371704974c8a input=eb47c4fbaeb3286e]*/ +/*[clinic end generated code: output=34f6371704974c8a input=df4fe486e38cd337]*/ { Py_ssize_t i; @@ -1279,14 +1278,13 @@ set_clear_impl(PySetObject *so) set.union so: setobject *others as args: object - / Return a new set with elements from the set and all others. [clinic start generated code]*/ static PyObject * set_union_impl(PySetObject *so, PyObject *args) -/*[clinic end generated code: output=2c83d05a446a1477 input=2e2024fa1e40ac84]*/ +/*[clinic end generated code: output=2c83d05a446a1477 input=ddf088706e9577b2]*/ { PySetObject *result; PyObject *other; @@ -1429,14 +1427,13 @@ set_intersection(PySetObject *so, PyObject *other) set.intersection as set_intersection_multi so: setobject *others as args: object - / Return a new set with elements common to the set and all others. [clinic start generated code]*/ static PyObject * set_intersection_multi_impl(PySetObject *so, PyObject *args) -/*[clinic end generated code: output=2406ef3387adbe2f input=04108ea6d7f0532b]*/ +/*[clinic end generated code: output=2406ef3387adbe2f input=0d9f3805ccbba6a4]*/ { Py_ssize_t i; @@ -1477,14 +1474,13 @@ set_intersection_update(PySetObject *so, PyObject *other) set.intersection_update as set_intersection_update_multi so: setobject *others as args: object - / Update the set, keeping only elements found in it and all others. [clinic start generated code]*/ static PyObject * set_intersection_update_multi_impl(PySetObject *so, PyObject *args) -/*[clinic end generated code: output=251c1f729063609d input=ff8f119f97458d16]*/ +/*[clinic end generated code: output=251c1f729063609d input=223c1e086aa669a9]*/ { PyObject *tmp; @@ -1665,14 +1661,13 @@ set_difference_update_internal(PySetObject *so, PyObject *other) set.difference_update so: setobject *others as args: object - / Update the set, removing elements found in others. [clinic start generated code]*/ static PyObject * set_difference_update_impl(PySetObject *so, PyObject *args) -/*[clinic end generated code: output=28685b2fc63e41c4 input=e7abb43c9f2c5a73]*/ +/*[clinic end generated code: output=28685b2fc63e41c4 input=024e6baa6fbcbb3d]*/ { Py_ssize_t i; @@ -1783,14 +1778,13 @@ set_difference(PySetObject *so, PyObject *other) set.difference as set_difference_multi so: setobject *others as args: object - / Return a new set with elements in the set that are not in the others. [clinic start generated code]*/ static PyObject * set_difference_multi_impl(PySetObject *so, PyObject *args) -/*[clinic end generated code: output=3130c3bb3cac873d input=d8ae9bb6d518ab95]*/ +/*[clinic end generated code: output=3130c3bb3cac873d input=ba78ea5f099e58df]*/ { Py_ssize_t i; PyObject *result, *other; diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index ab9b586693d..5ca3bd5cb6c 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -915,8 +915,8 @@ class DSLParser: f"invalid parameter declaration (**kwargs?): {line!r}") if function_args.vararg: - if any(p.is_vararg() for p in self.function.parameters.values()): - fail("Too many var args") + self.check_previous_star() + self.check_remaining_star() is_vararg = True parameter = function_args.vararg else: @@ -1124,6 +1124,9 @@ class DSLParser: key = f"{parameter_name}_as_{c_name}" if c_name else parameter_name self.function.parameters[key] = p + if is_vararg: + self.keyword_only = True + @staticmethod def parse_converter( annotation: ast.expr | None @@ -1165,8 +1168,6 @@ class DSLParser: the marker will take effect (None means it is already in effect). """ if version is None: - if self.keyword_only: - fail(f"Function {function.name!r} uses '*' more than once.") self.check_previous_star() self.check_remaining_star() self.keyword_only = True @@ -1456,6 +1457,7 @@ class DSLParser: if p.is_vararg(): p_lines.append("*") + added_star = True name = p.converter.signature_name or p.name p_lines.append(name) @@ -1565,7 +1567,8 @@ class DSLParser: for p in reversed(self.function.parameters.values()): if self.keyword_only: - if p.kind == inspect.Parameter.KEYWORD_ONLY: + if (p.kind == inspect.Parameter.KEYWORD_ONLY or + p.kind == inspect.Parameter.VAR_POSITIONAL): return elif self.deprecated_positional: if p.deprecated_positional == self.deprecated_positional: @@ -1575,12 +1578,11 @@ class DSLParser: fail(f"Function {self.function.name!r} specifies {symbol!r} " f"without following parameters.", line_number=lineno) - def check_previous_star(self, lineno: int | None = None) -> None: + def check_previous_star(self) -> None: assert isinstance(self.function, Function) - for p in self.function.parameters.values(): - if p.kind == inspect.Parameter.VAR_POSITIONAL: - fail(f"Function {self.function.name!r} uses '*' more than once.") + if self.keyword_only: + fail(f"Function {self.function.name!r} uses '*' more than once.") def do_post_block_processing_cleanup(self, lineno: int) -> None: diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index 0f67901dd86..96c9b919bff 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -262,7 +262,7 @@ class ParseArgsCodeGen: if p.is_keyword_only(): assert not p.is_positional_only() if not p.is_optional(): - self.min_kw_only = i - self.max_pos + self.min_kw_only = i - self.max_pos - int(self.vararg != NO_VARARG) elif p.is_vararg(): self.pseudo_args += 1 self.vararg = i - 1