Issue #26282: PyArg_ParseTupleAndKeywords() and Argument Clinic now support

positional-only and keyword parameters in the same function.

9 files changed, 208 insertions, 70 deletions

diff --git a/Doc/c-api/arg.rst b/Doc/c-api/arg.rst
index d9f0f43..d15f649 100644
--- a/Doc/c-api/arg.rst
+++ b/Doc/c-api/arg.rst
@@ -406,8 +406,15 @@ API Functions
 .. c:function:: int PyArg_ParseTupleAndKeywords(PyObject *args, PyObject *kw, const char *format, char *keywords[], ...)
 
    Parse the parameters of a function that takes both positional and keyword
-   parameters into local variables.  Returns true on success; on failure, it
-   returns false and raises the appropriate exception.
+   parameters into local variables.  The *keywords* argument is a
+   *NULL*-terminated array of keyword parameter names.  Empty names denote
+   :ref:`positional-only parameters <positional-only_parameter>`.
+   Returns true on success; on failure, it returns false and raises the
+   appropriate exception.
+
+   .. versionchanged:: 3.6
+      Added support for :ref:`positional-only parameters
+      <positional-only_parameter>`.
 
 
 .. c:function:: int PyArg_VaParseTupleAndKeywords(PyObject *args, PyObject *kw, const char *format, char *keywords[], va_list vargs)
diff --git a/Doc/glossary.rst b/Doc/glossary.rst
index e7bcb6a..b4f3089 100644
--- a/Doc/glossary.rst
+++ b/Doc/glossary.rst
@@ -718,6 +718,8 @@ Glossary
 
            def func(foo, bar=None): ...
 
+      .. _positional-only_parameter:
+
       * :dfn:`positional-only`: specifies an argument that can be supplied only
         by position.  Python has no syntax for defining positional-only
         parameters.  However, some built-in functions have positional-only
diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst
index f438f0b..5dc6076 100644
--- a/Doc/whatsnew/3.6.rst
+++ b/Doc/whatsnew/3.6.rst
@@ -503,6 +503,11 @@ Build and C API Changes
 * New :c:func:`Py_FinalizeEx` API which indicates if flushing buffered data
   failed (:issue:`5319`).
 
+* :c:func:`PyArg_ParseTupleAndKeywords` now supports :ref:`positional-only
+  parameters <positional-only_parameter>`.  Positional-only parameters are
+  defined by empty names.
+  (Contributed by Serhit Storchaka in :issue:`26282`).
+
 
 Deprecated
 ==========
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index a0746f0..6852381 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -527,6 +527,31 @@ class SkipitemTest(unittest.TestCase):
         self.assertRaises(ValueError, _testcapi.parse_tuple_and_keywords,
                           (), {}, b'', [42])
 
+    def test_positional_only(self):
+        parse = _testcapi.parse_tuple_and_keywords
+
+        parse((1, 2, 3), {}, b'OOO', ['', '', 'a'])
+        parse((1, 2), {'a': 3}, b'OOO', ['', '', 'a'])
+        with self.assertRaisesRegex(TypeError,
+                'Function takes at least 2 positional arguments \(1 given\)'):
+            parse((1,), {'a': 3}, b'OOO', ['', '', 'a'])
+        parse((1,), {}, b'O|OO', ['', '', 'a'])
+        with self.assertRaisesRegex(TypeError,
+                'Function takes at least 1 positional arguments \(0 given\)'):
+            parse((), {}, b'O|OO', ['', '', 'a'])
+        parse((1, 2), {'a': 3}, b'OO$O', ['', '', 'a'])
+        with self.assertRaisesRegex(TypeError,
+                'Function takes exactly 2 positional arguments \(1 given\)'):
+            parse((1,), {'a': 3}, b'OO$O', ['', '', 'a'])
+        parse((1,), {}, b'O|O$O', ['', '', 'a'])
+        with self.assertRaisesRegex(TypeError,
+                'Function takes at least 1 positional arguments \(0 given\)'):
+            parse((), {}, b'O|O$O', ['', '', 'a'])
+        with self.assertRaisesRegex(SystemError, 'Empty parameter name after \$'):
+            parse((1,), {}, b'O|$OO', ['', '', 'a'])
+        with self.assertRaisesRegex(SystemError, 'Empty keyword'):
+            parse((1,), {}, b'O|OO', ['', 'a', ''])
+
 
 @unittest.skipUnless(threading, 'Threading required for this test.')
 class TestThreadState(unittest.TestCase):
