Patches item #1390657, was opened at 2005-12-26 16:09 Message generated for change (Comment added) made by nascheme You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1390657&group_id=5470
Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: Core (C code) Group: None Status: Open Resolution: None Priority: 5 Submitted By: Armin Rigo (arigo) Assigned to: Nobody/Anonymous (nobody) Summary: NotImplemented->TypeError in __add__ and __mul__ Initial Comment: This is a fix for bug #1355842, with a comprehensive test checking that implementing a binary special method but returning NotImplemented does indeed cause a TypeError to be raised. In addition to the case reported in #1355842 (x+=y) the test found another case where NotImplemented was just returned (A()*5), a case where an OverflowError or ValueError was raised instead of TypeError (A()*large_integer), and a case where an unexpected AttributeError was raised (5*A()). The proposed patch also reverts the changes done in svn revisions 25556-25557 corresponding to bug #516727, because it makes that fix unnecessary by adopting a more radical approach. All the problems are due to hard-to-control interactions between the various PyTypeObject slots for addition and multiplication (nb_add vs sq_concat, nb_multiply vs sq_repeat). The approach of the present patch is to not define sq_concat, sq_repeat, sq_inplace_concat and sq_inplace_repeat slots *at all* for user-defined types, even if they have __add__ and __mul__ methods. This is the only sane solution I found, specially with respect to the OverflowError/ValueError problem (caused by trying to convert the integer to a C int in order to try to call sq_repeat). It is also consistent with the behavior of instances of old-style classes -- the InstanceType did not define sq_concat and sq_repeat, either. I'll propose a redefinition of operator.concat() and operator.repeat() on python-dev. ---------------------------------------------------------------------- >Comment By: Neil Schemenauer (nascheme) Date: 2005-12-27 21:35 Message: Logged In: YES user_id=35752 Your change to abstract.c that adds "result = binop_type_error(...)" is certainly correct. Py_NotImplemented should not be returned. Small nits: why not remove the SLOT1 lines from typeobject.c instead of commenting them out? If you want to leave them as comments then you should add an explaination as to why they should not be there. Also, I don't personally like having SF issue numbers in comments as I would rather have the comment be self explanatory. Who knows, SF might go away. The changes to PySequence_* look correct to me as well. ---------------------------------------------------------------------- Comment By: Armin Rigo (arigo) Date: 2005-12-27 16:16 Message: Logged In: YES user_id=4771 Attached a second patch, extending PySequence_Repeat() and PySequence_Concat() to match more precisely their documentation, which says that they should be equivalent to multiplication and addition respectively, at least when the proper arguments are sequences. In 2.4 and HEAD, it works with new-style instances defining __add__ or __mul__, but it does not work with old-style instances. If the concat_repeat_cleanup.diff patch is checked in, then it stops working for new-style instances too. So the sequence_operations.diff patch cleans up the situation by making this work in all cases. Compatibility note: PySequence_Concat/Repeat() are mostly never used in the core; they are essentially only used by operator.concat() and operator.repeat(). Both patches together should thus not break existing code and could be considered as a bug fix. I'd recommend that we check them in both HEAD and the 2.4 branch. The patch also addresses PySequence_InPlaceConcat/Repeat() but without testing them -- they can't be tested from Python, so they are already completely untested. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1390657&group_id=5470 _______________________________________________ Patches mailing list [email protected] http://mail.python.org/mailman/listinfo/patches
