Author vstinner
Recipients vstinner
Date 2017-01-24.08:49:43
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1485247784.31.0.540630884546.issue29360@psf.upfronthosting.co.za>
In-reply-to
Content
While running the test suite on issues #29259 (tp_fastcall) and #29358 (tp_fastnew and tp_fastinit), a few test failed on the following assertion of _PyStack_AsDict():

   assert(PyUnicode_CheckExact(key));

For example, test_dict calls dict(**{1: 2}) which must raise a TypeError. I'm not sure who is responsible to check if all keys are strings: _PyStack_AsDict(), PyArg_ParseXXX() or the final function?


The check is already implemented in dict_init(), dict_update_common() calls PyArg_ValidateKeywordArguments():

>>> dict(**{1:2})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: keyword arguments must be strings


In Python 3.7, PyArg_ParseTupleAndKeywords(), _PyArg_ParseTupleAndKeywordsFast() and _PyArg_ParseStackAndKeywords() and PyArg_ValidateKeywordArguments() raise an error if a key of keyword argumens is not a string:

    if (!PyUnicode_Check(key)) {
        PyErr_SetString(PyExc_TypeError,
                        "keywords must be strings");
        return cleanreturn(0, &freelist);
    }


IMHO the CALL_FUNCTION_EX instruction and the FASTCALL calling convention must not check if all keys are string: the check must only be done in the function (which can be a type constructor like dict_init()), for performance. Almost all functions use one the PyArg_ParseXXX() function. Only very special cases like dict_init() use more specific code to handle keyword arguments.

By the way, _PyObject_FastCallKeywords() already contains the following comment:

    /* kwnames must only contains str strings, no subclass, and all keys must
       be unique: these checks are implemented in Python/ceval.c and
       _PyArg_ParseStackAndKeywords(). */


Note: Technically, I know that it's possible to put non-string keys in a dictionary and in a type dictionary. PyPy (and other Python implementations?) don't support non-string in type dictionary. Do you know use cases where func(**kwargs) must accept non-string keys?


At the end, this long issue is a simple patch replacing an assertion with a comment in _PyStack_AsDict(): see attached patch ;-)
History
Date User Action Args
2017-01-24 08:49:44vstinnersetrecipients: + vstinner
2017-01-24 08:49:44vstinnersetmessageid: <1485247784.31.0.540630884546.issue29360@psf.upfronthosting.co.za>
2017-01-24 08:49:44vstinnerlinkissue29360 messages
2017-01-24 08:49:43vstinnercreate