diff --git a/Lib/test/test_getargs2.py b/Lib/test/test_getargs2.py
index ecc1908..16e163a 100644
--- a/Lib/test/test_getargs2.py
+++ b/Lib/test/test_getargs2.py
@@ -658,6 +658,39 @@ class KeywordOnly_TestCase(unittest.TestCase):
             getargs_keyword_only(1, 2, **{'\uDC80': 10})
 
 
+class PositionalOnlyAndKeywords_TestCase(unittest.TestCase):
+    from _testcapi import getargs_positional_only_and_keywords as getargs
+
+    def test_positional_args(self):
+        # using all possible positional args
+        self.assertEqual(self.getargs(1, 2, 3), (1, 2, 3))
+
+    def test_mixed_args(self):
+        # positional and keyword args
+        self.assertEqual(self.getargs(1, 2, keyword=3), (1, 2, 3))
+
+    def test_optional_args(self):
+        # missing optional args
+        self.assertEqual(self.getargs(1, 2), (1, 2, -1))
+        self.assertEqual(self.getargs(1, keyword=3), (1, -1, 3))
+
+    def test_required_args(self):
+        self.assertEqual(self.getargs(1), (1, -1, -1))
+        # required positional arg missing
+        with self.assertRaisesRegex(TypeError,
+            "Function takes at least 1 positional arguments \(0 given\)"):
+            self.getargs()
+
+        with self.assertRaisesRegex(TypeError,
+            "Function takes at least 1 positional arguments \(0 given\)"):
+            self.getargs(keyword=3)
+
+    def test_empty_keyword(self):
+        with self.assertRaisesRegex(TypeError,
+            "'' is an invalid keyword argument for this function"):
+            self.getargs(1, 2, **{'': 666})
+
+
 class Bytes_TestCase(unittest.TestCase):
     def test_c(self):
         from _testcapi import getargs_c
diff --git a/Misc/NEWS b/Misc/NEWS
index 3a9aa78..f382b3f 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -201,6 +201,18 @@ Misc
 - Issue #17500, and https://github.com/python/pythondotorg/issues/945: Remove 
   unused and outdated icons.
 
+C API
+-----
+
+- Issue #26282: PyArg_ParseTupleAndKeywords() now supports positional-only
+  parameters.
+
+Tools/Demos
+-----------
+
+- Issue #26282: Argument Clinic now supports positional-only and keyword
+  parameters in the same function.
+
 
 What's New in Python 3.6.0 alpha 1?
 ===================================
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 3893523..5d661f7 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -1028,6 +1028,21 @@ getargs_keyword_only(PyObject *self, PyObject *args, PyObject *kwargs)
     return Py_BuildValue("iii", required, optional, keyword_only);
 }
 
+/* test PyArg_ParseTupleAndKeywords positional-only arguments */
+static PyObject *
+getargs_positional_only_and_keywords(PyObject *self, PyObject *args, PyObject *kwargs)
+{
+    static char *keywords[] = {"", "", "keyword", NULL};
+    int required = -1;
+    int optional = -1;
+    int keyword = -1;
+
+    if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|ii", keywords,
+                                     &required, &optional, &keyword))
+        return NULL;
+    return Py_BuildValue("iii", required, optional, keyword);
+}
+
 /* Functions to call PyArg_ParseTuple with integer format codes,
    and return the result.
 */
@@ -3963,6 +3978,9 @@ static PyMethodDef TestMethods[] = {
       METH_VARARGS|METH_KEYWORDS},
     {"getargs_keyword_only", (PyCFunction)getargs_keyword_only,
       METH_VARARGS|METH_KEYWORDS},
+    {"getargs_positional_only_and_keywords",
+      (PyCFunction)getargs_positional_only_and_keywords,
+      METH_VARARGS|METH_KEYWORDS},
     {"getargs_b",               getargs_b,                       METH_VARARGS},
     {"getargs_B",               getargs_B,                       METH_VARARGS},
     {"getargs_h",               getargs_h,                       METH_VARARGS},
