gh-122943: Rework support of var-positional parameter in Argument Clinic (GH-122945)

Move creation of a tuple for var-positional parameter out of
_PyArg_UnpackKeywordsWithVararg().
Merge _PyArg_UnpackKeywordsWithVararg() with _PyArg_UnpackKeywords().
Add a new parameter in _PyArg_UnpackKeywords().

The "parameters" and "converters" attributes of ParseArgsCodeGen no
longer contain the var-positional parameter. It is now available as the
"varpos" attribute. Optimize code generation for var-positional
parameter and reuse the same generating code for functions with and without
keyword parameters.

Add special converters for var-positional parameter. "tuple" represents it as
a Python tuple and "array" represents it as a continuous array of PyObject*.
"object" is a temporary alias of "tuple".

6 files changed, 156 insertions, 84 deletions

diff --git a/Tools/c-analyzer/c_parser/parser/__init__.py b/Tools/c-analyzer/c_parser/parser/__init__.py
index 4227e93..ff4f303 100644
--- a/Tools/c-analyzer/c_parser/parser/__init__.py
+++ b/Tools/c-analyzer/c_parser/parser/__init__.py
@@ -164,7 +164,7 @@ def _parse(srclines, anon_name, **srckwargs):
 # We use defaults that cover most files.  Files with bigger declarations
 # are covered elsewhere (MAX_SIZES in cpython/_parser.py).
 
-def _iter_source(lines, *, maxtext=10_000, maxlines=200, showtext=False):
+def _iter_source(lines, *, maxtext=11_000, maxlines=200, showtext=False):
     maxtext = maxtext if maxtext and maxtext > 0 else None
     maxlines = maxlines if maxlines and maxlines > 0 else None
     filestack = []
diff --git a/Tools/clinic/libclinic/clanguage.py b/Tools/clinic/libclinic/clanguage.py
index 32aba81..32d2c04 100644
--- a/Tools/clinic/libclinic/clanguage.py
+++ b/Tools/clinic/libclinic/clanguage.py
@@ -15,7 +15,7 @@ from libclinic.function import (
     Module, Class, Function, Parameter,
     permute_optional_groups,
     GETTER, SETTER, METHOD_INIT)
-from libclinic.converters import defining_class_converter, self_converter
+from libclinic.converters import self_converter
 from libclinic.parse_args import ParseArgsCodeGen
 if TYPE_CHECKING:
     from libclinic.app import Clinic
@@ -396,12 +396,6 @@ class CLanguage(Language):
         first_optional = len(selfless)
         positional = selfless and selfless[-1].is_positional_only()
         has_option_groups = False
-        requires_defining_class = (len(selfless)
-                                   and isinstance(selfless[0].converter,
-                                                  defining_class_converter))
-        pass_vararg_directly = (all(p.is_positional_only() or p.is_vararg()
-                                    for p in selfless)
-                                and not requires_defining_class)
 
         # offset i by -1 because first_optional needs to ignore self
         for i, p in enumerate(parameters, -1):
@@ -410,9 +404,6 @@ class CLanguage(Language):
             if (i != -1) and (p.default is not unspecified):
                 first_optional = min(first_optional, i)
 
-            if p.is_vararg() and not pass_vararg_directly:
-                data.cleanup.append(f"Py_XDECREF({c.parser_name});")
-
             # insert group variable
             group = p.group
             if last_group != group:
@@ -424,11 +415,6 @@ class CLanguage(Language):
                     data.impl_parameters.append("int " + group_name)
                     has_option_groups = True
 
-            if p.is_vararg() and pass_vararg_directly:
-                data.impl_arguments.append('nvararg')
-                data.impl_parameters.append('Py_ssize_t nargs')
-                p.converter.type = 'PyObject *const *'
-
             c.render(p, data)
 
         if has_option_groups and (not positional):
diff --git a/Tools/clinic/libclinic/converter.py b/Tools/clinic/libclinic/converter.py
index 86853bb..2c93dda 100644
--- a/Tools/clinic/libclinic/converter.py
+++ b/Tools/clinic/libclinic/converter.py
@@ -312,7 +312,7 @@ class CConverter(metaclass=CConverterAutoRegister):
     def length_name(self) -> str:
         """Computes the name of the associated "length" variable."""
         assert self.length is not None
