[Python-checkins] cpython: Issue #26282: PyArg_ParseTupleAndKeywords() and Argument Clinic now support

Fri Jun 10 14:09:51 EDT 2016

https://hg.python.org/cpython/rev/69c0aa8a8185
changeset: 101854:69c0aa8a8185
user: Serhiy Storchaka <storchaka at gmail.com>
date: Thu Jun 09 16:30:29 2016 +0300
summary:
 Issue #26282: PyArg_ParseTupleAndKeywords() and Argument Clinic now support
positional-only and keyword parameters in the same function.
files:
 Doc/c-api/arg.rst | 11 +-
 Doc/glossary.rst | 2 +
 Doc/whatsnew/3.6.rst | 5 +
 Lib/test/test_capi.py | 25 +++++
 Lib/test/test_getargs2.py | 33 ++++++
 Misc/NEWS | 12 ++
 Modules/_testcapimodule.c | 18 +++
 Python/getargs.c | 125 ++++++++++++++++---------
 Tools/clinic/clinic.py | 51 +++++-----
 9 files changed, 210 insertions(+), 72 deletions(-)

diff --git a/Doc/c-api/arg.rst b/Doc/c-api/arg.rst
--- a/Doc/c-api/arg.rst
+++ b/Doc/c-api/arg.rst
@@ -406,8 +406,15 @@
 .. 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
--- a/Doc/glossary.rst
+++ b/Doc/glossary.rst
@@ -718,6 +718,8 @@
 
 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
--- a/Doc/whatsnew/3.6.rst
+++ b/Doc/whatsnew/3.6.rst
@@ -503,6 +503,11 @@
 * 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
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -527,6 +527,31 @@
 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
--- a/Lib/test/test_getargs2.py
+++ b/Lib/test/test_getargs2.py
@@ -658,6 +658,39 @@
 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
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -201,6 +201,18 @@
 - 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
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -1028,6 +1028,21 @@
 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 @@
 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
--- a/Python/getargs.c
+++ b/Python/getargs.c
@@ -1443,7 +1443,8 @@
 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 @@
 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 @@
 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 @@
 "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);
+ }
+ }
+ 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;
+ }
+
+ 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);
 }
 }
- 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);
- }
- continue;
- }
-
- if (i < min) {
- 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)
- return cleanreturn(1, &freelist);
 
 /* We are into optional args, skip thru to any remaining
 * keyword args */
@@ -1594,6 +1622,15 @@
 }
 }
 
+ 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 @@
 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
--- a/Tools/clinic/clinic.py
+++ b/Tools/clinic/clinic.py
@@ -644,7 +644,7 @@
 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 @@
 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 @@
 
 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 @@
 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 @@
 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 @@
 # "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 @@
 # 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 @@
 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 @@
 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 @@
 # 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
 
-- 
Repository URL: https://hg.python.org/cpython
