msg52300 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-03-23 03:27 |
Python allows arbitrary sequences after * in calls, but an expression following ** must be a (subclass of) dict. The attached patch makes Python accept arbitrary mappings after **. |
|
|
msg52301 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2007-03-23 18:45 |
Two flaws: - what if kwdict is NULL? Shouldn't happen, but the old version raises an error in this case. - if PyDict_Update fails, tmpdict is leaked. Please ask on python-dev whether to include this feature. |
|
|
msg52302 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-03-23 19:44 |
I've fixed the flaws in star-star-mapping-1.patch . I will ask on python-dev about the feature. If the proposal is accepted, I will probably refactor the code to avoid using the second goto label. File Added: star-star-mapping-1.patch |
|
|
msg52303 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-04-18 16:02 |
I have asked on python-dev almost a month ago, but did not get any response: http://mail.python.org/pipermail/python-dev/2007-March/072321.html I have recently posted another message that got no response as well: http://mail.python.org/pipermail/python-dev/2007-April/072653.html Is it possible that my messages are not being delivered to the list? |
|
|
msg52304 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-04-18 18:34 |
Georg, Thanks for reposting my proposal. Since BDFL is +1 on the idea, I feel motivated to polish the patch. In your early comment you noted that kwdict == NULL check may be superficial. I agree. It does not look like it is possible that ext_do_call is called with CALL_FLAG_KW set and a null pointer at the top of the stack. It looks like this may only happen if the stack is corrupted by a misbehaving c extension. In this case, however, throwing a type error is too gentle. PyErr_BadInternalCall() seems to be more appropriate if anything at all. I propose to eliminate the null check for kwdict alltogether. This would be consistent with the way stararg is handled later in this function. I am going to run the testsuit without the null check and will post a revised patch shortly. |
|
|
msg52305 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-04-18 19:38 |
The proposed patch handles ** arguments similar to the way * arguments are treated. If the expression following ** evaluates to an object that is not a dictionary, the new code attempts to update a new dictionary with that object. This procedure is closely similar to what PySequence_Tuple does with * arguments. (Python API does not provide PyMapping_Dict.) If new dictionary is succesfully created and updated, it replaces the original ** argument. If that attempt fails because kwdict is not a mapping (does not have a 'keys' attribute, a type error is raised explaining that a mapping is expected. All other errors, e.g. MemoryError are percolated up. (I am am removing the previous versions of the patch.) File Added: star-star-mapping.patch |
|
|
msg52306 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-04-18 19:53 |
File Added: star-star-mapping.patch |
|
|
msg52307 - (view) |
Author: Ziga Seilnacht (zseil) *  |
Date: 2007-04-24 10:32 |
The patch looks good to me. The only thing that could be improved is the error message. It would be nice if it contained the information about kwdict's type. |
|
|
msg52308 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-04-24 13:19 |
zseil: It would be nice if it contained the information about kwdict's type. I agree, the new patch adds type information to both * and ** error messages. File Added: star-star-mapping-msg.patch |
|
|
msg52309 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2007-05-20 15:30 |
What else needs to be done for the patch to go in? Guido OKed the feature and zseil gave a favorable code review. Is there any reason not to apply it? |
|
|
msg52310 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2007-05-21 20:34 |
I think it just needs someone to commit it. ;) See revision 55495. |
|
|