-        return self.parser_name + "_length"
+        return self.name + "_length"
 
     # Why is this one broken out separately?
     # For "positional-only" function parsing,
diff --git a/Tools/clinic/libclinic/converters.py b/Tools/clinic/libclinic/converters.py
index bd5c2a2..2d103c9 100644
--- a/Tools/clinic/libclinic/converters.py
+++ b/Tools/clinic/libclinic/converters.py
@@ -1228,3 +1228,30 @@ class self_converter(CConverter):
             type_object = cls.type_object
             type_ptr = f'PyTypeObject *base_tp = {type_object};'
             template_dict['base_type_ptr'] = type_ptr
+
+
+# Converters for var-positional parameter.
+
+class varpos_tuple_converter(CConverter):
+    type = 'PyObject *'
+    format_unit = ''
+    c_default = 'NULL'
+
+    def cleanup(self) -> str:
+        return f"""Py_XDECREF({self.parser_name});\n"""
+
+    def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> str | None:
+        raise AssertionError('should never be called')
+
+class varpos_array_converter(CConverter):
+    type = 'PyObject * const *'
+    format_unit = ''
+    length = True
+    c_ignored_default = ''
+
+    def parse_arg(self, argname: str, displayname: str, *, limited_capi: bool) -> str | None:
+        raise AssertionError('should never be called')
+
+# XXX: temporary
+class varpos_object_converter(varpos_tuple_converter):
+    pass
diff --git a/Tools/clinic/libclinic/dsl_parser.py b/Tools/clinic/libclinic/dsl_parser.py
index 5ca3bd5..4b4a8b9 100644
--- a/Tools/clinic/libclinic/dsl_parser.py
+++ b/Tools/clinic/libclinic/dsl_parser.py
@@ -925,16 +925,17 @@ class DSLParser:
 
         parameter_name = parameter.arg
         name, legacy, kwargs = self.parse_converter(parameter.annotation)
+        if is_vararg:
+            name = 'varpos_' + name
 
         value: object
         if not default:
-            if self.parameter_state is ParamState.OPTIONAL:
-                fail(f"Can't have a parameter without a default ({parameter_name!r}) "
-                      "after a parameter with a default!")
             if is_vararg:
                 value = NULL
-                kwargs.setdefault('c_default', "NULL")
             else:
+                if self.parameter_state is ParamState.OPTIONAL:
+                    fail(f"Can't have a parameter without a default ({parameter_name!r}) "
+                          "after a parameter with a default!")
                 value = unspecified
             if 'py_default' in kwargs:
                 fail("You can't specify py_default without specifying a default value!")
diff --git a/Tools/clinic/libclinic/parse_args.py b/Tools/clinic/libclinic/parse_args.py
index 559d4fb..2ce4e75 100644
--- a/Tools/clinic/libclinic/parse_args.py
+++ b/Tools/clinic/libclinic/parse_args.py
@@ -217,8 +217,7 @@ class ParseArgsCodeGen:
     min_pos: int = 0
     max_pos: int = 0
     min_kw_only: int = 0
-    pseudo_args: int = 0
-    vararg: int | str = NO_VARARG
+    varpos: Parameter | None = None
 
     docstring_prototype: str
     docstring_definition: str
@@ -246,6 +245,13 @@ class ParseArgsCodeGen:
         if self.parameters and isinstance(self.parameters[0].converter, defining_class_converter):
             self.requires_defining_class = True
             del self.parameters[0]
+
+        for i, p in enumerate(self.parameters):
+            if p.is_vararg():
+                self.varpos = p
+                del self.parameters[i]
+                break
+
         self.converters = [p.converter for p in self.parameters]
 
         if self.func.critical_section:
@@ -257,18 +263,13 @@ class ParseArgsCodeGen:
         self.min_pos = 0
         self.max_pos = 0
         self.min_kw_only = 0
-        self.pseudo_args = 0
         for i, p in enumerate(self.parameters, 1):
             if p.is_keyword_only():
                 assert not p.is_positional_only()
                 if not p.is_optional():
-                    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
+                    self.min_kw_only = i - self.max_pos
             else:
-                if self.vararg == NO_VARARG:
-                    self.max_pos = i
+                self.max_pos = i
                 if p.is_positional_only():
                     self.pos_only = i
                 if not p.is_optional():