diff --git a/Python/getargs.c b/Python/getargs.c
index 9858bd5..4418ebb 100644
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -1443,7 +1443,8 @@ vgetargskeywords(PyObject *args, PyObject *keywords, const char *format,
     const char *fname, *msg, *custom_msg, *keyword;
     int min = INT_MAX;
     int max = INT_MAX;
-    int i, len;
+    int i, pos, len;
+    int skip = 0;
     Py_ssize_t nargs, nkeywords;
     PyObject *current_arg;
     freelistentry_t static_entries[STATIC_FREELIST_ENTRIES];
@@ -1471,9 +1472,17 @@ vgetargskeywords(PyObject *args, PyObject *keywords, const char *format,
             custom_msg++;
     }
 
+    /* scan kwlist and count the number of positional-only parameters */
+    for (pos = 0; kwlist[pos] && !*kwlist[pos]; pos++) {
+    }
     /* scan kwlist and get greatest possible nbr of args */
-    for (len=0; kwlist[len]; len++)
-        continue;
+    for (len = pos; kwlist[len]; len++) {
+        if (!*kwlist[len]) {
+            PyErr_SetString(PyExc_SystemError,
+                            "Empty keyword parameter name");
+            return cleanreturn(0, &freelist);
+        }
+    }
 
     if (len > STATIC_FREELIST_ENTRIES) {
         freelist.entries = PyMem_NEW(freelistentry_t, len);
@@ -1526,6 +1535,14 @@ vgetargskeywords(PyObject *args, PyObject *keywords, const char *format,
             max = i;
             format++;
 
+            if (max < pos) {
+                PyErr_SetString(PyExc_SystemError,
+                                "Empty parameter name after $");
+                return cleanreturn(0, &freelist);
+            }
+            if (skip) {
+                break;
+            }
             if (max < nargs) {
                 PyErr_Format(PyExc_TypeError,
                              "Function takes %s %d positional arguments"
@@ -1541,48 +1558,59 @@ vgetargskeywords(PyObject *args, PyObject *keywords, const char *format,
                          "format specifiers (%d)", len, i);
             return cleanreturn(0, &freelist);
         }
-        current_arg = NULL;
-        if (nkeywords) {
-            current_arg = PyDict_GetItemString(keywords, keyword);
-        }
-        if (current_arg) {
-            --nkeywords;
-            if (i < nargs) {
-                /* arg present in tuple and in dict */
-                PyErr_Format(PyExc_TypeError,
-                             "Argument given by name ('%s') "
-                             "and position (%d)",
-                             keyword, i+1);
-                return cleanreturn(0, &freelist);
+        if (!skip) {
+            current_arg = NULL;
+            if (nkeywords && i >= pos) {
+                current_arg = PyDict_GetItemString(keywords, keyword);
+                if (!current_arg && PyErr_Occurred()) {
+                    return cleanreturn(0, &freelist);
+                }
             }
-        }
-        else if (nkeywords && PyErr_Occurred())
-            return cleanreturn(0, &freelist);
-        else if (i < nargs)
-            current_arg = PyTuple_GET_ITEM(args, i);
-
-        if (current_arg) {
-            msg = convertitem(current_arg, &format, p_va, flags,
-                levels, msgbuf, sizeof(msgbuf), &freelist);
-            if (msg) {
-                seterror(i+1, msg, levels, fname, custom_msg);
-                return cleanreturn(0, &freelist);
+            if (current_arg) {
+                --nkeywords;
+                if (i < nargs) {
+                    /* arg present in tuple and in dict */
+                    PyErr_Format(PyExc_TypeError,
+                                 "Argument given by name ('%s') "
+                                 "and position (%d)",
+                                 keyword, i+1);
+                    return cleanreturn(0, &freelist);
+                }
+            }
+            else if (i < nargs)
+                current_arg = PyTuple_GET_ITEM(args, i);
+
+            if (current_arg) {
+                msg = convertitem(current_arg, &format, p_va, flags,
+                    levels, msgbuf, sizeof(msgbuf), &freelist);
+                if (msg) {
+                    seterror(i+1, msg, levels, fname, custom_msg);
+                    return cleanreturn(0, &freelist);
+                }
+                continue;
             }
-            continue;
-        }
 
-        if (i < min) {
-            PyErr_Format(PyExc_TypeError, "Required argument "
-                         "'%s' (pos %d) not found",
-                         keyword, i+1);
-            return cleanreturn(0, &freelist);
+            if (i < min) {
+                if (i < pos) {
+                    assert (min == INT_MAX);
+                    assert (max == INT_MAX);
+                    skip = 1;
+                }
+                else {
+                    PyErr_Format(PyExc_TypeError, "Required argument "
+                                "'%s' (pos %d) not found",
+                                keyword, i+1);
+                    return cleanreturn(0, &freelist);
+                }
+            }
+            /* current code reports success when all required args
+             * fulfilled and no keyword args left, with no further
+             * validation. XXX Maybe skip this in debug build ?
+             */
+            if (!nkeywords && !skip) {
+                return cleanreturn(1, &freelist);
+            }
         }
-        /* current code reports success when all required args
-         * fulfilled and no keyword args left, with no further
-         * validation. XXX Maybe skip this in debug build ?
-         */
-        if (!nkeywords)
-            return cleanreturn(1, &freelist);
 
         /* We are into optional args, skip thru to any remaining
          * keyword args */
@@ -1594,6 +1622,15 @@ vgetargskeywords(PyObject *args, PyObject *keywords, const char *format,
         }
     }
 
+    if (skip) {
+        PyErr_Format(PyExc_TypeError,
+                     "Function takes %s %d positional arguments"
+                     " (%d given)",
+                     (Py_MIN(pos, min) < i) ? "at least" : "exactly",
+                     Py_MIN(pos, min), nargs);
+        return cleanreturn(0, &freelist);
+    }
+
     if (!IS_END_OF_FORMAT(*format) && (*format != '|') && (*format != '$')) {
         PyErr_Format(PyExc_SystemError,
             "more argument specifiers than keyword list entries "
@@ -1613,7 +1650,7 @@ vgetargskeywords(PyObject *args, PyObject *keywords, const char *format,
                 return cleanreturn(0, &freelist);
             }
             for (i = 0; i < len; i++) {
-                if (!PyUnicode_CompareWithASCIIString(key, kwlist[i])) {
+                if (*kwlist[i] && !PyUnicode_CompareWithASCIIString(key, kwlist[i])) {
                     match = 1;
                     break;
                 }
diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py
index ff3aa88..c964093 100755
--- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -644,7 +644,7 @@ class CLanguage(Language):
         default_return_converter = (not f.return_converter or
             f.return_converter.type == 'PyObject *')
 
-        positional = parameters and (parameters[-1].kind == inspect.Parameter.POSITIONAL_ONLY)
+        positional = parameters and parameters[-1].is_positional_only()
         all_boring_objects = False # yes, this will be false if there are 0 parameters, it's fine
         first_optional = len(parameters)
         for i, p in enumerate(parameters):
@@ -661,7 +661,7 @@ class CLanguage(Language):
         new_or_init = f.kind in (METHOD_NEW, METHOD_INIT)
 
         meth_o = (len(parameters) == 1 and
-              parameters[0].kind == inspect.Parameter.POSITIONAL_ONLY and
+              parameters[0].is_positional_only() and
               not converters[0].is_optional() and
               not new_or_init)
 
@@ -1075,7 +1075,7 @@ class CLanguage(Language):
 
         last_group = 0
         first_optional = len(selfless)
-        positional = selfless and selfless[-1].kind == inspect.Parameter.POSITIONAL_ONLY
+        positional = selfless and selfless[-1].is_positional_only()
         new_or_init = f.kind in (METHOD_NEW, METHOD_INIT)
         default_return_converter = (not f.return_converter or
             f.return_converter.type == 'PyObject *')
@@ -2367,7 +2367,10 @@ class CConverter(metaclass=CConverterAutoRegister):
             data.modifications.append('/* modifications for ' + name + ' */\n' + modifications.rstrip())
 
         # keywords
-        data.keywords.append(parameter.name)
+        if parameter.is_positional_only():
+            data.keywords.append('')
+        else:
+            data.keywords.append(parameter.name)
 
         # format_units
         if self.is_optional() and '|' not in data.format_units:
@@ -3192,6 +3195,7 @@ class DSLParser:
         self.state = self.state_dsl_start
         self.parameter_indent = None
         self.keyword_only = False
+        self.positional_only = False
         self.group = 0
         self.parameter_state = self.ps_start
         self.seen_positional_with_default = False
@@ -3570,8 +3574,8 @@ class DSLParser:
     # "parameter_state".  (Previously the code was a miasma of ifs and
     # separate boolean state variables.)  The states are:
     #
-    #  [ [ a, b, ] c, ] d, e, f=3, [ g, h, [ i ] ] /   <- line
-    # 01   2          3       4    5           6   7   <- state transitions
+    #  [ [ a, b, ] c, ] d, e, f=3, [ g, h, [ i ] ]   <- line
+    # 01   2          3       4    5           6     <- state transitions
     #
     # 0: ps_start.  before we've seen anything.  legal transitions are to 1 or 3.
     # 1: ps_left_square_before.  left square brackets before required parameters.
@@ -3582,9 +3586,8 @@ class DSLParser:
     #    now must have default values.
     # 5: ps_group_after.  in a group, after required parameters.
     # 6: ps_right_square_after.  right square brackets after required parameters.
-    # 7: ps_seen_slash.  seen slash.
     ps_start, ps_left_square_before, ps_group_before, ps_required, \
-    ps_optional, ps_group_after, ps_right_square_after, ps_seen_slash = range(8)
+    ps_optional, ps_group_after, ps_right_square_after = range(7)
 
     def state_parameters_start(self, line):
         if self.ignore_line(line):
@@ -3863,9 +3866,6 @@ class DSLParser:
         return name, False, kwargs
 
     def parse_special_symbol(self, symbol):
-        if self.parameter_state == self.ps_seen_slash:
-            fail("Function " + self.function.name + " specifies " + symbol + " after /, which is unsupported.")
-
         if symbol == '*':
             if self.keyword_only:
                 fail("Function " + self.function.name + " uses '*' more than once.")
@@ -3892,13 +3892,15 @@ class DSLParser:
             else:
                 fail("Function " + self.function.name + " has an unsupported group configuration. (Unexpected state " + str(self.parameter_state) + ".c)")
         elif symbol == '/':
+            if self.positional_only:
+                fail("Function " + self.function.name + " uses '/' more than once.")
+            self.positional_only = True
             # ps_required and ps_optional are allowed here, that allows positional-only without option groups
             # to work (and have default values!)
             if (self.parameter_state not in (self.ps_required, self.ps_optional, self.ps_right_square_after, self.ps_group_before)) or self.group:
                 fail("Function " + self.function.name + " has an unsupported group configuration. (Unexpected state " + str(self.parameter_state) + ".d)")
             if self.keyword_only:
                 fail("Function " + self.function.name + " mixes keyword-only and positional-only parameters, which is unsupported.")
-            self.parameter_state = self.ps_seen_slash
             # fixup preceding parameters
             for p in self.function.parameters.values():
                 if (p.kind != inspect.Parameter.POSITIONAL_OR_KEYWORD and not isinstance(p.converter, self_converter)):
@@ -3986,23 +3988,20 @@ class DSLParser:
         # populate "right_bracket_count" field for every parameter
         assert parameters, "We should always have a self parameter. " + repr(f)
         assert isinstance(parameters[0].converter, self_converter)
+        # self is always positional-only.
+        assert parameters[0].is_positional_only()
         parameters[0].right_bracket_count = 0
-        parameters_after_self = parameters[1:]
-        if parameters_after_self:
-            # for now, the only way Clinic supports positional-only parameters
-            # is if all of them are positional-only...
-            #
-            # ... except for self!  self is always positional-only.
-
-            positional_only_parameters = [p.kind == inspect.Parameter.POSITIONAL_ONLY for p in parameters_after_self]
-            if parameters_after_self[0].kind == inspect.Parameter.POSITIONAL_ONLY:
-                assert all(positional_only_parameters)
-                for p in parameters:
-                    p.right_bracket_count = abs(p.group)
+        positional_only = True
+        for p in parameters[1:]:
+            if not p.is_positional_only():
+                positional_only = False
+            else:
+                assert positional_only
+            if positional_only:
+                p.right_bracket_count = abs(p.group)
             else:
                 # don't put any right brackets around non-positional-only parameters, ever.
-                for p in parameters_after_self:
-                    p.right_bracket_count = 0
+                p.right_bracket_count = 0
 
         right_bracket_count = 0