Re: [Python-Dev] [Python-checkins] cpython: Move PyUnicode_WCHAR_KIND outside PyUnicode_Kind enum
Move PyUnicode_WCHAR_KIND outside PyUnicode_Kind enum
What's the rationale for that change? It's a valid kind value, after all, and the C convention is that an enumeration lists all valid values (else there wouldn't be a need for an enumeration in the first place). Regards, Martin
On 18/12/2011 20:34, "Martin v. Löwis" wrote:
Move PyUnicode_WCHAR_KIND outside PyUnicode_Kind enum
What's the rationale for that change? It's a valid kind value, after all, and the C convention is that an enumeration lists all valid values (else there wouldn't be a need for an enumeration in the first place).
PyUnicode_KIND() only returns PyUnicode_1BYTE_KIND, PyUnicode_2BYTE_KIND or PyUnicode_4BYTE_KIND. Outside unicodeobject.c, you are not supposed to see PyUnicode_WCHAR_KIND. For switch/case, it avoids the need of adding a dummy PyUnicode_WCHAR_KIND case (or a default case). Victor
Am 18.12.2011 20:45, schrieb Victor Stinner:
On 18/12/2011 20:34, "Martin v. Löwis" wrote:
Move PyUnicode_WCHAR_KIND outside PyUnicode_Kind enum
What's the rationale for that change? It's a valid kind value, after all, and the C convention is that an enumeration lists all valid values (else there wouldn't be a need for an enumeration in the first place).
PyUnicode_KIND() only returns PyUnicode_1BYTE_KIND, PyUnicode_2BYTE_KIND or PyUnicode_4BYTE_KIND. Outside unicodeobject.c, you are not supposed to see PyUnicode_WCHAR_KIND.
Why do you say that? It can very well happen, assuming you call PyUnicode_KIND on a string that is not ready. That would be a bug in the module, but people do make bugs when programming.
For switch/case, it avoids the need of adding a dummy PyUnicode_WCHAR_KIND case (or a default case).
... and thus hides a potential source of errors, as people may forget to call ready, and then fall through the case, letting god-knows-what happen. If the rationale is to simplify silencing compiler errors, I vote for reverting the enumeration back to a macro list. Regards, Martin
On 18/12/2011 21:04, "Martin v. Löwis" wrote:
PyUnicode_KIND() only returns PyUnicode_1BYTE_KIND, PyUnicode_2BYTE_KIND or PyUnicode_4BYTE_KIND. Outside unicodeobject.c, you are not supposed to see PyUnicode_WCHAR_KIND.
Why do you say that? It can very well happen, assuming you call PyUnicode_KIND on a string that is not ready. That would be a bug in the module, but people do make bugs when programming.
I added assert(PyUnicode_IS_READY(op)) to the macro, so the bug will be quickly catched in debug mode. I forgot that it is just an assertion and few people use Python compiled in debug mode.
If the rationale is to simplify silencing compiler errors, I vote for reverting the enumeration back to a macro list.
I'm not sure that gcc will not complain if only 3 values are handled. I agree to revert the commit if that helps developers to write bugs. Victor
Am 18.12.2011 21:16, schrieb Victor Stinner:
On 18/12/2011 21:04, "Martin v. Löwis" wrote:
PyUnicode_KIND() only returns PyUnicode_1BYTE_KIND, PyUnicode_2BYTE_KIND or PyUnicode_4BYTE_KIND. Outside unicodeobject.c, you are not supposed to see PyUnicode_WCHAR_KIND.
Why do you say that? It can very well happen, assuming you call PyUnicode_KIND on a string that is not ready. That would be a bug in the module, but people do make bugs when programming.
I added assert(PyUnicode_IS_READY(op)) to the macro, so the bug will be quickly catched in debug mode. I forgot that it is just an assertion and few people use Python compiled in debug mode.
If the rationale is to simplify silencing compiler errors, I vote for reverting the enumeration back to a macro list.
I'm not sure that gcc will not complain if only 3 values are handled. I agree to revert the commit if that helps developers to write bugs.
It helps to detect bugs. User should be aware that there is an additional case, and put something like case PyUnicode_WCHAR_KIND: /* string is guaranteed to be ready here */ assert(0); into their code. Regards, Martin
participants (2)
-
"Martin v. Löwis" -
Victor Stinner