@@ -285,6 +286,7 @@ class ParseArgsCodeGen:
         return (len(self.parameters) == 1
                 and self.parameters[0].is_positional_only()
                 and not self.converters[0].is_optional()
+                and not self.varpos
                 and not self.requires_defining_class
                 and not self.is_new_or_init())
 
@@ -315,7 +317,8 @@ class ParseArgsCodeGen:
 
     def init_limited_capi(self) -> None:
         self.limited_capi = self.codegen.limited_capi
-        if self.limited_capi and (self.pseudo_args or
+        if self.limited_capi and (
+                (self.varpos and self.pos_only < len(self.parameters)) or
                 (any(p.is_optional() for p in self.parameters) and
                  any(p.is_keyword_only() and not p.is_optional() for p in self.parameters)) or
                 any(c.broken_limited_capi for c in self.converters)):
@@ -447,6 +450,74 @@ class ParseArgsCodeGen:
         parser_code = '    {option_group_parsing}'
         self.parser_body(parser_code)
 
+    def _parse_vararg(self) -> str:
+        assert self.varpos is not None
+        paramname = self.varpos.converter.parser_name
+        if self.varpos.converter.length:
+            if not self.fastcall:
+                self.codegen.add_include('pycore_tuple.h',
+                                        '_PyTuple_ITEMS()')
+            start = 'args' if self.fastcall else '_PyTuple_ITEMS(args)'
+            size = 'nargs' if self.fastcall else 'PyTuple_GET_SIZE(args)'
+            if self.max_pos:
+                if min(self.pos_only, self.min_pos) < self.max_pos:
+                    start = f'{size} > {self.max_pos} ? {start} + {self.max_pos} : {start}'
+                    size = f'Py_MAX(0, {size} - {self.max_pos})'
+                else:
+                    start = f'{start} + {self.max_pos}'
+                    size = f'{size} - {self.max_pos}'
+            return f"""
+                {paramname} = {start};
+                {self.varpos.converter.length_name} = {size};
+                """
+
+        if self.fastcall:
+            if self.limited_capi:
+                if min(self.pos_only, self.min_pos) < self.max_pos:
+                    size = f'Py_MAX(nargs - {self.max_pos}, 0)'
+                else:
+                    size = f'nargs - {self.max_pos}' if self.max_pos else 'nargs'
+                return f"""
+                    {paramname} = PyTuple_New({size});
+                    if (!{paramname}) {{{{
+                        goto exit;
+                    }}}}
+                    for (Py_ssize_t i = {self.max_pos}; i < nargs; ++i) {{{{
+                        PyTuple_SET_ITEM({paramname}, i - {self.max_pos}, Py_NewRef(args[i]));
+                    }}}}
+                    """
+            else:
+                self.codegen.add_include('pycore_tuple.h',
+                                         '_PyTuple_FromArray()')
+                if min(self.pos_only, self.min_pos) < self.max_pos:
+                    return f"""
+                        {paramname} = nargs > {self.max_pos}
+                            ? _PyTuple_FromArray(args + {self.max_pos}, nargs - {self.max_pos})
+                            : PyTuple_New(0);
+                        if ({paramname} == NULL) {{{{
+                            goto exit;
+                        }}}}
+                        """
+                else:
+                    start = f'args + {self.max_pos}' if self.max_pos else 'args'
+                    size = f'nargs - {self.max_pos}' if self.max_pos else 'nargs'
+                    return f"""
+                        {paramname} = _PyTuple_FromArray({start}, {size});
+                        if ({paramname} == NULL) {{{{
+                            goto exit;
+                        }}}}
+                        """
+        else:
+            if self.max_pos:
+                return f"""
+                    {paramname} = PyTuple_GetSlice(args, {self.max_pos}, PY_SSIZE_T_MAX);
+                    if (!{paramname}) {{{{
+                        goto exit;
+                    }}}}
+                    """
+            else:
+                return f"{paramname} = Py_NewRef(args);\n"
+
     def parse_pos_only(self) -> None:
         if self.fastcall:
             # positional-only, but no option groups
@@ -469,14 +540,9 @@ class ParseArgsCodeGen:
                 nargs = 'PyTuple_GET_SIZE(args)'
                 argname_fmt = 'PyTuple_GET_ITEM(args, %d)'
 
-        if self.vararg != NO_VARARG:
-            self.declarations = f"Py_ssize_t nvararg = {nargs} - {self.max_pos};"
-        else:
-            self.declarations = ""
-
-        max_args = NO_VARARG if (self.vararg != NO_VARARG) else self.max_pos
+        parser_code = []
+        max_args = NO_VARARG if self.varpos else self.max_pos
         if self.limited_capi:
-            parser_code = []
             if nargs != 'nargs':
                 nargs_def = f'Py_ssize_t nargs = {nargs};'
                 parser_code.append(libclinic.normalize_snippet(nargs_def, indent=4))
@@ -509,33 +575,27 @@ class ParseArgsCodeGen:
                         }}}}
                         """,
                     indent=4))
-        else:
+        elif self.min_pos or max_args != NO_VARARG:
             self.codegen.add_include('pycore_modsupport.h',
                                      '_PyArg_CheckPositional()')
-            parser_code = [libclinic.normalize_snippet(f"""
+            parser_code.append(libclinic.normalize_snippet(f"""
                 if (!_PyArg_CheckPositional("{{name}}", {nargs}, {self.min_pos}, {max_args})) {{{{
                     goto exit;
                 }}}}
-                """, indent=4)]
+                """, indent=4))
 
         has_optional = False
         use_parser_code = True
         for i, p in enumerate(self.parameters):
-            if p.is_vararg():
-                var = p.converter.parser_name
-                if self.fastcall:
-                    code = f"{var} = args + {self.vararg};"
-                else:
-                    code = f"{var} = _PyTuple_CAST(args)->ob_item;"
-                formatted_code = libclinic.normalize_snippet(code, indent=4)
-                parser_code.append(formatted_code)
-                continue
-
             displayname = p.get_displayname(i+1)
             argname = argname_fmt % i
             parsearg: str | None
             parsearg = p.converter.parse_arg(argname, displayname, limited_capi=self.limited_capi)
             if parsearg is None:
+                if self.varpos:
+                    raise ValueError(
+                        f"Using converter {p.converter} is not supported "
+                        f"in function with var-positional parameter")
                 use_parser_code = False
                 parser_code = []
                 break
@@ -551,6 +611,8 @@ class ParseArgsCodeGen:
         if use_parser_code:
             if has_optional:
                 parser_code.append("skip_optional:")
+            if self.varpos:
+                parser_code.append(libclinic.normalize_snippet(self._parse_vararg(), indent=4))
         else:
             for parameter in self.parameters:
                 parameter.converter.use_converter()
@@ -575,7 +637,7 @@ class ParseArgsCodeGen:
                         goto exit;
                     }}
                     """, indent=4)]
