check for PyUnicode_READY look backwards
Hi, with the new Unicode API, there are many checks like: + if (PyUnicode_READY(*filename)) + goto handle_error; Every time I read it, I get it wrong: "If filename is ready, then fail" then I have to remember that the function returns either 0 or -1. I'd prefer it was written : if (PyUnicode_READY(*filename) < 0) because "< 0" clearly indicates an error condition. That's how all calls to PyType_Ready are written, for example. Am I the only one to be distracted by this idiom? -- Amaury Forgeot d'Arc
On 6 Oct, 2011, at 14:57, Amaury Forgeot d'Arc wrote:
Hi,
with the new Unicode API, there are many checks like: + if (PyUnicode_READY(*filename)) + goto handle_error;
Every time I read it, I get it wrong: "If filename is ready, then fail" then I have to remember that the function returns either 0 or -1.
I'd prefer it was written : if (PyUnicode_READY(*filename) < 0) because "< 0" clearly indicates an error condition. That's how all calls to PyType_Ready are written, for example.
Am I the only one to be distracted by this idiom?
I prefer the '< 0' variant as well, for the same reason as you. Ronald
-- Amaury Forgeot d'Arc _______________________________________________ Python-Dev mailing list Python-Dev@python.org http://mail.python.org/mailman/listinfo/python-dev Unsubscribe: http://mail.python.org/mailman/options/python-dev/ronaldoussoren%40mac.com
On Thu, Oct 6, 2011 at 10:31 AM, Ronald Oussoren <ronaldoussoren@mac.com> wrote:
On 6 Oct, 2011, at 14:57, Amaury Forgeot d'Arc wrote:
I'd prefer it was written : if (PyUnicode_READY(*filename) < 0) because "< 0" clearly indicates an error condition. That's how all calls to PyType_Ready are written, for example.
Am I the only one to be distracted by this idiom?
I prefer the '< 0' variant as well, for the same reason as you.
+1 here as well. The Unix/C "0 as success" idiom breaks my Python conditioned brain, so including the explicit "< 0" in the C code helps resolve that impedance mismatch. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
Amaury Forgeot d'Arc <amauryfa <at> gmail.com> writes:
I'd prefer it was written : if (PyUnicode_READY(*filename) < 0) because "< 0" clearly indicates an error condition.
Why not just have it return 0 on error? This would be more consistent with API functions that return "false" values like NULL and would just be if (!PyUnicode_READY(s)) return NULL; in code. Regards, Benjamin
On Thu, Oct 6, 2011 at 4:47 PM, Benjamin Peterson <benjamin@python.org> wrote:
Amaury Forgeot d'Arc <amauryfa <at> gmail.com> writes:
I'd prefer it was written : if (PyUnicode_READY(*filename) < 0) because "< 0" clearly indicates an error condition.
Why not just have it return 0 on error? This would be more consistent with API functions that return "false" values like NULL and would just be
if (!PyUnicode_READY(s)) return NULL;
in code.
Alas, that isn't the convention in C - courtesy of Unix, the convention is that for integer return codes, "0" means success. Yes, this is annoying, but violating it means you're not writing idiomatic C any more, you're trying to write Python-in-C. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Thu, 6 Oct 2011 17:40:20 -0400 Nick Coghlan <ncoghlan@gmail.com> wrote:
On Thu, Oct 6, 2011 at 4:47 PM, Benjamin Peterson <benjamin@python.org> wrote:
Amaury Forgeot d'Arc <amauryfa <at> gmail.com> writes:
I'd prefer it was written : if (PyUnicode_READY(*filename) < 0) because "< 0" clearly indicates an error condition.
Why not just have it return 0 on error? This would be more consistent with API functions that return "false" values like NULL and would just be
if (!PyUnicode_READY(s)) return NULL;
in code.
Alas, that isn't the convention in C - courtesy of Unix, the convention is that for integer return codes, "0" means success.
C is quite inconsistent, and so is our own C API. if (PyUnicode_READY(s)) { ...} definitely looks like the code block will be executed if the unicode string is ready, though. Regards Antoine.
2011/10/6 Benjamin Peterson <benjamin@python.org>:
Why not just have it return 0 on error? This would be more consistent with API functions that return "false" values like NULL and would just be
if (!PyUnicode_READY(s)) return NULL;
Most functions of the Python C API seems to follow one of two ways to indicate an error: - functions that return PyObject* will return NULL - functions that return an int will return -1 -- Amaury Forgeot d'Arc
Benjamin Peterson wrote:
Why not just have it return 0 on error? This would be more consistent with API functions that return "false" values like NULL
But that would make it confusingly different from all the other functions that return ints. The NULL convention is only used when the function returns a pointer. -- Greg
Am 06.10.11 14:57, schrieb Amaury Forgeot d'Arc:
Hi,
with the new Unicode API, there are many checks like: + if (PyUnicode_READY(*filename)) + goto handle_error;
I think you are misinterpreting what you are seeing. There are not *many* such checks. Of the PyUnicode_READY checks, 106 take the form if (PyUnicode_READY(foo) == -1) return NULL; 30 tests take the form that you mention. I believe all of those have been added by Victor, who just didn't follow the convention. So, Victor: please correct them. Regards, Martin
Le 07/10/2011 00:20, "Martin v. Löwis" a écrit :
Am 06.10.11 14:57, schrieb Amaury Forgeot d'Arc:
Hi,
with the new Unicode API, there are many checks like: + if (PyUnicode_READY(*filename)) + goto handle_error;
I think you are misinterpreting what you are seeing. There are not *many* such checks. Of the PyUnicode_READY checks, 106 take the form
if (PyUnicode_READY(foo) == -1) return NULL;
30 tests take the form that you mention.
I believe all of those have been added by Victor, who just didn't follow the convention.
Yes, I wrote if (PyUnicode_READY(foo)), but I agree that it is confusing when you read the code, especially because we have also a PyUnicode_IS_READY(foo) macro! if (!PyUnicode_READY(foo)) is not better, also because of PyUnicode_IS_READY(foo). I prefer PyUnicode_IS_READY(foo) < 0 over PyUnicode_IS_READY(foo) == -1. Victor
Victor Stinner <victor.stinner@haypocalc.com> wrote:
Yes, I wrote if (PyUnicode_READY(foo)), but I agree that it is confusing when you read the code, especially because we have also a PyUnicode_IS_READY(foo) macro!
if (!PyUnicode_READY(foo)) is not better, also because of PyUnicode_IS_READY(foo).
I prefer PyUnicode_IS_READY(foo) < 0 over PyUnicode_IS_READY(foo) == -1.
Do you mean PyUnicode_READY(foo) < 0? I also prefer that idiom. Stefan Krah
Le 07/10/2011 10:07, Stefan Krah a écrit :
Victor Stinner<victor.stinner@haypocalc.com> wrote:
Yes, I wrote if (PyUnicode_READY(foo)), but I agree that it is confusing when you read the code, especially because we have also a PyUnicode_IS_READY(foo) macro!
if (!PyUnicode_READY(foo)) is not better, also because of PyUnicode_IS_READY(foo).
I prefer PyUnicode_IS_READY(foo)< 0 over PyUnicode_IS_READY(foo) == -1.
Do you mean PyUnicode_READY(foo)< 0? I also prefer that idiom.
Oops, yes I mean PyUnicode_READY(foo)< 0. Victor
if (!PyUnicode_READY(foo)) is not better, also because of PyUnicode_IS_READY(foo).
I prefer PyUnicode_IS_READY(foo) < 0 over PyUnicode_IS_READY(foo) == -1.
Ok, so feel free to replace all == -1 tests with < 0 tests as well. I'll point out that the test for -1 is also widespread in Python, e.g. when checking return values from PyObject_SetAttrString, BaseException_init, PyThread_create_key, PyObject_DelAttrString, etc. Regards, Martin
On Fri, Oct 7, 2011 at 9:21 AM, "Martin v. Löwis" <martin@v.loewis.de> wrote:
> if (!PyUnicode_READY(foo)) is not better, also because of
PyUnicode_IS_READY(foo).
I prefer PyUnicode_IS_READY(foo) < 0 over PyUnicode_IS_READY(foo) == -1.
Ok, so feel free to replace all == -1 tests with < 0 tests as well.
I'll point out that the test for -1 is also widespread in Python, e.g. when checking return values from PyObject_SetAttrString, BaseException_init, PyThread_create_key, PyObject_DelAttrString, etc.
FWIW, I don't mind whether it's "< 0" or "== -1", so long as there's a comparison there to kick my brain out of Python boolean logic mode. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On 10/7/2011 10:06 AM, Nick Coghlan wrote:
On Fri, Oct 7, 2011 at 9:21 AM, "Martin v. Löwis"<martin@v.loewis.de> wrote:
if (!PyUnicode_READY(foo)) is not better, also because of
PyUnicode_IS_READY(foo).
I prefer PyUnicode_IS_READY(foo)< 0 over PyUnicode_IS_READY(foo) == -1.
Ok, so feel free to replace all == -1 tests with< 0 tests as well.
I'll point out that the test for -1 is also widespread in Python, e.g. when checking return values from PyObject_SetAttrString, BaseException_init, PyThread_create_key, PyObject_DelAttrString, etc.
FWIW, I don't mind whether it's "< 0" or "== -1", so long as there's a comparison there to kick my brain out of Python boolean logic mode.
Is there any speed difference (on common x86/64 processors and compilers)? I would expect that '< 0' should be optimized to just check the sign bit and 'if n < 0' to 'load n; jump-non-negative'. -- Terry Jan Reedy
On 10/7/2011 7:13 PM, Terry Reedy wrote:
On 10/7/2011 10:06 AM, Nick Coghlan wrote:
FWIW, I don't mind whether it's "< 0" or "== -1", so long as there's a comparison there to kick my brain out of Python boolean logic mode.
Is there any speed difference (on common x86/64 processors and compilers)? I would expect that '< 0' should be optimized to just check the sign bit and 'if n < 0' to 'load n; jump-non-negative'.
There are several different ways to express those operators depending on the context. If "n" is worth moving into a register, then "<0" will get to use a "test" and it's fewer instruction bytes than a "cmp", but otherwise, it is no better. So, there is a very special case where "<0" is better, but I think you'd be hard-pressed to measure it against the noise. -- Scott Dial scott@scottdial.com
participants (11)
-
"Martin v. Löwis" -
Amaury Forgeot d'Arc -
Antoine Pitrou -
Benjamin Peterson -
Greg Ewing -
Nick Coghlan -
Ronald Oussoren -
Scott Dial -
Stefan Krah -
Terry Reedy -
Victor Stinner