STINNER Victor <victor.stin...@haypocalc.com> added the comment:

I don't think that macros specific to unicodeobject.c should get the 
_PY_UNICODE_ prefix. "_Py_" prefix is reserved to exported symbols, but symbols 
reserved for the Python interprefer itself. For _Py_UNICODE_NEXT, you can call 
it NEXT_CHARACTER().

_Py_UNICODE_ISHIGHSURROGATE,_Py_UNICODE_ISLOWSURROGATE and 
_Py_UNICODE_JOIN_SURROGATES are only used once, I would prefer to see them 
inlined in _Py_UNICODE_NEXT.

The first cast to Py_UCS4 in _Py_UNICODE_JOIN_SURROGATES is useless.

It looks like the macro can be simplified to something like:

#define _Py_UNICODE_NEXT(ptr, end)                                      \
    (_Py_UNICODE_ISHIGHSURROGATE(*(ptr)) && (ptr) < (end)) && 
_Py_UNICODE_ISLOWSURROGATE((ptr)[1] ?           \
     ((ptr) += 2,_Py_UNICODE_JOIN_SURROGATES((ptr)[-2], (ptr)[-1])) :  \
     (Py_UCS4)*(ptr)++)

(you don't need two "a?b:c")

I would prefer to see _Py_UNICODE_NEXT as a function instead of a macro because 
it has border effect (ptr++ or ptr += 2). You cannot write 
Py_UNICODE_ISALNUM(_Py_UNICODE_NEXT(p, e)) for example, because 
Py_UNICODE_ISALNUM is defined as:

#define Py_UNICODE_ISALNUM(ch) (Py_UNICODE_ISALPHA(ch) || 
Py_UNICODE_ISDECIMAL(ch) || Py_UNICODE_ISDIGIT(ch) || Py_UNICODE_ISNUMERIC(ch))

And so _Py_UNICODE_NEXT is expanded 4 times. It is also horrible to debug a 
program having such long macro. If you really want to keep it as a macro, 
please add a least a big warning.

It's also confusing to have two attachments with the same name :-/

----------
nosy: +haypo

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue9200>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to