| msg28478 - (view) |
Author: dib (dib_at_work) |
Date: 2006-05-11 16:17 |
| The fix for bug #1119418 (xrange() builtin accepts keyword arg silently) included in Python 2.4.2c1+ breaks code that passes keyword argument(s) into classes derived from the built-in set class, even if those derived classes explictly accept those keyword arguments and avoid passing them down to the built-in base class. Simplified version of code in attached BuiltinSetKeywordArgumentsCheckBroken.py fails at (G) due to bug #1119418 if version < 2.4.2c1; if version >= 2.4.2c1 (G) passes thanks to that bug fix, but instead (H) incorrectly-in-my-view fails. [Presume similar cases would fail for xrange and the other classes mentioned in #1119418.] -- David Bruce (Tested on 2.4, 2.4.2, 2.5a2 on linux2, win32.) |
|
|
| msg28479 - (view) |
Author: Georg Brandl (georg.brandl) *  |
Date: 2006-05-11 17:23 |
| Logged In: YES user_id=849994 Raymond, what to do in this case? Note that other built-in types, such as list(), do accept keyword arguments. |
|
|
| msg28480 - (view) |
Author: Ziga Seilnacht (zseil) * |
Date: 2006-05-19 20:02 |
| Logged In: YES user_id=1326842 This bug was introduced as part of the fix for bug #1119418. It also affects collections.deque. Can't the _PyArg_NoKeywords check simply be moved to set_init and deque_init as it was done for zipimport.zipimporter? array.array doesn't need to be changed, since it already does all of its initialization in its __new__ method. The rest of the types changed in that fix should not be affected, since they are immutable. |
|
|
| msg28481 - (view) |
Author: Ziga Seilnacht (zseil) * |
Date: 2006-05-20 01:19 |
| Logged In: YES user_id=1326842 See patch #1491939 |
|
|
| msg28482 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2007-01-06 02:26 |
| I prefer the approach used by list(). |
|
|
| msg28483 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-01-10 21:30 |
| I'll do that, only in set_init, you have if (!PyArg_UnpackTuple(args, self->ob_type->tp_name, 0, 1, &iterable)) Changing this to use PyArg_ParseTupleAndKeywords would require a format string of "|O:" + self->ob_type->tp_name Is it worth constructing that string each time set_init() is called or should it just be " |
O:set" for sets and frozensets? |
|
| msg28484 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2007-01-11 00:49 |
| My proposed solution: - if(!PyArg_NoKeywords("set()", kwds) + if(type == &PySet_Type && !PyArg_NoKeywords("set()", kwds) |
|
|
| msg28485 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2007-01-11 18:30 |
| I fixed setobject.c in revisions 53380 and 53381. Please apply similar fixes to all the other places being bitten my the pervasive NoKeywords tests. |
|
|
| msg28486 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-01-11 19:56 |
| Attaching patch. File Added: nokeywordchecks.diff |
|
|
| msg28487 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2007-01-11 20:43 |
| That looks about right. Please add test cases that fail without the patch and succeed with the patch. Also, put a comment in Misc/NEWS. If the whole test suite passes, go ahead and check-in to Py2.5.1 and the head. Thanks, Raymond |
|
|
| msg28488 - (view) |
Author: Neal Norwitz (nnorwitz) * |
Date: 2007-01-17 07:22 |
| Were these changes applied by Raymond? I don't think there were NEWS entries though. |
|
|
| msg28489 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-01-17 09:13 |
| I'll create the testcases and commit the patch (as well as NEWS entries :) when I find the time. |
|
|
| msg28490 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-01-21 10:29 |
| Committed as rev. 53509, 53510 (2.5). |
|
|
| msg28491 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2007-02-19 02:38 |
| There are more of these that need to be fixed: bufferobject.c:236: if (!_PyArg_NoKeywords("buffer()", kw)) classobject.c:2261: if (!_PyArg_NoKeywords("instancemethod", kw)) exceptions.c:57: if (!_PyArg_NoKeywords(self->ob_type->tp_name, kwds)) funcobject.c:653: if (!_PyArg_NoKeywords("classmethod", kwds)) funcobject.c:810: if (!_PyArg_NoKeywords("staticmethod", kwds)) rangeobject.c:48: if (!_PyArg_NoKeywords("xrange()", kw)) sliceobject.c:197: if (!_PyArg_NoKeywords("slice()", kw)) typeobject.c:5773: if (!_PyArg_NoKeywords("super", kwds)) |
|
|
| msg28492 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2007-02-19 08:02 |
| The exceptions.c one is in __init__, so it is not a problem when the derived class doesn't pass along kwargs to super.__init__. I deliberately left the others as they were since their type objects lack the Py_TPFLAGS_BASETYPE flag. |
|
|
| msg28493 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2007-02-19 08:21 |
| Okay, it looks like this bug was already fixed. |
|
|