Patches item #1600346, was opened at 2006-11-21 06:57 Message generated for change (Comment added) made by jackdied You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1600346&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: Python 3000 Status: Open Resolution: None Priority: 5 Private: No Submitted By: ganges master (gangesmaster) Assigned to: Jack Diederich (jackdied) Summary: __bool__ instead of __nonzero__ Initial Comment: this patch replaces __nonzero__ with __bool__, to make bool type symmetrical to all other types (__int__, __float__, etc.) some notes: * the latex docs have been updated * the slot name was changed from nb_nonzero to nb_bool * all XXX_nonzero methods where changed to XXX_bool * all the comments relating to nb_nonzero have been updated * stdlib was updated * all the test suites were updated otoh, i couldn't get it to compile (MSVC++2003), because of a strange bug in ceval.c (none of my code). seems the GETLOCAL macro causes syntactic problems, but i didn't have time to check it thoroughly. ---------------------------------------------------------------------- >Comment By: Jack Diederich (jackdied) Date: 2006-11-22 19:15 Message: Logged In: YES user_id=591932 Originator: NO Thanks to the [ever vigilant] gbrandl I have sf permissions so the patch is now attached below. ---------------------------------------------------------------------- Comment By: Georg Brandl (gbrandl) Date: 2006-11-22 18:55 Message: Logged In: YES user_id=849994 Originator: NO Applies and compiles fine here. I was about to comment about a refleak somewhere (I ran test_iter with -R and it showed 222 leaked refs), but it shows up without the patch too. I'll update PEP 3100 after you checked it in, so that people writing the conversion tool/advisor will remember to include this one. ---------------------------------------------------------------------- Comment By: Georg Brandl (gbrandl) Date: 2006-11-22 18:47 Message: Logged In: YES user_id=849994 Originator: NO Applies and compiles fine here. I was about to comment about a refleak somewhere (I ran test_iter with -R and it showed 222 leaked refs), but it shows up without the patch too. I'll update PEP 3100 after you checked it in, so that people writing the conversion tool/advisor will remember to include this one. ---------------------------------------------------------------------- Comment By: Jack Diederich (jackdied) Date: 2006-11-22 18:35 Message: Logged In: YES user_id=591932 Originator: NO I don't have sf permissions so I put a tweaked version of the patch at http://jackdied.com/static/bool6.patch * keeps slots_nb_bool closer to the original * extra test in test_bool to exercise __len__ fallbacks * test_iter and test_decimal pass * reverted longobject.c indentation The regular __len__ machinery does get called but the 2.5 code does the bool/int check either way because it made the code shorter (both __len__ and __nonzero__ could return an int so doing the same check for both didn't hurt anything). The new patch treats the result of __len__ and __bool__ slightly differently because __bool__ must return __bool__. I added a couple lines but otherwise left the function as it was (so the lookup_maybe()s should be fine) If there are no objections I'll check it in. ---------------------------------------------------------------------- Comment By: ganges master (gangesmaster) Date: 2006-11-22 15:43 Message: Logged In: YES user_id=1406776 Originator: YES > In the > new code it looks like func will get decrefed if it is NULL > but PyErr_Occurred is not set. true. i neglected that :) ---------------------------------------------------------------------- Comment By: Jack Diederich (jackdied) Date: 2006-11-22 15:30 Message: Logged In: YES user_id=591932 Originator: NO Ah, I missed the default of -1 The refactoring changed how the results of lookup_maybe()s are used. From the note at the top of lookup_maybe() it might return NULL and _not_ set an exception. In the old code it checked for NULL versus PyErr_Occcurred. In the new code it looks like func will get decrefed if it is NULL but PyErr_Occurred is not set. I would revert the refactoring and change only the bits you have to. [I'll pass on the __len__ thing until I have a chance to look at it more] ---------------------------------------------------------------------- Comment By: ganges master (gangesmaster) Date: 2006-11-22 15:07 Message: Logged In: YES user_id=1406776 Originator: YES > There used to be a "result = -1;" result is initialized to -1 at the beginning of the function > did you mean to reindent the long_as_number struct in longobject.c? i realigned the struct with tab of 4, where it used to be tabs of 8, so things got messed up a little. i tried my best to fix it, but i can't fix what i can't see :) > bool5.patch removes the check for int for the __len__() result as this is > already done in the __len__() call itself no it's not! we are not using PyObject_Len, we directly invoke __len__, which may return anything it wishes. you can't skip that check. ---------------------------------------------------------------------- Comment By: Jack Diederich (jackdied) Date: 2006-11-22 14:58 Message: Logged In: YES user_id=591932 Originator: NO I'm not convinced slot_nb_bool is doing the right thing in the PyBool_Check(tmp) line. There used to be a "result = -1;" right after the PyErr_Format and there isn't anymore (maybe result gets set to -1 somewhere else but I can't tell where). Can you undo the refactoring of that function in general? Some of the decrefs moved around too and they look correct but it would be easier to tell if they just stayed the same. Also, did you mean to reindent the long_as_number struct in longobject.c? ---------------------------------------------------------------------- Comment By: Walter Dörwald (doerwalter) Date: 2006-11-22 13:55 Message: Logged In: YES user_id=89016 Originator: NO bool5.patch removes the check for int for the __len__() result as this is already done in the __len__() call itself. It adds a few tests to test_bool.py::BoolTest.test_convert_to_bool() and fixes the description of __bool__ in Doc/ref/ref3.tex. ---------------------------------------------------------------------- Comment By: ganges master (gangesmaster) Date: 2006-11-21 16:33 Message: Logged In: YES user_id=1406776 Originator: YES fixed a bug with type checking when using __len__ ---------------------------------------------------------------------- Comment By: ganges master (gangesmaster) Date: 2006-11-21 13:28 Message: Logged In: YES user_id=1406776 Originator: YES * slot_nb_bool now requires __bool__ to return only a boolean * tab-width issues (hopefully) fixed ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1600346&group_id=5470 _______________________________________________ Patches mailing list Patches@python.org http://mail.python.org/mailman/listinfo/patches
