Antoine Pitrou <report@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) {