From b051ae85c834feab4018ab8b0b6a989ad4036b1d Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Mon, 1 Sep 2025 15:34:52 +0100 Subject: [PATCH 01/10] Argument Clinic: Add support for ``**kwds`` This adds a scaffold of support, initially only working with strictly positional-only arguments. The FASTCALL calling convention is not yet supported. --- Modules/_testclinic.c | 88 ++++++++++++++++ Modules/clinic/_testclinic_kwds.c.h | 151 +++++++++++++++++++++++++++ Tools/clinic/libclinic/converter.py | 2 +- Tools/clinic/libclinic/converters.py | 27 +++++ Tools/clinic/libclinic/dsl_parser.py | 40 +++++-- Tools/clinic/libclinic/function.py | 3 + Tools/clinic/libclinic/parse_args.py | 69 +++++++++++- 7 files changed, 364 insertions(+), 16 deletions(-) create mode 100644 Modules/clinic/_testclinic_kwds.c.h diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index 3e903b6d87d89f..d033be9ae67be0 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -2303,6 +2303,88 @@ depr_multi_impl(PyObject *module, PyObject *a, PyObject *b, PyObject *c, #undef _SAVED_PY_VERSION +/*[clinic input] +output pop +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=e7c7c42daced52b0]*/ + + +/*[clinic input] +output push +destination kwarg new file '{dirname}/clinic/_testclinic_kwds.c.h' +output everything kwarg +output docstring_prototype suppress +output parser_prototype suppress +output impl_definition block +[clinic start generated code]*/ +/*[clinic end generated code: output=da39a3ee5e6b4b0d input=02965b54b3981cc4]*/ + +#include "clinic/_testclinic_kwds.c.h" + + +/*[clinic input] +lone_kwds + **kwds: dict +[clinic start generated code]*/ + +static PyObject * +lone_kwds_impl(PyObject *module, PyObject *kwds) +/*[clinic end generated code: output=572549c687a0432e input=6ef338b913ecae17]*/ +{ + Py_RETURN_NONE; +} + + +/*[clinic input] +kwds_with_pos_only + a: object + b: object + / + **kwds: dict +[clinic start generated code]*/ + +static PyObject * +kwds_with_pos_only_impl(PyObject *module, PyObject *a, PyObject *b, + PyObject *kwds) +/*[clinic end generated code: output=573096d3a7efcce5 input=da081a5d9ae8878a]*/ +{ + Py_RETURN_NONE; +} + + +/*[clinic input] +kwds_with_stararg + *args: tuple + **kwds: dict +[clinic start generated code]*/ + +static PyObject * +kwds_with_stararg_impl(PyObject *module, PyObject *args, PyObject *kwds) +/*[clinic end generated code: output=d4b0064626a25208 input=1be404572d685859]*/ +{ + Py_RETURN_NONE; +} + + +/*[clinic input] +kwds_with_pos_only_and_stararg + a: object + b: object + / + *args: tuple + **kwds: dict +[clinic start generated code]*/ + +static PyObject * +kwds_with_pos_only_and_stararg_impl(PyObject *module, PyObject *a, + PyObject *b, PyObject *args, + PyObject *kwds) +/*[clinic end generated code: output=af7df7640c792246 input=2fe330c7981f0829]*/ +{ + Py_RETURN_NONE; +} + + /*[clinic input] output pop [clinic start generated code]*/ @@ -2399,6 +2481,12 @@ static PyMethodDef tester_methods[] = { DEPR_KWD_NOINLINE_METHODDEF DEPR_KWD_MULTI_METHODDEF DEPR_MULTI_METHODDEF + + LONE_KWDS_METHODDEF + KWDS_WITH_POS_ONLY_METHODDEF + KWDS_WITH_STARARG_METHODDEF + KWDS_WITH_POS_ONLY_AND_STARARG_METHODDEF + {NULL, NULL} }; diff --git a/Modules/clinic/_testclinic_kwds.c.h b/Modules/clinic/_testclinic_kwds.c.h new file mode 100644 index 00000000000000..de3bdddab2b940 --- /dev/null +++ b/Modules/clinic/_testclinic_kwds.c.h @@ -0,0 +1,151 @@ +/*[clinic input] +preserve +[clinic start generated code]*/ + +#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) +# include "pycore_gc.h" // PyGC_Head +#endif +#include "pycore_abstract.h" // _PyNumber_Index() +#include "pycore_long.h" // _PyLong_UnsignedShort_Converter() +#include "pycore_modsupport.h" // _PyArg_CheckPositional() +#include "pycore_runtime.h" // _Py_ID() +#include "pycore_tuple.h" // _PyTuple_FromArray() + +PyDoc_STRVAR(lone_kwds__doc__, +"lone_kwds($module, /, **kwds)\n" +"--\n" +"\n"); + +#define LONE_KWDS_METHODDEF \ + {"lone_kwds", _PyCFunction_CAST(lone_kwds), METH_VARARGS|METH_KEYWORDS, lone_kwds__doc__}, + +static PyObject * +lone_kwds_impl(PyObject *module, PyObject *kwds); + +static PyObject * +lone_kwds(PyObject *module, PyObject *args, PyObject *kwargs) +{ + PyObject *return_value = NULL; + PyObject *kwds = NULL; + + if (!_PyArg_NoPositional("lone_kwds", args)) { + goto exit; + } + kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); + return_value = lone_kwds_impl(module, kwds); + +exit: + /* Cleanup for kwds */ + Py_XDECREF(kwds); + + return return_value; +} + +PyDoc_STRVAR(kwds_with_pos_only__doc__, +"kwds_with_pos_only($module, a, b, /, **kwds)\n" +"--\n" +"\n"); + +#define KWDS_WITH_POS_ONLY_METHODDEF \ + {"kwds_with_pos_only", _PyCFunction_CAST(kwds_with_pos_only), METH_VARARGS|METH_KEYWORDS, kwds_with_pos_only__doc__}, + +static PyObject * +kwds_with_pos_only_impl(PyObject *module, PyObject *a, PyObject *b, + PyObject *kwds); + +static PyObject * +kwds_with_pos_only(PyObject *module, PyObject *args, PyObject *kwargs) +{ + PyObject *return_value = NULL; + PyObject *a; + PyObject *b; + PyObject *kwds = NULL; + + if (!_PyArg_CheckPositional("kwds_with_pos_only", PyTuple_GET_SIZE(args), 2, 2)) { + goto exit; + } + a = PyTuple_GET_ITEM(args, 0); + b = PyTuple_GET_ITEM(args, 1); + kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); + return_value = kwds_with_pos_only_impl(module, a, b, kwds); + +exit: + /* Cleanup for kwds */ + Py_XDECREF(kwds); + + return return_value; +} + +PyDoc_STRVAR(kwds_with_stararg__doc__, +"kwds_with_stararg($module, /, *args, **kwds)\n" +"--\n" +"\n"); + +#define KWDS_WITH_STARARG_METHODDEF \ + {"kwds_with_stararg", _PyCFunction_CAST(kwds_with_stararg), METH_VARARGS|METH_KEYWORDS, kwds_with_stararg__doc__}, + +static PyObject * +kwds_with_stararg_impl(PyObject *module, PyObject *args, PyObject *kwds); + +static PyObject * +kwds_with_stararg(PyObject *module, PyObject *args, PyObject *kwargs) +{ + PyObject *return_value = NULL; + PyObject *__clinic_args = NULL; + PyObject *kwds = NULL; + + __clinic_args = Py_NewRef(args); + kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); + return_value = kwds_with_stararg_impl(module, __clinic_args, kwds); + + /* Cleanup for args */ + Py_XDECREF(__clinic_args); + /* Cleanup for kwds */ + Py_XDECREF(kwds); + + return return_value; +} + +PyDoc_STRVAR(kwds_with_pos_only_and_stararg__doc__, +"kwds_with_pos_only_and_stararg($module, a, b, /, *args, **kwds)\n" +"--\n" +"\n"); + +#define KWDS_WITH_POS_ONLY_AND_STARARG_METHODDEF \ + {"kwds_with_pos_only_and_stararg", _PyCFunction_CAST(kwds_with_pos_only_and_stararg), METH_VARARGS|METH_KEYWORDS, kwds_with_pos_only_and_stararg__doc__}, + +static PyObject * +kwds_with_pos_only_and_stararg_impl(PyObject *module, PyObject *a, + PyObject *b, PyObject *args, + PyObject *kwds); + +static PyObject * +kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwargs) +{ + PyObject *return_value = NULL; + PyObject *a; + PyObject *b; + PyObject *__clinic_args = NULL; + PyObject *kwds = NULL; + + if (!_PyArg_CheckPositional("kwds_with_pos_only_and_stararg", PyTuple_GET_SIZE(args), 2, PY_SSIZE_T_MAX)) { + goto exit; + } + a = PyTuple_GET_ITEM(args, 0); + b = PyTuple_GET_ITEM(args, 1); + __clinic_args = PyTuple_GetSlice(args, 2, PY_SSIZE_T_MAX); + if (!__clinic_args) { + goto exit; + } + kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); + return_value = kwds_with_pos_only_and_stararg_impl(module, a, b, __clinic_args, kwds); + +exit: + /* Cleanup for args */ + Py_XDECREF(__clinic_args); + /* Cleanup for kwds */ + Py_XDECREF(kwds); + + return return_value; +} +/*[clinic end generated code: output=0a1e2c2244a10a50 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/libclinic/converter.py b/Tools/clinic/libclinic/converter.py index 2c93dda3541030..5c7365d3e3683f 100644 --- a/Tools/clinic/libclinic/converter.py +++ b/Tools/clinic/libclinic/converter.py @@ -274,7 +274,7 @@ def _render_non_self( data.modifications.append('/* modifications for ' + name + ' */\n' + modifications.rstrip()) # keywords - if parameter.is_vararg(): + if parameter.is_vararg() or parameter.is_var_keyword(): pass elif parameter.is_positional_only(): data.keywords.append('') diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py index 6e89e8de7cccf1..74165ed2691780 100644 --- a/Tools/clinic/libclinic/converters.py +++ b/Tools/clinic/libclinic/converters.py @@ -1279,3 +1279,30 @@ def parse_vararg(self, *, pos_only: int, min_pos: int, max_pos: int, {paramname} = {start}; {self.length_name} = {size}; """ + + +# Converters for var-keyword parameters. + +class VarKeywordCConverter(CConverter): + format_unit = '' + + def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> str | None: + raise AssertionError('should never be called') + + def parse_var_keyword(self) -> str: + raise NotImplementedError + + +class var_keyword_dict_converter(VarKeywordCConverter): + type = 'PyObject *' + format_unit = '' + c_default = 'NULL' + + def cleanup(self) -> str: + return f'Py_XDECREF({self.parser_name});\n' + + def parse_var_keyword(self) -> str: + param_name = self.parser_name + return f""" + {param_name} = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); + """ diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index f9587d20383c7a..f26b941a1a41fc 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -909,27 +909,37 @@ def parse_parameter(self, line: str) -> None: if len(function_args.args) > 1: fail(f"Function {self.function.name!r} has an " f"invalid parameter declaration (comma?): {line!r}") - if function_args.kwarg: - fail(f"Function {self.function.name!r} has an " - f"invalid parameter declaration (**kwargs?): {line!r}") + is_vararg = is_var_keyword = False if function_args.vararg: self.check_previous_star() self.check_remaining_star() is_vararg = True parameter = function_args.vararg + elif function_args.kwarg: + # If the existing parameters are all positional only or ``*args`` + # (var-positional), then we allow ``**kwds`` (var-keyword). + # Currently, pos-or-keyword or keyword-only arguments are not + # allowed with the ``**kwds`` converter. + if not all(p.is_positional_only() or p.is_vararg() + for p in self.function.parameters.values()): + fail(f"Function {self.function.name!r} has an " + f"invalid parameter declaration (**kwargs?): {line!r}") + is_var_keyword = True + parameter = function_args.kwarg else: - is_vararg = False parameter = function_args.args[0] parameter_name = parameter.arg name, legacy, kwargs = self.parse_converter(parameter.annotation) if is_vararg: - name = 'varpos_' + name + name = f'varpos_{name}' + elif is_var_keyword: + name = f'var_keyword_{name}' value: object if not function_args.defaults: - if is_vararg: + if is_vararg or is_var_keyword: value = NULL else: if self.parameter_state is ParamState.OPTIONAL: @@ -1065,6 +1075,8 @@ def bad_node(self, node: ast.AST) -> None: kind: inspect._ParameterKind if is_vararg: kind = inspect.Parameter.VAR_POSITIONAL + elif is_var_keyword: + kind = inspect.Parameter.VAR_KEYWORD elif self.keyword_only: kind = inspect.Parameter.KEYWORD_ONLY else: @@ -1116,7 +1128,7 @@ def bad_node(self, node: ast.AST) -> None: key = f"{parameter_name}_as_{c_name}" if c_name else parameter_name self.function.parameters[key] = p - if is_vararg: + if is_vararg or is_var_keyword: self.keyword_only = True @staticmethod @@ -1450,11 +1462,16 @@ def add_parameter(text: str) -> None: if p.is_vararg(): p_lines.append("*") added_star = True + if p.is_var_keyword(): + p_lines.append("**") name = p.converter.signature_name or p.name p_lines.append(name) - if not p.is_vararg() and p.converter.is_optional(): + if ( + not (p.is_vararg() or p.is_var_keyword()) + and p.converter.is_optional() + ): p_lines.append('=') value = p.converter.py_default if not value: @@ -1583,8 +1600,11 @@ def check_remaining_star(self, lineno: int | None = None) -> None: for p in reversed(self.function.parameters.values()): if self.keyword_only: - if (p.kind == inspect.Parameter.KEYWORD_ONLY or - p.kind == inspect.Parameter.VAR_POSITIONAL): + if p.kind in { + inspect.Parameter.KEYWORD_ONLY, + inspect.Parameter.VAR_POSITIONAL, + inspect.Parameter.VAR_KEYWORD + }: return elif self.deprecated_positional: if p.deprecated_positional == self.deprecated_positional: diff --git a/Tools/clinic/libclinic/function.py b/Tools/clinic/libclinic/function.py index 4280af0c4c9b49..573689e1c459cd 100644 --- a/Tools/clinic/libclinic/function.py +++ b/Tools/clinic/libclinic/function.py @@ -223,6 +223,9 @@ def is_positional_only(self) -> bool: def is_vararg(self) -> bool: return self.kind == inspect.Parameter.VAR_POSITIONAL + def is_var_keyword(self) -> bool: + return self.kind == inspect.Parameter.VAR_KEYWORD + def is_optional(self) -> bool: return not self.is_vararg() and (self.default is not unspecified) diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index 0e15d2f163b816..ce9a2cc3a53a5f 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -34,10 +34,10 @@ def declare_parser( fname = '.fname = "{name}",' format_ = '' - num_keywords = len([ - p for p in f.parameters.values() - if not p.is_positional_only() and not p.is_vararg() - ]) + num_keywords = sum( + 1 for p in f.parameters.values() + if not p.is_positional_only() and not (p.is_vararg() or p.is_var_keyword()) + ) condition = '#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)' if limited_capi: @@ -220,6 +220,7 @@ class ParseArgsCodeGen: max_pos: int = 0 min_kw_only: int = 0 varpos: Parameter | None = None + var_keyword: Parameter | None = None docstring_prototype: str docstring_definition: str @@ -255,6 +256,12 @@ def __init__(self, func: Function, codegen: CodeGen) -> None: del self.parameters[i] break + for i, p in enumerate(self.parameters): + if p.is_var_keyword(): + self.var_keyword = p + del self.parameters[i] + break + self.converters = [p.converter for p in self.parameters] if self.func.critical_section: @@ -262,6 +269,9 @@ def __init__(self, func: Function, codegen: CodeGen) -> None: 'Py_BEGIN_CRITICAL_SECTION()') if self.func.disable_fastcall: self.fastcall = False + elif self.var_keyword is not None: + has_args = self.parameters or self.varpos + self.fastcall = not has_args else: self.fastcall = not self.is_new_or_init() @@ -469,6 +479,12 @@ def _parse_vararg(self) -> str: fastcall=self.fastcall, limited_capi=self.limited_capi) + def _parse_kwarg(self) -> str: + assert self.var_keyword is not None + c = self.var_keyword.converter + assert isinstance(c, libclinic.converters.VarKeywordCConverter) + return c.parse_var_keyword() + def parse_pos_only(self) -> None: if self.fastcall: # positional-only, but no option groups @@ -564,6 +580,8 @@ def parse_pos_only(self) -> None: parser_code.append("skip_optional:") if self.varpos: parser_code.append(libclinic.normalize_snippet(self._parse_vararg(), indent=4)) + elif self.var_keyword: + parser_code.append(libclinic.normalize_snippet(self._parse_kwarg(), indent=4)) else: for parameter in self.parameters: parameter.converter.use_converter() @@ -590,6 +608,45 @@ def parse_pos_only(self) -> None: """, indent=4)] self.parser_body(*parser_code) + def parse_var_keyword(self) -> None: + self.flags = "METH_VARARGS|METH_KEYWORDS" + self.parser_prototype = PARSER_PROTOTYPE_KEYWORD + nargs = 'PyTuple_GET_SIZE(args)' + + parser_code = [] + max_args = NO_VARARG if self.varpos else self.max_pos + if self.varpos is None and self.min_pos == self.max_pos == 0: + self.codegen.add_include('pycore_modsupport.h', + '_PyArg_NoPositional()') + parser_code.append(libclinic.normalize_snippet(""" + if (!_PyArg_NoPositional("{name}", args)) {{ + goto exit; + }} + """, indent=4)) + elif self.min_pos or max_args != NO_VARARG: + self.codegen.add_include('pycore_modsupport.h', + '_PyArg_CheckPositional()') + parser_code.append(libclinic.normalize_snippet(f""" + if (!_PyArg_CheckPositional("{{name}}", {nargs}, {self.min_pos}, {max_args})) {{{{ + goto exit; + }}}} + """, indent=4)) + + for i, p in enumerate(self.parameters): + parse_arg = p.converter.parse_arg( + f'PyTuple_GET_ITEM(args, {i})', + p.get_displayname(i+1), + limited_capi=self.limited_capi, + ) + assert parse_arg is not None + parser_code.append(libclinic.normalize_snippet(parse_arg, indent=4)) + + if self.varpos: + parser_code.append(libclinic.normalize_snippet(self._parse_vararg(), indent=4)) + if self.var_keyword: + parser_code.append(libclinic.normalize_snippet(self._parse_kwarg(), indent=4)) + self.parser_body(*parser_code) + def parse_general(self, clang: CLanguage) -> None: parsearg: str | None deprecated_positionals: dict[int, Parameter] = {} @@ -921,12 +978,14 @@ def parse_args(self, clang: CLanguage) -> dict[str, str]: # previous call to parser_body. this is used for an awful hack. self.parser_body_fields: tuple[str, ...] = () - if not self.parameters and not self.varpos: + if not self.parameters and not self.varpos and not self.var_keyword: self.parse_no_args() elif self.use_meth_o(): self.parse_one_arg() elif self.has_option_groups(): self.parse_option_groups() + elif self.var_keyword is not None: + self.parse_var_keyword() elif (not self.requires_defining_class and self.pos_only == len(self.parameters)): self.parse_pos_only() From b6785c43332b1ccf6289facae9bf4287c7e27708 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 3 Sep 2025 16:59:51 +0100 Subject: [PATCH 02/10] revert to len() --- Tools/clinic/libclinic/parse_args.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index ce9a2cc3a53a5f..89a5eac5c2e14f 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -34,10 +34,10 @@ def declare_parser( fname = '.fname = "{name}",' format_ = '' - num_keywords = sum( - 1 for p in f.parameters.values() + num_keywords = len([ + p for p in f.parameters.values() if not p.is_positional_only() and not (p.is_vararg() or p.is_var_keyword()) - ) + ]) condition = '#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)' if limited_capi: From 9e02b3f0edb8b6aea81a1e4ea439651bd09b0aa3 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:09:56 +0100 Subject: [PATCH 03/10] Add ``Parameter.is_variable_length()`` --- Tools/clinic/libclinic/converter.py | 2 +- Tools/clinic/libclinic/dsl_parser.py | 5 +---- Tools/clinic/libclinic/function.py | 12 +++++++++--- Tools/clinic/libclinic/parse_args.py | 2 +- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Tools/clinic/libclinic/converter.py b/Tools/clinic/libclinic/converter.py index 5c7365d3e3683f..ac66e79f93b735 100644 --- a/Tools/clinic/libclinic/converter.py +++ b/Tools/clinic/libclinic/converter.py @@ -274,7 +274,7 @@ def _render_non_self( data.modifications.append('/* modifications for ' + name + ' */\n' + modifications.rstrip()) # keywords - if parameter.is_vararg() or parameter.is_var_keyword(): + if parameter.is_variable_length(): pass elif parameter.is_positional_only(): data.keywords.append('') diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index f26b941a1a41fc..6ea1fb61624c5b 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -1468,10 +1468,7 @@ def add_parameter(text: str) -> None: name = p.converter.signature_name or p.name p_lines.append(name) - if ( - not (p.is_vararg() or p.is_var_keyword()) - and p.converter.is_optional() - ): + if not p.is_variable_length() and p.converter.is_optional(): p_lines.append('=') value = p.converter.py_default if not value: diff --git a/Tools/clinic/libclinic/function.py b/Tools/clinic/libclinic/function.py index 573689e1c459cd..2901ac519e2e55 100644 --- a/Tools/clinic/libclinic/function.py +++ b/Tools/clinic/libclinic/function.py @@ -214,18 +214,24 @@ class Parameter: def __repr__(self) -> str: return f'' - def is_keyword_only(self) -> bool: - return self.kind == inspect.Parameter.KEYWORD_ONLY - def is_positional_only(self) -> bool: return self.kind == inspect.Parameter.POSITIONAL_ONLY + def is_positional_or_keyword(self) -> bool: + return self.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD + + def is_keyword_only(self) -> bool: + return self.kind == inspect.Parameter.KEYWORD_ONLY + def is_vararg(self) -> bool: return self.kind == inspect.Parameter.VAR_POSITIONAL def is_var_keyword(self) -> bool: return self.kind == inspect.Parameter.VAR_KEYWORD + def is_variable_length(self) -> bool: + return self.is_vararg() or self.is_var_keyword() + def is_optional(self) -> bool: return not self.is_vararg() and (self.default is not unspecified) diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index 89a5eac5c2e14f..fb9dea08deee3c 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -36,7 +36,7 @@ def declare_parser( num_keywords = len([ p for p in f.parameters.values() - if not p.is_positional_only() and not (p.is_vararg() or p.is_var_keyword()) + if p.is_positional_or_keyword() or p.is_keyword_only() ]) condition = '#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)' From 8939a899eea46ee0fe20c6d2a2859e18e2af51a3 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 12:30:48 +0100 Subject: [PATCH 04/10] Review (part 1) --- Modules/clinic/_testclinic_kwds.c.h | 59 ++++++++++++++++++++-------- Tools/clinic/libclinic/__init__.py | 1 + Tools/clinic/libclinic/converters.py | 8 +++- Tools/clinic/libclinic/dsl_parser.py | 10 ++++- 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/Modules/clinic/_testclinic_kwds.c.h b/Modules/clinic/_testclinic_kwds.c.h index de3bdddab2b940..274fdcc8fd8082 100644 --- a/Modules/clinic/_testclinic_kwds.c.h +++ b/Modules/clinic/_testclinic_kwds.c.h @@ -26,17 +26,23 @@ static PyObject * lone_kwds(PyObject *module, PyObject *args, PyObject *kwargs) { PyObject *return_value = NULL; - PyObject *kwds = NULL; + PyObject *__clinic_kwds = NULL; if (!_PyArg_NoPositional("lone_kwds", args)) { goto exit; } - kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); - return_value = lone_kwds_impl(module, kwds); + if (kwargs == NULL) { + __clinic_kwds = PyDict_New(); + if (__clinic_kwds == NULL) goto exit; + } + else { + __clinic_kwds = Py_NewRef(kwargs); + } + return_value = lone_kwds_impl(module, __clinic_kwds); exit: /* Cleanup for kwds */ - Py_XDECREF(kwds); + Py_XDECREF(__clinic_kwds); return return_value; } @@ -59,19 +65,25 @@ kwds_with_pos_only(PyObject *module, PyObject *args, PyObject *kwargs) PyObject *return_value = NULL; PyObject *a; PyObject *b; - PyObject *kwds = NULL; + PyObject *__clinic_kwds = NULL; if (!_PyArg_CheckPositional("kwds_with_pos_only", PyTuple_GET_SIZE(args), 2, 2)) { goto exit; } a = PyTuple_GET_ITEM(args, 0); b = PyTuple_GET_ITEM(args, 1); - kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); - return_value = kwds_with_pos_only_impl(module, a, b, kwds); + if (kwargs == NULL) { + __clinic_kwds = PyDict_New(); + if (__clinic_kwds == NULL) goto exit; + } + else { + __clinic_kwds = Py_NewRef(kwargs); + } + return_value = kwds_with_pos_only_impl(module, a, b, __clinic_kwds); exit: /* Cleanup for kwds */ - Py_XDECREF(kwds); + Py_XDECREF(__clinic_kwds); return return_value; } @@ -92,16 +104,23 @@ kwds_with_stararg(PyObject *module, PyObject *args, PyObject *kwargs) { PyObject *return_value = NULL; PyObject *__clinic_args = NULL; - PyObject *kwds = NULL; + PyObject *__clinic_kwds = NULL; __clinic_args = Py_NewRef(args); - kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); - return_value = kwds_with_stararg_impl(module, __clinic_args, kwds); + if (kwargs == NULL) { + __clinic_kwds = PyDict_New(); + if (__clinic_kwds == NULL) goto exit; + } + else { + __clinic_kwds = Py_NewRef(kwargs); + } + return_value = kwds_with_stararg_impl(module, __clinic_args, __clinic_kwds); +exit: /* Cleanup for args */ Py_XDECREF(__clinic_args); /* Cleanup for kwds */ - Py_XDECREF(kwds); + Py_XDECREF(__clinic_kwds); return return_value; } @@ -126,7 +145,7 @@ kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwarg PyObject *a; PyObject *b; PyObject *__clinic_args = NULL; - PyObject *kwds = NULL; + PyObject *__clinic_kwds = NULL; if (!_PyArg_CheckPositional("kwds_with_pos_only_and_stararg", PyTuple_GET_SIZE(args), 2, PY_SSIZE_T_MAX)) { goto exit; @@ -137,15 +156,21 @@ kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwarg if (!__clinic_args) { goto exit; } - kwds = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); - return_value = kwds_with_pos_only_and_stararg_impl(module, a, b, __clinic_args, kwds); + if (kwargs == NULL) { + __clinic_kwds = PyDict_New(); + if (__clinic_kwds == NULL) goto exit; + } + else { + __clinic_kwds = Py_NewRef(kwargs); + } + return_value = kwds_with_pos_only_and_stararg_impl(module, a, b, __clinic_args, __clinic_kwds); exit: /* Cleanup for args */ Py_XDECREF(__clinic_args); /* Cleanup for kwds */ - Py_XDECREF(kwds); + Py_XDECREF(__clinic_kwds); return return_value; } -/*[clinic end generated code: output=0a1e2c2244a10a50 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=c391b2a6f7d5ab41 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/libclinic/__init__.py b/Tools/clinic/libclinic/__init__.py index 7c5cede2396677..9e9bdeadcc0fe1 100644 --- a/Tools/clinic/libclinic/__init__.py +++ b/Tools/clinic/libclinic/__init__.py @@ -84,6 +84,7 @@ "argsbuf", "fastargs", "kwargs", + "kwds", "kwnames", "nargs", "noptargs", diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py index 74165ed2691780..ef70ede0d7d819 100644 --- a/Tools/clinic/libclinic/converters.py +++ b/Tools/clinic/libclinic/converters.py @@ -1304,5 +1304,11 @@ def cleanup(self) -> str: def parse_var_keyword(self) -> str: param_name = self.parser_name return f""" - {param_name} = (kwargs != NULL) ? Py_NewRef(kwargs) : PyDict_New(); + if (kwargs == NULL) {{{{ + {param_name} = PyDict_New(); + if ({param_name} == NULL) goto exit; + }}}} + else {{{{ + {param_name} = Py_NewRef(kwargs); + }}}} """ diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index 6ea1fb61624c5b..644eb3d85e19eb 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -246,6 +246,7 @@ def dedent(self, line: str) -> str: class DSLParser: function: Function | None state: StateKeeper + expecting_parameters: bool keyword_only: bool positional_only: bool deprecated_positional: VersionTuple | None @@ -285,6 +286,7 @@ def __init__(self, clinic: Clinic) -> None: def reset(self) -> None: self.function = None self.state = self.state_dsl_start + self.expecting_parameters = True self.keyword_only = False self.positional_only = False self.deprecated_positional = None @@ -848,6 +850,10 @@ def state_parameter(self, line: str) -> None: # we indented, must be to new parameter docstring column return self.next(self.state_parameter_docstring_start, line) + if not self.expecting_parameters: + fail('Encountered parameter line when not expecting ' + f'parameters: {line}') + line = line.rstrip() if line.endswith('\\'): self.parameter_continuation = line[:-1] @@ -1128,8 +1134,10 @@ def bad_node(self, node: ast.AST) -> None: key = f"{parameter_name}_as_{c_name}" if c_name else parameter_name self.function.parameters[key] = p - if is_vararg or is_var_keyword: + if is_vararg: self.keyword_only = True + if is_var_keyword: + self.expecting_parameters = False @staticmethod def parse_converter( From f2efcbd22fcab992b5bbbe5a970b5143493af460 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 12:46:04 +0100 Subject: [PATCH 05/10] Review (part 2): add more tests --- Lib/test/test_clinic.py | 134 +++++++++++++++++++++++++++ Tools/clinic/libclinic/dsl_parser.py | 14 ++- 2 files changed, 144 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index f8d9b0af8f92ec..455ed42e793435 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -357,6 +357,32 @@ def test_vararg_after_star(self): """ self.expect_failure(block, err, lineno=6) + def test_double_star_after_var_keyword(self): + err = "Function 'my_test_func' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + block = """ + /*[clinic input] + my_test_func + + pos_arg: object + **kwds: dict + ** + [clinic start generated code]*/ + """ + self.expect_failure(block, err, lineno=5) + + def test_var_keyword_after_star(self): + err = "Function 'my_test_func' has an invalid parameter declaration: '**'" + block = """ + /*[clinic input] + my_test_func + + pos_arg: object + ** + **kwds: dict + [clinic start generated code]*/ + """ + self.expect_failure(block, err, lineno=5) + def test_module_already_got_one(self): err = "Already defined module 'm'!" block = """ @@ -748,6 +774,16 @@ def test_ignore_preprocessor_in_comments(self): """) self.clinic.parse(raw) + def test_var_keyword_non_dict(self): + err = "'var_keyword_object' is not a valid converter" + block = """ + /*[clinic input] + my_test_func + + **kwds: object + [clinic start generated code]*/ + """ + self.expect_failure(block, err, lineno=4) class ParseFileUnitTest(TestCase): def expect_parsing_failure( @@ -1608,6 +1644,11 @@ def test_disallowed_grouping__must_be_position_only(self): [ a: object ] + """, """ + with_kwds + [ + **kwds: dict + ] """) err = ( "You cannot use optional groups ('[' and ']') unless all " @@ -1991,6 +2032,44 @@ def test_slash_after_vararg(self): err = "Function 'bar': '/' must precede '*'" self.expect_failure(block, err) + def test_slash_after_var_keyword(self): + block = """ + module foo + foo.bar + x: int + y: int + **kwds: dict + z: int + / + """ + err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + self.expect_failure(block, err) + + def test_star_after_var_keyword(self): + block = """ + module foo + foo.bar + x: int + y: int + **kwds: dict + z: int + * + """ + err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + self.expect_failure(block, err) + + def test_parameter_after_var_keyword(self): + block = """ + module foo + foo.bar + x: int + y: int + **kwds: dict + z: int + """ + err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + self.expect_failure(block, err) + def test_depr_star_must_come_after_slash(self): block = """ module foo @@ -2079,6 +2158,16 @@ def test_parameters_no_more_than_one_vararg(self): """ self.expect_failure(block, err, lineno=3) + def test_parameters_no_more_than_one_var_keyword(self): + err = "Encountered parameter line when not expecting parameters: **var_keyword_2: dict" + block = """ + module foo + foo.bar + **var_keyword_1: dict + **var_keyword_2: dict + """ + self.expect_failure(block, err, lineno=3) + def test_function_not_at_column_0(self): function = self.parse_function(""" module foo @@ -2513,6 +2602,14 @@ def test_vararg_cannot_take_default_value(self): """ self.expect_failure(block, err, lineno=1) + def test_var_keyword_cannot_take_default_value(self): + err = "Function 'fn' has an invalid parameter declaration:" + block = """ + fn + **kwds: dict = None + """ + self.expect_failure(block, err, lineno=1) + def test_default_is_not_of_correct_type(self): err = ("int_converter: default value 2.5 for field 'a' " "is not of type 'int'") @@ -2610,6 +2707,43 @@ def test_disallow_defining_class_at_module_level(self): """ self.expect_failure(block, err, lineno=2) + def test_var_keyword_with_pos_or_kw(self): + block = """ + module foo + foo.bar + x: int + **kwds: dict + """ + err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + self.expect_failure(block, err) + + def test_var_keyword_with_kw_only(self): + block = """ + module foo + foo.bar + x: int + / + * + y: int + **kwds: dict + """ + err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + self.expect_failure(block, err) + + def test_var_keyword_with_pos_or_kw_and_kw_only(self): + block = """ + module foo + foo.bar + x: int + / + y: int + * + z: int + **kwds: dict + """ + err = "Function 'bar' has an invalid parameter declaration (**kwargs?): '**kwds: dict'" + self.expect_failure(block, err) + class ClinicExternalTest(TestCase): maxDiff = None diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index 644eb3d85e19eb..8e6d6820e2c8c3 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -850,10 +850,6 @@ def state_parameter(self, line: str) -> None: # we indented, must be to new parameter docstring column return self.next(self.state_parameter_docstring_start, line) - if not self.expecting_parameters: - fail('Encountered parameter line when not expecting ' - f'parameters: {line}') - line = line.rstrip() if line.endswith('\\'): self.parameter_continuation = line[:-1] @@ -882,6 +878,10 @@ def state_parameter(self, line: str) -> None: def parse_parameter(self, line: str) -> None: assert self.function is not None + if not self.expecting_parameters: + fail('Encountered parameter line when not expecting ' + f'parameters: {line}') + match self.parameter_state: case ParamState.START | ParamState.REQUIRED: self.to_required() @@ -1179,6 +1179,9 @@ def parse_star(self, function: Function, version: VersionTuple | None) -> None: The 'version' parameter signifies the future version from which the marker will take effect (None means it is already in effect). """ + if not self.expecting_parameters: + fail("Encountered '*' when not expecting parameters") + if version is None: self.check_previous_star() self.check_remaining_star() @@ -1234,6 +1237,9 @@ def parse_slash(self, function: Function, version: VersionTuple | None) -> None: The 'version' parameter signifies the future version from which the marker will take effect (None means it is already in effect). """ + if not self.expecting_parameters: + fail("Encountered '/' when not expecting parameters") + if version is None: if self.deprecated_keyword: fail(f"Function {function.name!r}: '/' must precede '/ [from ...]'") From 51add1af18322580df47f78ebcab68454e3d7646 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 13:32:48 +0100 Subject: [PATCH 06/10] Review (part 3): add more tests --- Lib/test/test_clinic.py | 43 +++++++++++++++++++++++++++++++++++++++++ Modules/_testclinic.c | 8 ++++---- 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index fcf53e1360e0d1..d54dd546ea36fb 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -4088,6 +4088,49 @@ def test_depr_multi(self): check("a", b="b", c="c", d="d", e="e", f="f", g="g") self.assertRaises(TypeError, fn, a="a", b="b", c="c", d="d", e="e", f="f", g="g") + def test_lone_kwds(self): + with self.assertRaises(TypeError): + ac_tester.lone_kwds(1, 2) + self.assertEqual(ac_tester.lone_kwds(), ({},)) + self.assertEqual(ac_tester.lone_kwds(y='y'), ({'y': 'y'},)) + kwds = {'y': 'y', 'z': 'z'} + self.assertEqual(ac_tester.lone_kwds(y='y', z='z'), (kwds,)) + self.assertEqual(ac_tester.lone_kwds(**kwds), (kwds,)) + + def test_kwds_with_pos_only(self): + with self.assertRaises(TypeError): + ac_tester.kwds_with_pos_only() + with self.assertRaises(TypeError): + ac_tester.kwds_with_pos_only(y='y') + with self.assertRaises(TypeError): + ac_tester.kwds_with_pos_only(1, y='y') + self.assertEqual(ac_tester.kwds_with_pos_only(1, 2), (1, 2, {})) + self.assertEqual(ac_tester.kwds_with_pos_only(1, 2, y='y'), (1, 2, {'y': 'y'})) + kwds = {'y': 'y', 'z': 'z'} + self.assertEqual(ac_tester.kwds_with_pos_only(1, 2, y='y', z='z'), (1, 2, kwds)) + self.assertEqual(ac_tester.kwds_with_pos_only(1, 2, **kwds), (1, 2, kwds)) + + def test_kwds_with_stararg(self): + self.assertEqual(ac_tester.kwds_with_stararg(), ((), {})) + self.assertEqual(ac_tester.kwds_with_stararg(1, 2), ((1, 2), {})) + self.assertEqual(ac_tester.kwds_with_stararg(y='y'), ((), {'y': 'y'})) + args = (1, 2) + kwds = {'y': 'y', 'z': 'z'} + self.assertEqual(ac_tester.kwds_with_stararg(1, 2, y='y', z='z'), (args, kwds)) + self.assertEqual(ac_tester.kwds_with_stararg(*args, **kwds), (args, kwds)) + + def test_kwds_with_pos_only_and_stararg(self): + with self.assertRaises(TypeError): + ac_tester.kwds_with_pos_only_and_stararg() + with self.assertRaises(TypeError): + ac_tester.kwds_with_pos_only_and_stararg(y='y') + self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2), (1, 2, (), {})) + self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2, y='y'), (1, 2, (), {'y': 'y'})) + args = ('lobster', 'thermidor') + kwds = {'y': 'y', 'z': 'z'} + self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2, 'lobster', 'thermidor', y='y', z='z'), (1, 2, args, kwds)) + self.assertEqual(ac_tester.kwds_with_pos_only_and_stararg(1, 2, *args, **kwds), (1, 2, args, kwds)) + class LimitedCAPIOutputTests(unittest.TestCase): diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index d45d28a1c83537..5c196c0dd0fb01 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -2336,7 +2336,7 @@ static PyObject * lone_kwds_impl(PyObject *module, PyObject *kwds) /*[clinic end generated code: output=572549c687a0432e input=6ef338b913ecae17]*/ { - Py_RETURN_NONE; + return pack_arguments_newref(1, kwds); } @@ -2353,7 +2353,7 @@ kwds_with_pos_only_impl(PyObject *module, PyObject *a, PyObject *b, PyObject *kwds) /*[clinic end generated code: output=573096d3a7efcce5 input=da081a5d9ae8878a]*/ { - Py_RETURN_NONE; + return pack_arguments_newref(3, a, b, kwds); } @@ -2367,7 +2367,7 @@ static PyObject * kwds_with_stararg_impl(PyObject *module, PyObject *args, PyObject *kwds) /*[clinic end generated code: output=d4b0064626a25208 input=1be404572d685859]*/ { - Py_RETURN_NONE; + return pack_arguments_newref(2, args, kwds); } @@ -2386,7 +2386,7 @@ kwds_with_pos_only_and_stararg_impl(PyObject *module, PyObject *a, PyObject *kwds) /*[clinic end generated code: output=af7df7640c792246 input=2fe330c7981f0829]*/ { - Py_RETURN_NONE; + return pack_arguments_newref(4, a, b, args, kwds); } From 1e1a4b284fa59acff18a46c222eeeeaea5d30dbc Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 13:57:46 +0100 Subject: [PATCH 07/10] Reduce diff --- Tools/clinic/libclinic/function.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Tools/clinic/libclinic/function.py b/Tools/clinic/libclinic/function.py index 2901ac519e2e55..f981f0bcaf89f0 100644 --- a/Tools/clinic/libclinic/function.py +++ b/Tools/clinic/libclinic/function.py @@ -214,15 +214,15 @@ class Parameter: def __repr__(self) -> str: return f'' + def is_keyword_only(self) -> bool: + return self.kind == inspect.Parameter.KEYWORD_ONLY + def is_positional_only(self) -> bool: return self.kind == inspect.Parameter.POSITIONAL_ONLY def is_positional_or_keyword(self) -> bool: return self.kind == inspect.Parameter.POSITIONAL_OR_KEYWORD - def is_keyword_only(self) -> bool: - return self.kind == inspect.Parameter.KEYWORD_ONLY - def is_vararg(self) -> bool: return self.kind == inspect.Parameter.VAR_POSITIONAL From 26a6b87892a088ce218926c7098a99c7db3c4e41 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 13:58:07 +0100 Subject: [PATCH 08/10] Simplify fastcall control & add comment --- Tools/clinic/libclinic/parse_args.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py index fb9dea08deee3c..bca87ecd75100c 100644 --- a/Tools/clinic/libclinic/parse_args.py +++ b/Tools/clinic/libclinic/parse_args.py @@ -267,11 +267,13 @@ def __init__(self, func: Function, codegen: CodeGen) -> None: if self.func.critical_section: self.codegen.add_include('pycore_critical_section.h', 'Py_BEGIN_CRITICAL_SECTION()') + + # Use fastcall if not disabled, except if in a __new__ or + # __init__ method, or if there is a **kwargs parameter. if self.func.disable_fastcall: self.fastcall = False elif self.var_keyword is not None: - has_args = self.parameters or self.varpos - self.fastcall = not has_args + self.fastcall = False else: self.fastcall = not self.is_new_or_init() From c46ed0d308548495c90ed394df2082b3280b6895 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 13:58:45 +0100 Subject: [PATCH 09/10] Simplify non-positional parameter check --- Tools/clinic/libclinic/dsl_parser.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py index 8e6d6820e2c8c3..0d83baeba9e508 100644 --- a/Tools/clinic/libclinic/dsl_parser.py +++ b/Tools/clinic/libclinic/dsl_parser.py @@ -927,8 +927,11 @@ def parse_parameter(self, line: str) -> None: # (var-positional), then we allow ``**kwds`` (var-keyword). # Currently, pos-or-keyword or keyword-only arguments are not # allowed with the ``**kwds`` converter. - if not all(p.is_positional_only() or p.is_vararg() - for p in self.function.parameters.values()): + has_non_positional_param = any( + p.is_positional_or_keyword() or p.is_keyword_only() + for p in self.function.parameters.values() + ) + if has_non_positional_param: fail(f"Function {self.function.name!r} has an " f"invalid parameter declaration (**kwargs?): {line!r}") is_var_keyword = True From bc2f864beb5614a873ccc83be6b487f24e866e9c Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Thu, 18 Sep 2025 14:03:32 +0100 Subject: [PATCH 10/10] PEP 7 / code style --- Modules/clinic/_testclinic_kwds.c.h | 18 +++++++++++++----- Tools/clinic/libclinic/converters.py | 5 +++-- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/Modules/clinic/_testclinic_kwds.c.h b/Modules/clinic/_testclinic_kwds.c.h index 274fdcc8fd8082..e2fd4d9f3b4591 100644 --- a/Modules/clinic/_testclinic_kwds.c.h +++ b/Modules/clinic/_testclinic_kwds.c.h @@ -33,7 +33,9 @@ lone_kwds(PyObject *module, PyObject *args, PyObject *kwargs) } if (kwargs == NULL) { __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) goto exit; + if (__clinic_kwds == NULL) { + goto exit; + } } else { __clinic_kwds = Py_NewRef(kwargs); @@ -74,7 +76,9 @@ kwds_with_pos_only(PyObject *module, PyObject *args, PyObject *kwargs) b = PyTuple_GET_ITEM(args, 1); if (kwargs == NULL) { __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) goto exit; + if (__clinic_kwds == NULL) { + goto exit; + } } else { __clinic_kwds = Py_NewRef(kwargs); @@ -109,7 +113,9 @@ kwds_with_stararg(PyObject *module, PyObject *args, PyObject *kwargs) __clinic_args = Py_NewRef(args); if (kwargs == NULL) { __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) goto exit; + if (__clinic_kwds == NULL) { + goto exit; + } } else { __clinic_kwds = Py_NewRef(kwargs); @@ -158,7 +164,9 @@ kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwarg } if (kwargs == NULL) { __clinic_kwds = PyDict_New(); - if (__clinic_kwds == NULL) goto exit; + if (__clinic_kwds == NULL) { + goto exit; + } } else { __clinic_kwds = Py_NewRef(kwargs); @@ -173,4 +181,4 @@ kwds_with_pos_only_and_stararg(PyObject *module, PyObject *args, PyObject *kwarg return return_value; } -/*[clinic end generated code: output=c391b2a6f7d5ab41 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=e4dea1070e003f5d input=a9049054013a1b77]*/ diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py index d1002da72e0f78..3154299e31b4dc 100644 --- a/Tools/clinic/libclinic/converters.py +++ b/Tools/clinic/libclinic/converters.py @@ -1316,7 +1316,6 @@ def parse_var_keyword(self) -> str: class var_keyword_dict_converter(VarKeywordCConverter): type = 'PyObject *' - format_unit = '' c_default = 'NULL' def cleanup(self) -> str: @@ -1327,7 +1326,9 @@ def parse_var_keyword(self) -> str: return f""" if (kwargs == NULL) {{{{ {param_name} = PyDict_New(); - if ({param_name} == NULL) goto exit; + if ({param_name} == NULL) {{{{ + goto exit; + }}}} }}}} else {{{{ {param_name} = Py_NewRef(kwargs);