-        self.parser_body(*parser_code, declarations=self.declarations)
+        self.parser_body(*parser_code)
 
     def parse_general(self, clang: CLanguage) -> None:
         parsearg: str | None
@@ -589,7 +651,7 @@ class ParseArgsCodeGen:
 
         has_optional_kw = (
             max(self.pos_only, self.min_pos) + self.min_kw_only
-            < len(self.converters) - int(self.vararg != NO_VARARG)
+            < len(self.converters)
         )
 
         use_parser_code = True
@@ -598,57 +660,53 @@ class ParseArgsCodeGen:
             use_parser_code = False
             self.fastcall = False
         else:
-            if self.vararg == NO_VARARG:
+            if not self.varpos:
                 self.codegen.add_include('pycore_modsupport.h',
                                          '_PyArg_UnpackKeywords()')
-                args_declaration = "_PyArg_UnpackKeywords", "%s, %s, %s" % (
-                    self.min_pos,
-                    self.max_pos,
-                    self.min_kw_only
-                )
+                unpack_func = '_PyArg_UnpackKeywords'
                 nargs = "nargs"
             else:
                 self.codegen.add_include('pycore_modsupport.h',
                                          '_PyArg_UnpackKeywordsWithVararg()')
-                args_declaration = "_PyArg_UnpackKeywordsWithVararg", "%s, %s, %s, %s" % (
-                    self.min_pos,
-                    self.max_pos,
-                    self.min_kw_only,
-                    self.vararg
-                )
+                unpack_func = '_PyArg_UnpackKeywordsWithVararg'
                 nargs = f"Py_MIN(nargs, {self.max_pos})" if self.max_pos else "0"
 
             if self.fastcall:
                 self.flags = "METH_FASTCALL|METH_KEYWORDS"
                 self.parser_prototype = PARSER_PROTOTYPE_FASTCALL_KEYWORDS
