Stefan Krah <stefan-use...@bytereef.org> added the comment:
Antoine Pitrou <rep...@bugs.python.org> wrote:
> > Thus,
> > ((unsigned char)((c) & 0xff)) and ((unsigned char)(c)) should produce
> > the same results.
>
> If it's the case, it's probably optimized away by the compiler anyway.
Yes, the asm output is the same. I was more concerned about readability.
Historically, Py_CHARMASK started as ((c) & 0xff), hence the name. In r34974
UCHAR_MAX = 2**CHAR_BIT-1 = 255 was enforced. In r61936 the cast was added.
> > There is no reason not to do the cast when __CHAR_UNSIGNED__ is
> > defined (it will be a no-op).
>
> Agreed. It also reduces the opportunity for bugs :)
Ok, let's say we use ((unsigned char)((c) & 0xff)) also for __CHAR_UNSIGNED__.
What should the comment say about the intended argument? I've examined all
occurrences in the tree and almost always Py_CHARMASK is used with either
char arguments or int arguments that are directly derived from a char, so
no EOF issues arise.
Exceptions:
===========
Index: Objects/unicodeobject.c
===================================================================
--- Objects/unicodeobject.c (revision 82192)
+++ Objects/unicodeobject.c (working copy)
@@ -8417,6 +8417,7 @@
else if (c >= '0' && c <= '9') {
prec = c - '0';
while (--fmtcnt >= 0) {
+ /* XXX: c and *fmt are Py_UNICODE */
c = Py_CHARMASK(*fmt++);
if (c < '0' || c > '9')
break;
Index: Modules/_json.c
===================================================================
--- Modules/_json.c (revision 82192)
+++ Modules/_json.c (working copy)
@@ -603,6 +603,7 @@
}
}
else {
+ /* XXX: c is Py_UNICODE */
char c_char = Py_CHARMASK(c);
chunk = PyString_FromStringAndSize(&c_char, 1);
if (chunk == NULL) {
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue9036>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
