Message160000
Attached is rev 3 of my patch, incorporating mainly backing out of dumb ideas. Thanks for the feedback, Serhiy and Mark!
> My name is Serhiy. :)
My genuine apologies! In my defense it was rather late.
> 'P' has the advantage that you can safely backward-compatibly
> remove the restriction by replacing 'P' on 'p'. :)
That's not a reason to use 'P'. Why you should use 'P' in the first place?
> In this line in the patch (Python/getargs.c):
> + if (val == -1 || PyErr_Occurred()) {
> Isn't that call to PyErr_Occurred() redundant?
Certainly one of the two expressions is!
My thinking was: if the call fails, then the val == -1 will be an early-exit and we can save the call to PyErr_Occurred. This was always a dumb idea, as the 99.999999% case is that PyObject_IsTrue succeeds, in which case we would have called PyErr_Occured anyway. Some savings!
I can't find any documentation on permitted return values from nb_bool. However, PyObject_IsTrue itself says about its own return value:
/* if it is negative, it should be either -1 or -2 */
So I definitely shouldn't check specifically for -1.
Having meditated on it, I think either I should either just call PyErr_Occured, check for explicit failure (val < 0), or explicit success (val >= 0). I've opted for the last of those.
I considered briefly trying to make 'P' handle subclasses of bool. But then I hit the problem of: okay, what now? Call nb_bool? I note that bool itself doesn't define nb_bool. Anyway, what lunatic would subclass bool?
I'm really on the fence about 'P'. Serhiy is definitely pro-, everyone else seems to think it shouldn't be used. However nobody has argued against its inclusion. At the moment I'm -0 on it myself, but since the code is written...
Do we have an anti-champion? |
|
Date |
User |
Action |
Args |
2012-05-05 16:20:40 | larry | set | recipients:
+ larry, loewis, mark.dickinson, eric.smith, serhiy.storchaka |
2012-05-05 16:20:40 | larry | set | messageid: <1336234840.08.0.314625865379.issue14705@psf.upfronthosting.co.za> |
2012-05-05 16:20:39 | larry | link | issue14705 messages |
2012-05-05 16:20:38 | larry | create | |
|