diff --git a/Doc/c-api/arg.rst b/Doc/c-api/arg.rst index b41130ede416e3..46ad5da1f9d799 100644 --- a/Doc/c-api/arg.rst +++ b/Doc/c-api/arg.rst @@ -384,12 +384,19 @@ inside nested parentheses. They are: ``$`` :c:func:`PyArg_ParseTupleAndKeywords` only: Indicates that the remaining arguments in the Python argument list are - keyword-only. Currently, all keyword-only arguments must also be optional - arguments, so ``|`` must always be specified before ``$`` in the format - string. + keyword-only. By default, arguments following ``$`` are optional and + ``|`` must be specified before ``$``. For required, keyword-only arguments, + specify ``@`` after ``$``. .. versionadded:: 3.3 +``@`` + :c:func:`PyArg_ParseTupleAndKeywords` only: + Indicates that the remaining arguments in the Python argument list are + required keyword-only arguments. Must be specified after ``|`` and ``$``. + + .. versionadded:: 3.8 + ``:`` The list of format units ends here; the string after the colon is used as the function name in error messages (the "associated value" of the exception that diff --git a/Include/modsupport.h b/Include/modsupport.h index f17060c2e1e2a4..5bcd91b5b1944d 100644 --- a/Include/modsupport.h +++ b/Include/modsupport.h @@ -94,6 +94,7 @@ typedef struct _PyArg_Parser { int pos; /* number of positional-only arguments */ int min; /* minimal number of arguments */ int max; /* maximal number of positional arguments */ + int required_kwonly_start; /* first required kwonly argument */ PyObject *kwtuple; /* tuple of keyword parameter names */ struct _PyArg_Parser *next; } _PyArg_Parser; diff --git a/Lib/test/test_getargs2.py b/Lib/test/test_getargs2.py index d9bcb1d75bd029..3c2735dd8ad782 100644 --- a/Lib/test/test_getargs2.py +++ b/Lib/test/test_getargs2.py @@ -661,6 +661,185 @@ def test_surrogate_keyword(self): "'\udc80' is an invalid keyword argument for this function"): getargs_keyword_only(1, 2, **{'\uDC80': 10}) +class RequiredKeywordOnly_TestCase(unittest.TestCase): + from _testcapi import getargs_required_keyword_only as getargs + + def test_basic_args(self): + self.assertEqual( + self.getargs(1, kw_required=2), + (1, -1, 2, -1) + ) + self.assertEqual( + self.getargs(1, 2, kw_required=3), + (1, 2, 3, -1) + ) + self.assertEqual( + self.getargs(1, 2, kw_required=3, kw_optional=4), + (1, 2, 3, 4) + ) + self.assertEqual( + self.getargs(1, arg2=2, kw_required=3, kw_optional=4), + (1, 2, 3, 4) + ) + self.assertEqual( + self.getargs(1, arg2=2, kw_required=3), + (1, 2, 3, -1) + ) + self.assertEqual( + self.getargs(arg1=1, arg2=2, kw_required=3, kw_optional=4), + (1, 2, 3, 4) + ) + self.assertEqual( + self.getargs(arg1=1, arg2=2, kw_required=3), + (1, 2, 3, -1) + ) + self.assertEqual( + self.getargs(arg1=1, kw_required=3, kw_optional=4), + (1, -1, 3, 4) + ) + self.assertEqual( + self.getargs(arg1=1, kw_required=3), + (1, -1, 3, -1) + ) + + def test_required_positional_args(self): + # required positional arg missing + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs() + + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs(kw_required=3) + + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs(arg2=2, kw_required=4) + + def test_required_keyword_args(self): + # required keyword arg missing + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1, 2) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1, arg2=2) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1, arg2=2, kw_optional=3) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(arg1=1, arg2=2, kw_optional=3) + + def test_keyword_only(self): + with self.assertRaisesRegex(TypeError, + r"function takes at most 2 positional arguments \(3 given\)"): + self.getargs(1, 2, 3) + +class RequiredKeywordOnly2_TestCase(unittest.TestCase): + from _testcapi import getargs_required_keyword_only2 as getargs + + def test_basic_args(self): + self.assertEqual( + self.getargs(1, kw_required=3), + (1, 3, -1) + ) + self.assertEqual( + self.getargs(1, 4, kw_required=3), + (1, 3, 4) + ) + self.assertEqual( + self.getargs(arg1=1, kw_required=3), + (1, 3, -1) + ) + + def test_required_positional_args(self): + # required positional arg missing + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs() + + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs(kw_required=3) + + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs(arg2=2, kw_required=4) + + def test_required_keyword_args(self): + # required keyword arg missing + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1, 2) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1, arg2=2) + + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(arg1=1, arg2=2) + + def test_keyword_only(self): + with self.assertRaisesRegex(TypeError, + r"function takes at most 2 positional arguments \(3 given\)"): + self.getargs(1, 2, 3) + +class RequiredKeywordOnly3_TestCase(unittest.TestCase): + from _testcapi import getargs_required_keyword_only3 as getargs + + def test_basic_args(self): + self.assertEqual( + self.getargs(1, kw_required=2), + (1, 2) + ) + self.assertEqual( + self.getargs(arg1=1, kw_required=2), + (1, 2) + ) + + def test_required_positional_args(self): + # required positional arg missing + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs() + + with self.assertRaisesRegex(TypeError, + r"function missing required argument 'arg1' \(pos 1\)"): + self.getargs(kw_required=3) + + def test_required_keyword_args(self): + # required keyword arg missing + with self.assertRaisesRegex(TypeError, + r"function missing required keyword-only argument 'kw_required'"): + self.getargs(1) + + def test_keyword_only(self): + with self.assertRaisesRegex(TypeError, + r"function takes at most 1 positional argument \(2 given\)"): + self.getargs(1, 2) + +# Run all the same required keywords tests against the "fast" versions +class RequiredKeywordOnlyFast_TestCase(RequiredKeywordOnly_TestCase): + from _testcapi import getargs_required_keyword_only_fast as getargs + +class RequiredKeywordOnlyFast2_TestCase(RequiredKeywordOnly2_TestCase): + from _testcapi import getargs_required_keyword_only_fast2 as getargs + +class RequiredKeywordOnlyFast3_TestCase(RequiredKeywordOnly3_TestCase): + from _testcapi import getargs_required_keyword_only_fast3 as getargs class PositionalOnlyAndKeywords_TestCase(unittest.TestCase): from _testcapi import getargs_positional_only_and_keywords as getargs @@ -1001,8 +1180,8 @@ def test_skipitem(self): # skip parentheses, the error reporting is inconsistent about them # skip 'e', it's always a two-character code - # skip '|' and '$', they don't represent arguments anyway - if c in '()e|$': + # skip '|', '$', and '@', they don't represent arguments anyway + if c in '()e|$@': continue # test the format unit when not skipped diff --git a/Misc/NEWS.d/next/C API/2019-02-12-16-48-41.bpo-34235.sOA_0A.rst b/Misc/NEWS.d/next/C API/2019-02-12-16-48-41.bpo-34235.sOA_0A.rst new file mode 100644 index 00000000000000..8518d92d098c2c --- /dev/null +++ b/Misc/NEWS.d/next/C API/2019-02-12-16-48-41.bpo-34235.sOA_0A.rst @@ -0,0 +1 @@ +Support required keyword-only arguments in `PyArg_ParseTupleAndKeywords`. diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 85810f30b1c528..cc6a701dc9a0fd 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1063,6 +1063,102 @@ getargs_positional_only_and_keywords(PyObject *self, PyObject *args, PyObject *k return Py_BuildValue("iii", required, optional, keyword); } +static PyObject * +getargs_required_keyword_only(PyObject *self, PyObject *args, PyObject *kwargs) +{ + static char *keywords[] = { + "arg1", "arg2", "kw_optional", "kw_required", NULL}; + int arg1 = -1; + int arg2 = -1; + int required = -1; + int optional = -1; + + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|i$i@i", keywords, + &arg1, &arg2, &optional, &required)) + return NULL; + return Py_BuildValue("iiii", arg1, arg2, required, optional); +} + +static PyObject * +getargs_required_keyword_only_fast( + PyObject *self, PyObject *args, PyObject *kwargs) +{ + static const char * const keywords[] = { + "arg1", "arg2", "kw_optional", "kw_required", NULL}; + static _PyArg_Parser parser = {"i|i$i@i", keywords, 0}; + int arg1 = -1; + int arg2 = -1; + int required = -1; + int optional = -1; + + if (!_PyArg_ParseTupleAndKeywordsFast(args, kwargs, &parser, + &arg1, &arg2, &optional, &required)) + return NULL; + return Py_BuildValue("iiii", arg1, arg2, required, optional); +} + +static PyObject * +getargs_required_keyword_only2(PyObject *self, PyObject *args, PyObject *kwargs) +{ + static char *keywords[] = { + "arg1", "arg2", "kw_required", NULL}; + int arg1 = -1; + int arg2 = -1; + int required = -1; + + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|i$@i", keywords, + &arg1, &arg2, &required)) + return NULL; + return Py_BuildValue("iii", arg1, required, arg2); +} + +static PyObject * +getargs_required_keyword_only_fast2( + PyObject *self, PyObject *args, PyObject *kwargs) +{ + static const char * const keywords[] = { + "arg1", "arg2", "kw_required", NULL}; + static _PyArg_Parser parser = {"i|i$@i", keywords, 0}; + int arg1 = -1; + int arg2 = -1; + int required = -1; + + if (!_PyArg_ParseTupleAndKeywordsFast(args, kwargs, &parser, + &arg1, &arg2, &required)) + return NULL; + return Py_BuildValue("iii", arg1, required, arg2); +} + +static PyObject * +getargs_required_keyword_only3(PyObject *self, PyObject *args, PyObject *kwargs) +{ + static char *keywords[] = { + "arg1", "kw_required", NULL}; + int arg1 = -1; + int required = -1; + + if (!PyArg_ParseTupleAndKeywords(args, kwargs, "i|$@i", keywords, + &arg1, &required)) + return NULL; + return Py_BuildValue("ii", arg1, required); +} + +static PyObject * +getargs_required_keyword_only_fast3( + PyObject *self, PyObject *args, PyObject *kwargs) +{ + static const char * const keywords[] = { + "arg1", "kw_required", NULL}; + static _PyArg_Parser parser = {"i|$@i", keywords, 0}; + int arg1 = -1; + int required = -1; + + if (!_PyArg_ParseTupleAndKeywordsFast(args, kwargs, &parser, + &arg1, &required)) + return NULL; + return Py_BuildValue("ii", arg1, required); +} + /* Functions to call PyArg_ParseTuple with integer format codes, and return the result. */ @@ -4777,6 +4873,24 @@ static PyMethodDef TestMethods[] = { {"getargs_positional_only_and_keywords", (PyCFunction)(void(*)(void))getargs_positional_only_and_keywords, METH_VARARGS|METH_KEYWORDS}, + {"getargs_required_keyword_only", + (PyCFunction)(void(*)(void))getargs_required_keyword_only, + METH_VARARGS|METH_KEYWORDS}, + {"getargs_required_keyword_only2", + (PyCFunction)(void(*)(void))getargs_required_keyword_only2, + METH_VARARGS|METH_KEYWORDS}, + {"getargs_required_keyword_only3", + (PyCFunction)(void(*)(void))getargs_required_keyword_only3, + METH_VARARGS|METH_KEYWORDS}, + {"getargs_required_keyword_only_fast", + (PyCFunction)(void(*)(void))getargs_required_keyword_only_fast, + METH_VARARGS|METH_KEYWORDS}, + {"getargs_required_keyword_only_fast2", + (PyCFunction)(void(*)(void))getargs_required_keyword_only_fast2, + METH_VARARGS|METH_KEYWORDS}, + {"getargs_required_keyword_only_fast3", + (PyCFunction)(void(*)(void))getargs_required_keyword_only_fast3, + 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 c491169abe5793..15a35728ad2ef1 100644 --- a/Python/getargs.c +++ b/Python/getargs.c @@ -1624,6 +1624,8 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, const char *fname, *msg, *custom_msg; int min = INT_MAX; int max = INT_MAX; + int required_kwonly_start = INT_MAX; + int has_required_kws = 0; int i, pos, len; int skip = 0; Py_ssize_t nargs, nkwargs; @@ -1707,6 +1709,11 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, "Invalid format string ($ before |)"); return cleanreturn(0, &freelist); } + + /* If there are optional args, figure out whether we have + * required keyword arguments so that we don't bail without + * enforcing them. */ + has_required_kws = strchr(format, '@') != NULL; } if (*format == '$') { if (max != INT_MAX) { @@ -1750,6 +1757,22 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, return cleanreturn(0, &freelist); } } + if (*format == '@') { + if (min == INT_MAX && max == INT_MAX) { + PyErr_SetString(PyExc_SystemError, + "Invalid format string " + "(@ without preceding | and $)"); + return cleanreturn(0, &freelist); + } + if (required_kwonly_start != INT_MAX) { + PyErr_SetString(PyExc_SystemError, + "Invalid format string (@ specified twice)"); + return cleanreturn(0, &freelist); + } + + required_kwonly_start = i; + format++; + } if (IS_END_OF_FORMAT(*format)) { PyErr_Format(PyExc_SystemError, "More keyword list entries (%d) than " @@ -1779,7 +1802,7 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, continue; } - if (i < min) { + if (i < min || i >= required_kwonly_start) { if (i < pos) { assert (min == INT_MAX); assert (max == INT_MAX); @@ -1790,11 +1813,22 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, * or the end of the format. */ } else { - PyErr_Format(PyExc_TypeError, "%.200s%s missing required " - "argument '%s' (pos %d)", - (fname == NULL) ? "function" : fname, - (fname == NULL) ? "" : "()", - kwlist[i], i+1); + if (i >= max) { + PyErr_Format(PyExc_TypeError, + "%.200s%s missing required " + "keyword-only argument '%s'", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", + kwlist[i]); + } + else { + PyErr_Format(PyExc_TypeError, + "%.200s%s missing required " + "argument '%s' (pos %d)", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", + kwlist[i], i+1); + } return cleanreturn(0, &freelist); } } @@ -1802,7 +1836,7 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, * fulfilled and no keyword args left, with no further * validation. XXX Maybe skip this in debug build ? */ - if (!nkwargs && !skip) { + if (!nkwargs && !skip && !has_required_kws) { return cleanreturn(1, &freelist); } } @@ -1830,7 +1864,9 @@ vgetargskeywords(PyObject *args, PyObject *kwargs, const char *format, return cleanreturn(0, &freelist); } - if (!IS_END_OF_FORMAT(*format) && (*format != '|') && (*format != '$')) { + if (!IS_END_OF_FORMAT(*format) && + (*format != '|') && (*format != '$') && (*format != '@')) + { PyErr_Format(PyExc_SystemError, "more argument specifiers than keyword list entries " "(remaining format:'%s')", format); @@ -1894,6 +1930,7 @@ parser_init(struct _PyArg_Parser *parser) const char * const *keywords; const char *format, *msg; int i, len, min, max, nkw; + int required_kwonly_start; PyObject *kwtuple; assert(parser->format != NULL); @@ -1930,6 +1967,7 @@ parser_init(struct _PyArg_Parser *parser) len = i; min = max = INT_MAX; + required_kwonly_start = INT_MAX; format = parser->format; for (i = 0; i < len; i++) { if (*format == '|') { @@ -1960,6 +1998,22 @@ parser_init(struct _PyArg_Parser *parser) max = i; format++; } + if (*format == '@') { + if (min == INT_MAX && max == INT_MAX) { + PyErr_SetString(PyExc_SystemError, + "Invalid format string " + "(@ without preceding | and $)"); + return 0; + } + if (required_kwonly_start != INT_MAX) { + PyErr_SetString(PyExc_SystemError, + "Invalid format string (@ specified twice)"); + return 0; + } + + required_kwonly_start = i; + format++; + } if (IS_END_OF_FORMAT(*format)) { PyErr_Format(PyExc_SystemError, "More keyword list entries (%d) than " @@ -1976,8 +2030,11 @@ parser_init(struct _PyArg_Parser *parser) } parser->min = Py_MIN(min, len); parser->max = Py_MIN(max, len); + parser->required_kwonly_start = required_kwonly_start; - if (!IS_END_OF_FORMAT(*format) && (*format != '|') && (*format != '$')) { + if (!IS_END_OF_FORMAT(*format) && + (*format != '|') && (*format != '$') && (*format != '@')) + { PyErr_Format(PyExc_SystemError, "more argument specifiers than keyword list entries " "(remaining format:'%s')", format); @@ -2148,6 +2205,9 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs, if (*format == '$') { format++; } + if (*format == '@') { + format++; + } assert(!IS_END_OF_FORMAT(*format)); if (i < nargs) { @@ -2173,8 +2233,8 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs, continue; } - if (i < parser->min) { - /* Less arguments than required */ + if (i < parser->min || i >= parser->required_kwonly_start) { + /* Fewer arguments than required */ if (i < pos) { Py_ssize_t min = Py_MIN(pos, parser->min); PyErr_Format(PyExc_TypeError, @@ -2189,11 +2249,21 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs, } else { keyword = PyTuple_GET_ITEM(kwtuple, i - pos); - PyErr_Format(PyExc_TypeError, "%.200s%s missing required " - "argument '%U' (pos %d)", - (parser->fname == NULL) ? "function" : parser->fname, - (parser->fname == NULL) ? "" : "()", - keyword, i+1); + if (i >= parser->max) { + PyErr_Format( + PyExc_TypeError, "%.200s%s missing required " + "keyword-only argument '%U'", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()", + keyword); + } else { + PyErr_Format( + PyExc_TypeError, "%.200s%s missing required " + "argument '%U' (pos %d)", + (parser->fname == NULL) ? "function" : parser->fname, + (parser->fname == NULL) ? "" : "()", + keyword, i+1); + } } return cleanreturn(0, &freelist); } @@ -2201,7 +2271,7 @@ vgetargskeywordsfast_impl(PyObject *const *args, Py_ssize_t nargs, * fulfilled and no keyword args left, with no further * validation. XXX Maybe skip this in debug build ? */ - if (!nkwargs) { + if (!nkwargs && parser->required_kwonly_start == INT_MAX) { return cleanreturn(1, &freelist); }