-                argname_fmt = 'args[%d]'
                 self.declarations = declare_parser(self.func, codegen=self.codegen)
-                self.declarations += "\nPyObject *argsbuf[%s];" % len(self.converters)
+                self.declarations += "\nPyObject *argsbuf[%s];" % (len(self.converters) or 1)
+                if self.varpos:
+                    self.declarations += "\nPyObject * const *fastargs;"
+                    argsname = 'fastargs'
+                    argname_fmt = 'fastargs[%d]'
+                else:
+                    argsname = 'args'
+                    argname_fmt = 'args[%d]'
                 if has_optional_kw:
                     self.declarations += "\nPy_ssize_t noptargs = %s + (kwnames ? PyTuple_GET_SIZE(kwnames) : 0) - %d;" % (nargs, self.min_pos + self.min_kw_only)
-                parser_code = [libclinic.normalize_snippet("""
-                    args = %s(args, nargs, NULL, kwnames, &_parser, %s, argsbuf);
-                    if (!args) {{
-                        goto exit;
-                    }}
-                    """ % args_declaration, indent=4)]
+                unpack_args = 'args, nargs, NULL, kwnames'
             else:
                 # positional-or-keyword arguments
                 self.flags = "METH_VARARGS|METH_KEYWORDS"
                 self.parser_prototype = PARSER_PROTOTYPE_KEYWORD
+                argsname = 'fastargs'
                 argname_fmt = 'fastargs[%d]'
                 self.declarations = declare_parser(self.func, codegen=self.codegen)
-                self.declarations += "\nPyObject *argsbuf[%s];" % len(self.converters)
+                self.declarations += "\nPyObject *argsbuf[%s];" % (len(self.converters) or 1)
                 self.declarations += "\nPyObject * const *fastargs;"
                 self.declarations += "\nPy_ssize_t nargs = PyTuple_GET_SIZE(args);"
                 if has_optional_kw:
                     self.declarations += "\nPy_ssize_t noptargs = %s + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - %d;" % (nargs, self.min_pos + self.min_kw_only)
-                parser_code = [libclinic.normalize_snippet("""
-                    fastargs = %s(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, %s, argsbuf);
-                    if (!fastargs) {{
-                        goto exit;
-                    }}
-                    """ % args_declaration, indent=4)]
+                unpack_args = '_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL'
+            unpack_args += (f', &_parser, {self.min_pos}, {self.max_pos}, '
+                            f'{self.min_kw_only}, argsbuf')
+            parser_code = [libclinic.normalize_snippet(f"""
+                {argsname} = {unpack_func}({unpack_args});
+                if (!{argsname}) {{{{
+                    goto exit;
+                }}}}
+                """, indent=4)]
 
         if self.requires_defining_class:
             self.flags = 'METH_METHOD|' + self.flags
@@ -697,8 +755,6 @@ class ParseArgsCodeGen:
                     else:
                         label = 'skip_optional_kwonly'
                         first_opt = self.max_pos + self.min_kw_only
-                        if self.vararg != NO_VARARG:
-                            first_opt += 1
                     if i == first_opt:
                         add_label = label
                         parser_code.append(libclinic.normalize_snippet("""
@@ -724,6 +780,8 @@ class ParseArgsCodeGen:
         if use_parser_code:
             if add_label:
                 parser_code.append("%s:" % add_label)
+            if self.varpos:
+                parser_code.append(libclinic.normalize_snippet(self._parse_vararg(), indent=4))
         else:
             for parameter in self.parameters:
                 parameter.converter.use_converter()
@@ -914,14 +972,14 @@ class ParseArgsCodeGen:
         # previous call to parser_body. this is used for an awful hack.
         self.parser_body_fields: tuple[str, ...] = ()
 
-        if not self.parameters:
+        if not self.parameters and not self.varpos:
             self.parse_no_args()
         elif self.use_meth_o():
             self.parse_one_arg()
         elif self.has_option_groups():
             self.parse_option_groups()
         elif (not self.requires_defining_class
-              and self.pos_only == len(self.parameters) - self.pseudo_args):
+              and self.pos_only == len(self.parameters)):
             self.parse_pos_only()
         else:
             self.parse_general(clang)