Index: Python/getargs.c =================================================================== --- Python/getargs.c (Revision 61080) +++ Python/getargs.c (Arbeitskopie) @@ -1345,6 +1345,7 @@ return retval; } +#define IS_END_OF_FORMAT(c) (c == '\0' || c == ';' || c == ':') static int vgetargskeywords(PyObject *args, PyObject *keywords, const char *format, @@ -1352,13 +1353,10 @@ { char msgbuf[512]; int levels[32]; - const char *fname, *message; - int min, max; - const char *formatsave; + const char *fname, *msg, *custom_msg, *keyword; + int min = INT_MAX; int i, len, nargs, nkeywords; - const char *msg; - char **p; - PyObject *freelist = NULL; + PyObject *freelist = NULL, *current_arg; assert(args != NULL && PyTuple_Check(args)); assert(keywords == NULL || PyDict_Check(keywords)); @@ -1366,168 +1364,108 @@ assert(kwlist != NULL); assert(p_va != NULL); - /* Search the format: - message <- error msg, if any (else NULL). - fname <- routine name, if any (else NULL). - min <- # of required arguments, or -1 if all are required. - max <- most arguments (required + optional). - Check that kwlist has a non-NULL entry for each arg. - Raise error if a tuple arg spec is found. - */ - fname = message = NULL; - formatsave = format; - p = kwlist; - min = -1; - max = 0; - while ((i = *format++) != '\0') { - if (isalpha(Py_CHARMASK(i)) && i != 'e') { - max++; - if (*p == NULL) { - PyErr_SetString(PyExc_RuntimeError, - "more argument specifiers than " - "keyword list entries"); - return 0; - } - p++; - } - else if (i == '|') - min = max; - else if (i == ':') { - fname = format; - break; - } - else if (i == ';') { - message = format; - break; - } - else if (i == '(') { - PyErr_SetString(PyExc_RuntimeError, - "tuple found in format when using keyword " - "arguments"); - return 0; - } + /* grab the function name or custom error msg first (mutually exclusive) */ + fname = strchr(format, ':'); + if (fname) { + fname++; + custom_msg = NULL; } - format = formatsave; - if (*p != NULL) { - PyErr_SetString(PyExc_RuntimeError, - "more keyword list entries than " - "argument specifiers"); - return 0; + else { + custom_msg = strchr(format,';'); + if (custom_msg) + custom_msg++; } - if (min < 0) { - /* All arguments are required. */ - min = max; - } - nargs = PyTuple_GET_SIZE(args); - nkeywords = keywords == NULL ? 0 : PyDict_Size(keywords); + /* scan kwlist and get greatest possible nbr of args */ + for (len=0; kwlist[len]; len++) + continue; - /* make sure there are no duplicate values for an argument; - its not clear when to use the term "keyword argument vs. - keyword parameter in messages */ - if (nkeywords > 0) { - for (i = 0; i < nargs; i++) { - const char *thiskw = kwlist[i]; - if (thiskw == NULL) - break; - if (PyDict_GetItemString(keywords, thiskw)) { - PyErr_Format(PyExc_TypeError, - "keyword parameter '%s' was given " - "by position and by name", - thiskw); - return 0; - } - else if (PyErr_Occurred()) - return 0; - } - } - - /* required arguments missing from args can be supplied by keyword - arguments; set len to the number of positional arguments, and, - if that's less than the minimum required, add in the number of - required arguments that are supplied by keywords */ - len = nargs; - if (nkeywords > 0 && nargs < min) { - for (i = nargs; i < min; i++) { - if (PyDict_GetItemString(keywords, kwlist[i])) - len++; - else if (PyErr_Occurred()) - return 0; - } - } - - /* make sure we got an acceptable number of arguments; the message - is a little confusing with keywords since keyword arguments - which are supplied, but don't match the required arguments - are not included in the "%d given" part of the message - XXX and this isn't a bug!? */ - if (len < min || max < len) { - if (message == NULL) { - PyOS_snprintf(msgbuf, sizeof(msgbuf), - "%.200s%s takes %s %d argument%s " - "(%d given)", - fname==NULL ? "function" : fname, - fname==NULL ? "" : "()", - min==max ? "exactly" - : len < min ? "at least" : "at most", - len < min ? min : max, - (len < min ? min : max) == 1 ? "" : "s", - len); - message = msgbuf; - } - PyErr_SetString(PyExc_TypeError, message); + nargs = PyTuple_GET_SIZE(args); + nkeywords = (keywords == NULL) ? 0 : PyDict_Size(keywords); + if (nargs + nkeywords > len) { + PyErr_Format(PyExc_TypeError, "%s%s takes at most %d " + "argument%s (%d given)", + (fname == NULL) ? "function" : fname, + (fname == NULL) ? "" : "()", + len, + (len == 1) ? "" : "s", + nargs + nkeywords); return 0; } - /* convert the positional arguments */ - for (i = 0; i < nargs; i++) { - if (*format == '|') + /* convert tuple args and keyword args in same loop, using kwlist to drive process */ + for (i = 0; i < len; i++) { + keyword = kwlist[i]; + if (*format == '|') { + min = i; format++; - msg = convertitem(PyTuple_GET_ITEM(args, i), &format, p_va, - flags, levels, msgbuf, sizeof(msgbuf), - &freelist); - if (msg) { - seterror(i+1, msg, levels, fname, message); + } + if (IS_END_OF_FORMAT(*format)) { + PyErr_Format(PyExc_RuntimeError, + "More keyword list entries (%d) than " + "format specifiers (%d)", len, i); return cleanreturn(0, freelist); } - } - - /* handle no keyword parameters in call */ - if (nkeywords == 0) - return cleanreturn(1, freelist); - - /* convert the keyword arguments; this uses the format - string where it was left after processing args */ - for (i = nargs; i < max; i++) { - PyObject *item; - if (*format == '|') - format++; - item = PyDict_GetItemString(keywords, kwlist[i]); - if (item != NULL) { - Py_INCREF(item); - msg = convertitem(item, &format, p_va, flags, levels, - msgbuf, sizeof(msgbuf), &freelist); - Py_DECREF(item); - if (msg) { - seterror(i+1, msg, levels, fname, message); + 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); } - --nkeywords; - if (nkeywords == 0) - break; } - else if (PyErr_Occurred()) + else if (nkeywords && PyErr_Occurred()) return cleanreturn(0, freelist); - else { - msg = skipitem(&format, p_va, flags); + 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) { - levels[0] = 0; - seterror(i+1, msg, levels, fname, message); + 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 */ + msg = skipitem(&format, p_va, flags); + if (msg) { + PyErr_Format(PyExc_RuntimeError, "%s: '%s'", msg, + format); + return cleanreturn(0, freelist); + } } + if (!IS_END_OF_FORMAT(*format)) { + PyErr_Format(PyExc_RuntimeError, + "more argument specifiers than keyword list entries " + "(remaining format:'%s')", format); + return cleanreturn(0, freelist); + } + /* make sure there are no extraneous keyword arguments */ if (nkeywords > 0) { PyObject *key, *value; @@ -1541,7 +1479,7 @@ return cleanreturn(0, freelist); } ks = PyString_AsString(key); - for (i = 0; i < max; i++) { + for (i = 0; i < len; i++) { if (!strcmp(ks, kwlist[i])) { match = 1; break; @@ -1564,7 +1502,7 @@ static char * skipitem(const char **p_format, va_list *p_va, int flags) { - const char *format = *p_format; + const char *format = *p_format; char c = *format++; switch (c) { @@ -1671,16 +1609,33 @@ } break; } - + + case '(': /* bypass tuple, not handled at all previously */ + { + char *msg; + for (;;) { + if (*format==')') + break; + if (IS_END_OF_FORMAT(*format)) + return "Unmatched left paren in format " + "string"; + msg = skipitem(&format, p_va, flags); + if (msg) + return msg; + } + format++; + break; + } + + case ')': + return "Unmatched right paren in format string"; + default: err: return "impossible"; } - /* The "(...)" format code for tuples is not handled here because - * it is not allowed with keyword args. */ - *p_format = format; return NULL; } Index: Lib/test/test_getargs2.py =================================================================== --- Lib/test/test_getargs2.py (Revision 61080) +++ Lib/test/test_getargs2.py (Arbeitskopie) @@ -1,5 +1,6 @@ import unittest from test import test_support +from _testcapi import getargs_keywords import warnings warnings.filterwarnings("ignore", @@ -248,9 +249,57 @@ raise ValueError self.assertRaises(TypeError, getargs_tuple, 1, seq()) +class Keywords_TestCase(unittest.TestCase): + def test_positional_args(self): + # using all positional args + self.assertEquals( + getargs_keywords((1,2), 3, (4,(5,6)), (7,8,9), 10), + (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) + ) + def test_mixed_args(self): + # positional and keyword args + self.assertEquals( + getargs_keywords((1,2), 3, (4,(5,6)), arg4=(7,8,9), arg5=10), + (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) + ) + def test_keyword_args(self): + # all keywords + self.assertEquals( + getargs_keywords(arg1=(1,2), arg2=3, arg3=(4,(5,6)), arg4=(7,8,9), arg5=10), + (1, 2, 3, 4, 5, 6, 7, 8, 9, 10) + ) + def test_optional_args(self): + # missing optional keyword args, skipping tuples + self.assertEquals( + getargs_keywords(arg1=(1,2), arg2=3, arg5=10), + (1, 2, 3, -1, -1, -1, -1, -1, -1, 10) + ) + def test_required_args(self): + # required arg missing + try: + getargs_keywords(arg1=(1,2)) + except TypeError, err: + self.assertEquals(str(err), "Required argument 'arg2' (pos 2) not found") + else: + self.fail('TypeError should have been raised') + def test_too_many_args(self): + try: + getargs_keywords((1,2),3,(4,(5,6)),(7,8,9),10,111) + except TypeError, err: + self.assertEquals(str(err), "function takes at most 5 arguments (6 given)") + else: + self.fail('TypeError should have been raised') + def test_invalid_keyword(self): + # extraneous keyword arg + try: + getargs_keywords((1,2),3,arg5=10,arg666=666) + except TypeError, err: + self.assertEquals(str(err), "'arg666' is an invalid keyword argument for this function") + else: + self.fail('TypeError should have been raised') def test_main(): - tests = [Signed_TestCase, Unsigned_TestCase, Tuple_TestCase] + tests = [Signed_TestCase, Unsigned_TestCase, Tuple_TestCase, Keywords_TestCase] try: from _testcapi import getargs_L, getargs_K except ImportError: Index: Modules/_testcapimodule.c =================================================================== --- Modules/_testcapimodule.c (Revision 61080) +++ Modules/_testcapimodule.c (Arbeitskopie) @@ -306,6 +306,22 @@ return Py_BuildValue("iii", a, b, c); } +/* test PyArg_ParseTupleAndKeywords */ +static PyObject *getargs_keywords(PyObject *self, PyObject *args, PyObject *kwargs) +{ + static char *keywords[] = {"arg1","arg2","arg3","arg4","arg5", NULL}; + static char *fmt="(ii)i|(i(ii))(iii)i"; + int int_args[10]={-1, -1, -1, -1, -1, -1, -1, -1, -1, -1}; + + if (!PyArg_ParseTupleAndKeywords(args, kwargs, fmt, keywords, + &int_args[0], &int_args[1], &int_args[2], &int_args[3], &int_args[4], + &int_args[5], &int_args[6], &int_args[7], &int_args[8], &int_args[9])) + return NULL; + return Py_BuildValue("iiiiiiiiii", + int_args[0], int_args[1], int_args[2], int_args[3], int_args[4], + int_args[5], int_args[6], int_args[7], int_args[8], int_args[9]); +} + /* Functions to call PyArg_ParseTuple with integer format codes, and return the result. */ @@ -732,6 +748,8 @@ PyDoc_STR("This is a pretty normal docstring.")}, {"getargs_tuple", getargs_tuple, METH_VARARGS}, + {"getargs_keywords", (PyCFunction)getargs_keywords, + METH_VARARGS|METH_KEYWORDS}, {"getargs_b", getargs_b, METH_VARARGS}, {"getargs_B", getargs_B, METH_VARARGS}, {"getargs_H", getargs_H, METH_VARARGS},