This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients methane, serhiy.storchaka, vstinner
Date 2017-01-24.11:18:15
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1485256695.73.0.136946276018.issue29360@psf.upfronthosting.co.za>
In-reply-to
Content
Attached pystack_asdict-2.patch removes the two assertions and rewrites _PyStack_AsDict() documentation to describe better how it handles non-string and duplicated keys (don't check keys type and merge duplicated keys).


> We should either ensure that _PyStack_AsDict() is called only by CALL_FUNCTION_KW, and not by other way, or remove both assertions. I prefer the second option. They were here only for self-testing.

CALL_FUNTION_EX doesn't use _PyStack_AsDict() (at least, currently, I should carefully review my tp_fast{new,init,call} patches).

When I wrote _PyStack_AsDict(), I added the GetItem==NULL assertion because I didn't know how to handle this corner case.

For best performances, I'm in favor of not calling GetItem in _PyStack_AsDict() and so merge duplicated keys. But before removing the assertion, I wanted to make sure that it doesn't break the Python semantics.

The problem is that I'm unable to find any concrete tcase in Python 3.6 nor Python 3.7 where it would be possible to call _PyStack_AsDict() with duplicate keys. See my analysis below.

I agree to remove both assertion. I just checked, _PyStack_AsDict() already explicitly explain that keys must be strings and must be unique, but are not checked. My patch now also mentions that duplicated keys are merged.

--

(1) When a Python is called by _PyFunction_FastCallDict(), a TypeError is raised if an argument is passed as a positional argument and a keyword argument. Example: "def func(x): pass; func(1, x=2)".

(2) If for some reasons, two keys of keyword arguments are equal (same hash value and are equals) but go into "**kwargs", the last value is kept: _PyEval_EvalCodeWithName() calls PyDict_SetItem() which replaces existing key to the new value. I failed to find an example here.

(3) Calling other functions using kwargs dictionary from a FASTCALL function converts stack+kwnames arguments to kwargs dictionary using _PyStack_AsDict(). _PyCFunction_FastCallKeywords() implements that for the METH_VARARGS|METH_KEYWORDS calling convention. For example, round(3.14, ndigits=1) calls _PyCFunction_FastCallKeywords() with kwnames=('ndigits',) and args=[3.14, 1], _PyStack_AsDict() creates {'ndigits': 1} dictionary.


The tricky question is how (3) handles duplicated keys in dictionary.


"round(3.14, ndigits=1)" uses CALL_FUNCTION_KW which creates kwnames=('ndigits,') constant tuple. The compiler raises a SyntaxError if a function is called with a duplicate key. Without modifying a code object, I don't see any obvious way to pass duplicate keys in kwnames to CALL_FUNCTION_KW.

CALL_FUNCTION_EX expects a dictionary on the stack. For METH_VARARGS|METH_KEYWORDS C functions, the dictionary is passed through, there is not conversion.

If CALL_FUNCTION_EX gets a mapping which is not exactly the dict type, PyDict_New() + PyDict_Update() is used to convert the mapping into a regular dict. This function merges duplicated keys.


Without modifying code objects nor writing buggy C code using _PyObject_FastCallKeywords(), I'm not sure that it's possible to pass duplicated keys to _PyStack_AsDict(). It seems that the issue is more theorical, no?
History
Date User Action Args
2017-01-24 11:18:15vstinnersetrecipients: + vstinner, methane, serhiy.storchaka
2017-01-24 11:18:15vstinnersetmessageid: <1485256695.73.0.136946276018.issue29360@psf.upfronthosting.co.za>
2017-01-24 11:18:15vstinnerlinkissue29360 messages
2017-01-24 11:18:15vstinnercreate