what's up with _COMPILING_NEWLIB

what's up with _COMPILING_NEWLIB

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 3830 bytes --]

i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol
that indicates the code currently being compiled is newlib itself so that the
header can change behavior for that environment specifically.  is that what
it's meant for ?

if so, why does it seem to be inconsistently defined ?  newlib/configure.host
will add it for a few random targets, as does the mips-specific
newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/
files.  it feels like the patch below is what we should have.

if that's not what this is for, is there a define that has this meaning ?
in the glibc & gnulib world, the plain _LIBC define indicates this.
-mike

--- a/newlib/configure.host
+++ b/newlib/configure.host
@@ -54,7 +54,7 @@
 #   have_init_fini	have init/fini ("yes" or "no", set to "yes" by default)
 #   noinclude		list of include files to not install
 
-newlib_cflags=
+newlib_cflags="-D_COMPILING_NEWLIB"
 libm_machine_dir=
 machine_dir=
 shared_machine_dir=
@@ -467,15 +467,11 @@ case "${host}" in
 	sys_dir=a29khif
 	signal_dir=
 	;;
-  aarch64*-*-*)
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
-	;;
   amdgcn*)
 	sys_dir=amdgcn
 	have_crt0="no"
 	;;
   arm*-*-*)
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
 	sys_dir=arm
 	if [ "x${newlib_may_supply_syscalls}" = "xno" ] ; then
 	  have_crt0="no"
@@ -652,11 +648,11 @@ case "${host}" in
 	default_newlib_io_long_double="yes"
 	default_newlib_io_pos_args="yes"
 	CC="${CC} -I${cygwin_srcdir}/include"
-	newlib_cflags="${newlib_cflags} -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED"
+	newlib_cflags="${newlib_cflags} -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED"
 	syscall_dir=syscalls
 	;;
   *-*-phoenix*)
-	newlib_cflags="${newlib_cflags} -DMISSING_SYSCALL_NAMES -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_NANOSLEEP"
+	newlib_cflags="${newlib_cflags} -DMISSING_SYSCALL_NAMES -DHAVE_BLKSIZE -DHAVE_NANOSLEEP"
 	default_newlib_io_long_long="yes"
 	syscall_dir=
 	;;
@@ -671,7 +667,6 @@ case "${host}" in
 	default_newlib_io_long_long="yes"
 	default_newlib_io_c99_formats="yes"
 	newlib_cflags="${newlib_cflags} -ffunction-sections -fdata-sections "
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
 newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED -DEXIT_PROVIDED -DSIGNAL_PROVIDED -DGETREENT_PROVIDED -DREENTRANT_SYSCALLS_PROVIDED -DHAVE_NANOSLEEP -DHAVE_BLKSIZE -DHAVE_FCNTL -DHAVE_ASSERT_FUNC"
         # turn off unsupported items in posix directory 
 	newlib_cflags="${newlib_cflags} -D_NO_GETLOGIN -D_NO_GETPWENT -D_NO_GETUT -D_NO_GETPASS -D_NO_SIGSET -D_NO_WORDEXP -D_NO_POPEN -D_NO_POSIX_SPAWN"
--- a/newlib/libc/machine/mips/Makefile.am
+++ b/newlib/libc/machine/mips/Makefile.am
@@ -9,8 +9,6 @@ AM_CCASFLAGS = $(INCLUDES)
 noinst_LIBRARIES = lib.a
 
 lib_a_SOURCES = setjmp.S strlen.c strcmp.S strncpy.c memset.S memcpy.S
-lib_a_CCASFLAGS=$(AM_CCASFLAGS) -D_COMPILING_NEWLIB
-lib_a_CFLAGS=$(AM_CFLAGS) -D_COMPILING_NEWLIB
 
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
--- a/newlib/libc/machine/mips/Makefile.in
+++ b/newlib/libc/machine/mips/Makefile.in
@@ -198,8 +198,6 @@ INCLUDES = $(NEWLIB_CFLAGS) $(CROSS_CFLAGS) $(TARGET_CFLAGS)
 AM_CCASFLAGS = $(INCLUDES)
 noinst_LIBRARIES = lib.a
 lib_a_SOURCES = setjmp.S strlen.c strcmp.S strncpy.c memset.S memcpy.S
-lib_a_CCASFLAGS = $(AM_CCASFLAGS) -D_COMPILING_NEWLIB
-lib_a_CFLAGS = $(AM_CFLAGS) -D_COMPILING_NEWLIB
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
 all: all-am

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: what's up with _COMPILING_NEWLIB

[Corinna Vinschen]

On Nov  6 20:21, Mike Frysinger wrote:
> i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol
> that indicates the code currently being compiled is newlib itself so that the
> header can change behavior for that environment specifically.  is that what
> it's meant for ?
> 
> if so, why does it seem to be inconsistently defined ?  newlib/configure.host
> will add it for a few random targets, as does the mips-specific
> newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/
> files.  it feels like the patch below is what we should have.
> 
> if that's not what this is for, is there a define that has this meaning ?
> in the glibc & gnulib world, the plain _LIBC define indicates this.

_COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need
it during build.  Your patch looks good to me, did you test it for some
targets?


Thanks,
Corinna

Re: what's up with _COMPILING_NEWLIB

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1139 bytes --]

On 08 Nov 2021 11:05, Corinna Vinschen wrote:
> On Nov  6 20:21, Mike Frysinger wrote:
> > i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol
> > that indicates the code currently being compiled is newlib itself so that the
> > header can change behavior for that environment specifically.  is that what
> > it's meant for ?
> > 
> > if so, why does it seem to be inconsistently defined ?  newlib/configure.host
> > will add it for a few random targets, as does the mips-specific
> > newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/
> > files.  it feels like the patch below is what we should have.
> > 
> > if that's not what this is for, is there a define that has this meaning ?
> > in the glibc & gnulib world, the plain _LIBC define indicates this.
> 
> _COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need
> it during build.  Your patch looks good to me, did you test it for some
> targets?

yes, i tested it for bfin-elf and with a change that needed it in ctype.h.
the ctype.h change didn't work until i updated the build this way.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: what's up with _COMPILING_NEWLIB

[Corinna Vinschen]

On Nov  8 06:46, Mike Frysinger wrote:
> On 08 Nov 2021 11:05, Corinna Vinschen wrote:
> > On Nov  6 20:21, Mike Frysinger wrote:
> > > i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol
> > > that indicates the code currently being compiled is newlib itself so that the
> > > header can change behavior for that environment specifically.  is that what
> > > it's meant for ?
> > > 
> > > if so, why does it seem to be inconsistently defined ?  newlib/configure.host
> > > will add it for a few random targets, as does the mips-specific
> > > newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/
> > > files.  it feels like the patch below is what we should have.
> > > 
> > > if that's not what this is for, is there a define that has this meaning ?
> > > in the glibc & gnulib world, the plain _LIBC define indicates this.
> > 
> > _COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need
> > it during build.  Your patch looks good to me, did you test it for some
> > targets?
> 
> yes, i tested it for bfin-elf and with a change that needed it in ctype.h.
> the ctype.h change didn't work until i updated the build this way.

uhm... why does a change in a header file depend on the build system?
That sounds weird.

I tested building on Cygwin, which looks good.

So, here's a question: The patch is ok, just a change to the commit
message would be nice.  However, would you like to take the opportunity
to change _COMPILING_NEWLIB to _LIBC throughout?  That's something we
should have done long ago, I guess.


Thanks,
Corinna

Re: what's up with _COMPILING_NEWLIB

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2083 bytes --]

On 08 Nov 2021 16:05, Corinna Vinschen wrote:
> On Nov  8 06:46, Mike Frysinger wrote:
> > On 08 Nov 2021 11:05, Corinna Vinschen wrote:
> > > On Nov  6 20:21, Mike Frysinger wrote:
> > > > i stumbled across _COMPILING_NEWLIB and it seems to be what i want: a symbol
> > > > that indicates the code currently being compiled is newlib itself so that the
> > > > header can change behavior for that environment specifically.  is that what
> > > > it's meant for ?
> > > > 
> > > > if so, why does it seem to be inconsistently defined ?  newlib/configure.host
> > > > will add it for a few random targets, as does the mips-specific
> > > > newlib/libc/machine/mips/Makefile.am, as do a few specific winsup/cygwin/
> > > > files.  it feels like the patch below is what we should have.
> > > > 
> > > > if that's not what this is for, is there a define that has this meaning ?
> > > > in the glibc & gnulib world, the plain _LIBC define indicates this.
> > > 
> > > _COMPILING_NEWLIB might be older than that.  In Cygwin we certainly need
> > > it during build.  Your patch looks good to me, did you test it for some
> > > targets?
> > 
> > yes, i tested it for bfin-elf and with a change that needed it in ctype.h.
> > the ctype.h change didn't work until i updated the build this way.
> 
> uhm... why does a change in a header file depend on the build system?
> That sounds weird.

i'm making a fix to ctype.h that would utilize _COMPILING_NEWLIB as part of
its fix.  i was holding off posting that until the question of this symbol
could be sorted out.

> I tested building on Cygwin, which looks good.
> 
> So, here's a question: The patch is ok, just a change to the commit
> message would be nice.  However, would you like to take the opportunity
> to change _COMPILING_NEWLIB to _LIBC throughout?  That's something we
> should have done long ago, I guess.

let me do it as a series.  _LIBC is already used in newlib, so i'm afraid
it might not be as clean as just renaming the define.  but i'll give it a
shot and see what blows up.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/2] define _COMPILING_NEWLIB for all targets when compiling

[Mike Frysinger]

The _COMPILING_NEWLIB symbol is for declaring "the code is being
compiled for newlib itself" so headers can change behavior vs the
header being used by users (who should get the normal clean API).
Unfortunately, this symbol is defined inconsistently leading to it
only being useful for a few subsections of the tree.

Pull it out so that it's defined all the time for all targets.
---
 newlib/configure.host                | 11 +++--------
 newlib/libc/machine/mips/Makefile.am |  2 --
 newlib/libc/machine/mips/Makefile.in |  2 --
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/newlib/configure.host b/newlib/configure.host
index ef481ff52f44..e737f73f5f75 100644
--- a/newlib/configure.host
+++ b/newlib/configure.host
@@ -54,7 +54,7 @@
 #   have_init_fini	have init/fini ("yes" or "no", set to "yes" by default)
 #   noinclude		list of include files to not install
 
-newlib_cflags=
+newlib_cflags="-D_COMPILING_NEWLIB"
 libm_machine_dir=
 machine_dir=
 shared_machine_dir=
@@ -467,15 +467,11 @@ case "${host}" in
 	sys_dir=a29khif
 	signal_dir=
 	;;
-  aarch64*-*-*)
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
-	;;
   amdgcn*)
 	sys_dir=amdgcn
 	have_crt0="no"
 	;;
   arm*-*-*)
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
 	sys_dir=arm
 	if [ "x${newlib_may_supply_syscalls}" = "xno" ] ; then
 	  have_crt0="no"
@@ -652,11 +648,11 @@ case "${host}" in
 	default_newlib_io_long_double="yes"
 	default_newlib_io_pos_args="yes"
 	CC="${CC} -I${cygwin_srcdir}/include"
-	newlib_cflags="${newlib_cflags} -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED"
+	newlib_cflags="${newlib_cflags} -DHAVE_OPENDIR -DHAVE_RENAME -DGETREENT_PROVIDED -DSIGNAL_PROVIDED -DHAVE_BLKSIZE -DHAVE_FCNTL -DMALLOC_PROVIDED"
 	syscall_dir=syscalls
 	;;
   *-*-phoenix*)
-	newlib_cflags="${newlib_cflags} -DMISSING_SYSCALL_NAMES -D_COMPILING_NEWLIB -DHAVE_BLKSIZE -DHAVE_NANOSLEEP"
+	newlib_cflags="${newlib_cflags} -DMISSING_SYSCALL_NAMES -DHAVE_BLKSIZE -DHAVE_NANOSLEEP"
 	default_newlib_io_long_long="yes"
 	syscall_dir=
 	;;
@@ -671,7 +667,6 @@ case "${host}" in
 	default_newlib_io_long_long="yes"
 	default_newlib_io_c99_formats="yes"
 	newlib_cflags="${newlib_cflags} -ffunction-sections -fdata-sections "
-	newlib_cflags="${newlib_cflags} -D_COMPILING_NEWLIB"
 newlib_cflags="${newlib_cflags} -DCLOCK_PROVIDED -DMALLOC_PROVIDED -DEXIT_PROVIDED -DSIGNAL_PROVIDED -DGETREENT_PROVIDED -DREENTRANT_SYSCALLS_PROVIDED -DHAVE_NANOSLEEP -DHAVE_BLKSIZE -DHAVE_FCNTL -DHAVE_ASSERT_FUNC"
         # turn off unsupported items in posix directory 
 	newlib_cflags="${newlib_cflags} -D_NO_GETLOGIN -D_NO_GETPWENT -D_NO_GETUT -D_NO_GETPASS -D_NO_SIGSET -D_NO_WORDEXP -D_NO_POPEN -D_NO_POSIX_SPAWN"
diff --git a/newlib/libc/machine/mips/Makefile.am b/newlib/libc/machine/mips/Makefile.am
index 1695b18ff5a7..17f78aad7a2a 100644
--- a/newlib/libc/machine/mips/Makefile.am
+++ b/newlib/libc/machine/mips/Makefile.am
@@ -9,8 +9,6 @@ AM_CCASFLAGS = $(INCLUDES)
 noinst_LIBRARIES = lib.a
 
 lib_a_SOURCES = setjmp.S strlen.c strcmp.S strncpy.c memset.S memcpy.S
-lib_a_CCASFLAGS=$(AM_CCASFLAGS) -D_COMPILING_NEWLIB
-lib_a_CFLAGS=$(AM_CFLAGS) -D_COMPILING_NEWLIB
 
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
diff --git a/newlib/libc/machine/mips/Makefile.in b/newlib/libc/machine/mips/Makefile.in
index 5396e38ad596..d3d900edc1d0 100644
--- a/newlib/libc/machine/mips/Makefile.in
+++ b/newlib/libc/machine/mips/Makefile.in
@@ -198,8 +198,6 @@ INCLUDES = $(NEWLIB_CFLAGS) $(CROSS_CFLAGS) $(TARGET_CFLAGS)
 AM_CCASFLAGS = $(INCLUDES)
 noinst_LIBRARIES = lib.a
 lib_a_SOURCES = setjmp.S strlen.c strcmp.S strncpy.c memset.S memcpy.S
-lib_a_CCASFLAGS = $(AM_CCASFLAGS) -D_COMPILING_NEWLIB
-lib_a_CFLAGS = $(AM_CFLAGS) -D_COMPILING_NEWLIB
 ACLOCAL_AMFLAGS = -I ../../.. -I ../../../..
 CONFIG_STATUS_DEPENDENCIES = $(newlib_basedir)/configure.host
 all: all-am
-- 
2.33.0

[PATCH 2/2] ctype: use less short names in public header

[Mike Frysinger]

We're seeing a build failure in GNU sim code which is using _P locally
but the ctype.h define clashes with it.  Rename these to use the same
symbols that glibc does.  They're a bit more verbose, but seems likely
that we'll have fewer conflicts if glibc isn't seeing them.

However, these shortnames are still used internally by ctype modules
to produce pretty concise source code, so use _COMPILING_NEWLIB to
keep them around when compiling newlib itself where we have better
control over short name conflicts.
---
 newlib/libc/include/ctype.h | 77 ++++++++++++++++++++++---------------
 1 file changed, 45 insertions(+), 32 deletions(-)

diff --git a/newlib/libc/include/ctype.h b/newlib/libc/include/ctype.h
index 932a567e25db..f2a4368da5d2 100644
--- a/newlib/libc/include/ctype.h
+++ b/newlib/libc/include/ctype.h
@@ -57,14 +57,27 @@ extern int isascii_l (int __c, locale_t __l);
 extern int toascii_l (int __c, locale_t __l);
 #endif
 
-#define	_U	01
-#define	_L	02
-#define	_N	04
-#define	_S	010
-#define _P	020
-#define _C	040
-#define _X	0100
-#define	_B	0200
+enum
+{
+  _ISupper = 01,
+  _ISlower = 02,
+  _ISdigit = 04,
+  _ISspace = 010,
+  _ISpunct = 020,
+  _IScntrl = 040,
+  _ISxdigit = 0100,
+  _ISblank = 0200,
+};
+#ifdef _COMPILING_NEWLIB
+# define _U _ISupper
+# define _L _ISlower
+# define _N _ISdigit
+# define _S _ISspace
+# define _P _ISpunct
+# define _C _IScntrl
+# define _X _ISxdigit
+# define _B _ISblank
+#endif
 
 /* For C++ backward-compatibility only.  */
 extern	__IMPORT const char	_ctype_[];
@@ -89,22 +102,22 @@ const char *__locale_ctype_ptr (void);
    an out-of-bounds reference on a 64-bit machine.  */
 #define __ctype_lookup(__c) ((__CTYPE_PTR+sizeof(""[__c]))[(int)(__c)])
 
-#define	isalpha(__c)	(__ctype_lookup(__c)&(_U|_L))
-#define	isupper(__c)	((__ctype_lookup(__c)&(_U|_L))==_U)
-#define	islower(__c)	((__ctype_lookup(__c)&(_U|_L))==_L)
-#define	isdigit(__c)	(__ctype_lookup(__c)&_N)
-#define	isxdigit(__c)	(__ctype_lookup(__c)&(_X|_N))
-#define	isspace(__c)	(__ctype_lookup(__c)&_S)
-#define ispunct(__c)	(__ctype_lookup(__c)&_P)
-#define isalnum(__c)	(__ctype_lookup(__c)&(_U|_L|_N))
-#define isprint(__c)	(__ctype_lookup(__c)&(_P|_U|_L|_N|_B))
-#define	isgraph(__c)	(__ctype_lookup(__c)&(_P|_U|_L|_N))
-#define iscntrl(__c)	(__ctype_lookup(__c)&_C)
+#define	isalpha(__c)	(__ctype_lookup(__c) & (_ISupper|_ISlower))
+#define	isupper(__c)	((__ctype_lookup(__c) & (_ISupper|_ISlower)) == _ISupper)
+#define	islower(__c)	((__ctype_lookup(__c) & (_ISupper|_ISlower)) == _ISlower)
+#define	isdigit(__c)	(__ctype_lookup(__c) & _ISdigit)
+#define	isxdigit(__c)	(__ctype_lookup(__c) & (_ISxdigit|_ISdigit))
+#define	isspace(__c)	(__ctype_lookup(__c) & _ISspace)
+#define ispunct(__c)	(__ctype_lookup(__c) & _ISpunct)
+#define isalnum(__c)	(__ctype_lookup(__c) & (_ISupper|_ISlower|_ISdigit))
+#define isprint(__c)	(__ctype_lookup(__c) & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank))
+#define	isgraph(__c)	(__ctype_lookup(__c) & (_ISpunct|_ISupper|_ISlower|_ISdigit))
+#define iscntrl(__c)	(__ctype_lookup(__c) & _IScntrl)
 
 #if defined(__GNUC__) && __ISO_C_VISIBLE >= 1999
 #define isblank(__c) \
   __extension__ ({ __typeof__ (__c) __x = (__c);		\
-        (__ctype_lookup(__x)&_B) || (int) (__x) == '\t';})
+        (__ctype_lookup(__x)&_ISblank) || (int) (__x) == '\t';})
 #endif
 
 #if __POSIX_VISIBLE >= 200809
@@ -120,22 +133,22 @@ __locale_ctype_ptr_l(locale_t _l)
 #endif
 #define __ctype_lookup_l(__c,__l) ((__locale_ctype_ptr_l(__l)+sizeof(""[__c]))[(int)(__c)])
 
-#define	isalpha_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_U|_L))
-#define	isupper_l(__c,__l)	((__ctype_lookup_l(__c,__l)&(_U|_L))==_U)
-#define	islower_l(__c,__l)	((__ctype_lookup_l(__c,__l)&(_U|_L))==_L)
-#define	isdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_N)
-#define	isxdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_X|_N))
-#define	isspace_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_S)
-#define ispunct_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_P)
-#define isalnum_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_U|_L|_N))
-#define isprint_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_P|_U|_L|_N|_B))
-#define	isgraph_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_P|_U|_L|_N))
-#define iscntrl_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_C)
+#define	isalpha_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower))
+#define	isupper_l(__c,__l)	((__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower)) == _ISupper)
+#define	islower_l(__c,__l)	((__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower)) == _ISlower)
+#define	isdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISdigit)
+#define	isxdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISxdigit|_ISdigit))
+#define	isspace_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISspace)
+#define ispunct_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISpunct)
+#define isalnum_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower|_ISdigit))
+#define isprint_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank))
+#define	isgraph_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISpunct|_ISupper|_ISlower|_ISdigit))
+#define iscntrl_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _IScntrl)
 
 #if defined(__GNUC__)
 #define isblank_l(__c, __l) \
   __extension__ ({ __typeof__ (__c) __x = (__c);		\
-        (__ctype_lookup_l(__x,__l)&_B) || (int) (__x) == '\t';})
+        (__ctype_lookup_l(__x,__l)&_ISblank) || (int) (__x) == '\t';})
 #endif
 
 #endif /* __POSIX_VISIBLE >= 200809 */
-- 
2.33.0

Re: [PATCH 2/2] ctype: use less short names in public header

[Corinna Vinschen]

On Nov  8 20:24, Mike Frysinger wrote:
> We're seeing a build failure in GNU sim code which is using _P locally
> but the ctype.h define clashes with it.  Rename these to use the same
> symbols that glibc does.  They're a bit more verbose, but seems likely
> that we'll have fewer conflicts if glibc isn't seeing them.
                                     ^^^^^
Mixing newlib and glibc?  That's just a typo, I guess?

> However, these shortnames are still used internally by ctype modules
> to produce pretty concise source code, so use _COMPILING_NEWLIB to
> keep them around when compiling newlib itself where we have better
> control over short name conflicts.

Hmm.  I'm not sure we should really maintain two different sets of
symbols.  I think it would be better to go the entire way and replace
the single letter symbols with the new, more speaking ones throughout.
There are not that many affected files and the change might be done with
sed mostly.

The only exceptions *could* be libc/ctype/ctype_.c and the local
ctype_iso.h and ctype_cp.h headers it includes.  A local definition
of the single letter symbols in ctype_.c would be sufficient then.
But even there we might be better off with the new symbols in the long
run...


Corinna

Re: [PATCH 2/2] ctype: use less short names in public header

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1826 bytes --]

On 09 Nov 2021 12:38, Corinna Vinschen wrote:
> On Nov  8 20:24, Mike Frysinger wrote:
> > We're seeing a build failure in GNU sim code which is using _P locally
> > but the ctype.h define clashes with it.  Rename these to use the same
> > symbols that glibc does.  They're a bit more verbose, but seems likely
> > that we'll have fewer conflicts if glibc isn't seeing them.
>                                      ^^^^^
> Mixing newlib and glibc?  That's just a typo, I guess?

i meant glibc here.  these are the same symbol names that glibc is using,
and it's not seeing conflicts in the wider ecosystem.  so if they aren't
seeing issues, it's likely newlib won't either if it uses the same names.

> > However, these shortnames are still used internally by ctype modules
> > to produce pretty concise source code, so use _COMPILING_NEWLIB to
> > keep them around when compiling newlib itself where we have better
> > control over short name conflicts.
> 
> Hmm.  I'm not sure we should really maintain two different sets of
> symbols.  I think it would be better to go the entire way and replace
> the single letter symbols with the new, more speaking ones throughout.
> There are not that many affected files and the change might be done with
> sed mostly.
> 
> The only exceptions *could* be libc/ctype/ctype_.c and the local
> ctype_iso.h and ctype_cp.h headers it includes.  A local definition
> of the single letter symbols in ctype_.c would be sufficient then.
> But even there we might be better off with the new symbols in the long
> run...

i don't have an opinion.  i can easily run sed on the files and send the
result out.  i figured people would prefer having the condense tables so
they could scan them quicker by eye.

so let me know what you want and i'll do it ;).
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] ctype: use less short names in public header

[Corinna Vinschen]

On Nov  9 19:18, Mike Frysinger wrote:
> On 09 Nov 2021 12:38, Corinna Vinschen wrote:
> > On Nov  8 20:24, Mike Frysinger wrote:
> > > We're seeing a build failure in GNU sim code which is using _P locally
> > > but the ctype.h define clashes with it.  Rename these to use the same
> > > symbols that glibc does.  They're a bit more verbose, but seems likely
> > > that we'll have fewer conflicts if glibc isn't seeing them.
> >                                      ^^^^^
> > Mixing newlib and glibc?  That's just a typo, I guess?
> 
> i meant glibc here.  these are the same symbol names that glibc is using,
> and it's not seeing conflicts in the wider ecosystem.  so if they aren't
> seeing issues, it's likely newlib won't either if it uses the same names.
> 
> > > However, these shortnames are still used internally by ctype modules
> > > to produce pretty concise source code, so use _COMPILING_NEWLIB to
> > > keep them around when compiling newlib itself where we have better
> > > control over short name conflicts.
> > 
> > Hmm.  I'm not sure we should really maintain two different sets of
> > symbols.  I think it would be better to go the entire way and replace
> > the single letter symbols with the new, more speaking ones throughout.
> > There are not that many affected files and the change might be done with
> > sed mostly.
> > 
> > The only exceptions *could* be libc/ctype/ctype_.c and the local
> > ctype_iso.h and ctype_cp.h headers it includes.  A local definition
> > of the single letter symbols in ctype_.c would be sufficient then.
> > But even there we might be better off with the new symbols in the long
> > run...
> 
> i don't have an opinion.  i can easily run sed on the files and send the
> result out.  i figured people would prefer having the condense tables so
> they could scan them quicker by eye.
> 
> so let me know what you want and i'll do it ;).

Great.  Let's try the compromise from above: Drop _X etc from the
header, just define them in libc/ctype/ctype_.c.


Thanks,
Corinna

[PATCH v2] ctype: use less short names in public header

[Mike Frysinger]

We're seeing a build failure in GNU sim code which is using _P locally
but the ctype.h define clashes with it.  Rename these to use the same
symbols that glibc does.  They're a bit more verbose, but seems likely
that we'll have fewer conflicts if glibc isn't seeing them.

However, these shortnames are still used internally by ctype modules
to produce pretty concise source code, so move the short names to the
internal ctype_.h where short name conflicts shouldn't show up.
---
 newlib/libc/ctype/ctype_.h     | 10 +++++
 newlib/libc/ctype/isalnum.c    |  2 +-
 newlib/libc/ctype/isalnum_l.c  |  2 +-
 newlib/libc/ctype/isalpha.c    |  2 +-
 newlib/libc/ctype/isalpha_l.c  |  2 +-
 newlib/libc/ctype/isblank.c    |  2 +-
 newlib/libc/ctype/isblank_l.c  |  2 +-
 newlib/libc/ctype/iscntrl.c    |  2 +-
 newlib/libc/ctype/iscntrl_l.c  |  2 +-
 newlib/libc/ctype/isdigit.c    |  2 +-
 newlib/libc/ctype/isdigit_l.c  |  2 +-
 newlib/libc/ctype/islower.c    |  2 +-
 newlib/libc/ctype/islower_l.c  |  2 +-
 newlib/libc/ctype/isprint.c    |  4 +-
 newlib/libc/ctype/isprint_l.c  |  4 +-
 newlib/libc/ctype/ispunct.c    |  2 +-
 newlib/libc/ctype/ispunct_l.c  |  2 +-
 newlib/libc/ctype/isspace.c    |  2 +-
 newlib/libc/ctype/isspace_l.c  |  2 +-
 newlib/libc/ctype/isupper.c    |  2 +-
 newlib/libc/ctype/isupper_l.c  |  2 +-
 newlib/libc/ctype/isxdigit.c   |  2 +-
 newlib/libc/ctype/isxdigit_l.c |  2 +-
 newlib/libc/include/ctype.h    | 67 ++++++++++++++++++----------------
 24 files changed, 69 insertions(+), 56 deletions(-)

diff --git a/newlib/libc/ctype/ctype_.h b/newlib/libc/ctype/ctype_.h
index a73870b3e4ad..42ad2c87024d 100644
--- a/newlib/libc/ctype/ctype_.h
+++ b/newlib/libc/ctype/ctype_.h
@@ -1,5 +1,15 @@
 #include <ctype.h>
 
+/* Define some short names to keep internal files shorter.  */
+#define _U _ISupper
+#define _L _ISlower
+#define _N _ISdigit
+#define _S _ISspace
+#define _P _ISpunct
+#define _C _IScntrl
+#define _X _ISxdigit
+#define _B _ISblank
+
 #if (defined(__GNUC__) && !defined(__CHAR_UNSIGNED__) && !defined(COMPACT_CTYPE)) || defined (__CYGWIN__)
 #define ALLOW_NEGATIVE_CTYPE_INDEX
 #endif
diff --git a/newlib/libc/ctype/isalnum.c b/newlib/libc/ctype/isalnum.c
index d926f97b7c3f..3ddf0d2e1f18 100644
--- a/newlib/libc/ctype/isalnum.c
+++ b/newlib/libc/ctype/isalnum.c
@@ -46,5 +46,5 @@ No OS subroutines are required.
 int
 isalnum (int c)
 {
-	return(__CTYPE_PTR[c+1] & (_U|_L|_N));
+	return(__CTYPE_PTR[c+1] & (_ISupper|_ISlower|_ISdigit));
 }
diff --git a/newlib/libc/ctype/isalnum_l.c b/newlib/libc/ctype/isalnum_l.c
index dcb7e3652313..e3587f1b2995 100644
--- a/newlib/libc/ctype/isalnum_l.c
+++ b/newlib/libc/ctype/isalnum_l.c
@@ -6,5 +6,5 @@
 int
 isalnum_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & (_U|_L|_N);
+  return __locale_ctype_ptr_l (locale)[c+1] & (_ISupper|_ISlower|_ISdigit);
 }
diff --git a/newlib/libc/ctype/isalpha.c b/newlib/libc/ctype/isalpha.c
index 8b8e78a29635..c8de62cea3c2 100644
--- a/newlib/libc/ctype/isalpha.c
+++ b/newlib/libc/ctype/isalpha.c
@@ -45,5 +45,5 @@ No supporting OS subroutines are required.
 int
 isalpha (int c)
 {
-	return(__CTYPE_PTR[c+1] & (_U|_L));
+	return(__CTYPE_PTR[c+1] & (_ISupper|_ISlower));
 }
diff --git a/newlib/libc/ctype/isalpha_l.c b/newlib/libc/ctype/isalpha_l.c
index dcae3ccb4408..6ff1e3b686ac 100644
--- a/newlib/libc/ctype/isalpha_l.c
+++ b/newlib/libc/ctype/isalpha_l.c
@@ -6,5 +6,5 @@
 int
 isalpha_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & (_U|_L);
+  return __locale_ctype_ptr_l (locale)[c+1] & (_ISupper|_ISlower);
 }
diff --git a/newlib/libc/ctype/isblank.c b/newlib/libc/ctype/isblank.c
index 0ebc2192c5c9..2fa29f33fb6f 100644
--- a/newlib/libc/ctype/isblank.c
+++ b/newlib/libc/ctype/isblank.c
@@ -44,5 +44,5 @@ No supporting OS subroutines are required.
 int
 isblank (int c)
 {
-	return ((__CTYPE_PTR[c+1] & _B) || (c == '\t'));
+	return ((__CTYPE_PTR[c+1] & _ISblank) || (c == '\t'));
 }
diff --git a/newlib/libc/ctype/isblank_l.c b/newlib/libc/ctype/isblank_l.c
index 8bbb84e1f6a8..6aff2f7fc354 100644
--- a/newlib/libc/ctype/isblank_l.c
+++ b/newlib/libc/ctype/isblank_l.c
@@ -6,5 +6,5 @@
 int
 isblank_l (int c, struct __locale_t *locale)
 {
-  return (__locale_ctype_ptr_l (locale)[c+1] & _B) || (c == '\t');
+  return (__locale_ctype_ptr_l (locale)[c+1] & _ISblank) || (c == '\t');
 }
diff --git a/newlib/libc/ctype/iscntrl.c b/newlib/libc/ctype/iscntrl.c
index ebbdd7371d73..bd99ebafb511 100644
--- a/newlib/libc/ctype/iscntrl.c
+++ b/newlib/libc/ctype/iscntrl.c
@@ -48,5 +48,5 @@ No supporting OS subroutines are required.
 int
 iscntrl (int c)
 {
-	return(__CTYPE_PTR[c+1] & _C);
+	return(__CTYPE_PTR[c+1] & _IScntrl);
 }
diff --git a/newlib/libc/ctype/iscntrl_l.c b/newlib/libc/ctype/iscntrl_l.c
index 0ae17c7f7414..85f4d6900022 100644
--- a/newlib/libc/ctype/iscntrl_l.c
+++ b/newlib/libc/ctype/iscntrl_l.c
@@ -6,5 +6,5 @@
 int
 iscntrl_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & _C;
+  return __locale_ctype_ptr_l (locale)[c+1] & _IScntrl;
 }
diff --git a/newlib/libc/ctype/isdigit.c b/newlib/libc/ctype/isdigit.c
index a5c511964f21..911781df3c5a 100644
--- a/newlib/libc/ctype/isdigit.c
+++ b/newlib/libc/ctype/isdigit.c
@@ -47,5 +47,5 @@ No supporting OS subroutines are required.
 int
 isdigit (int c)
 {
-	return(__CTYPE_PTR[c+1] & _N);
+	return(__CTYPE_PTR[c+1] & _ISdigit);
 }
diff --git a/newlib/libc/ctype/isdigit_l.c b/newlib/libc/ctype/isdigit_l.c
index 1fb79e0001a0..6e24c100ce2e 100644
--- a/newlib/libc/ctype/isdigit_l.c
+++ b/newlib/libc/ctype/isdigit_l.c
@@ -6,5 +6,5 @@
 int
 isdigit_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & _N;
+  return __locale_ctype_ptr_l (locale)[c+1] & _ISdigit;
 }
diff --git a/newlib/libc/ctype/islower.c b/newlib/libc/ctype/islower.c
index 2b344048957a..c45c6c8ad6f5 100644
--- a/newlib/libc/ctype/islower.c
+++ b/newlib/libc/ctype/islower.c
@@ -45,5 +45,5 @@ No supporting OS subroutines are required.
 int
 islower (int c)
 {
-	return ((__CTYPE_PTR[c+1] & (_U|_L)) == _L);
+	return ((__CTYPE_PTR[c+1] & (_ISupper|_ISlower)) == _ISlower);
 }
diff --git a/newlib/libc/ctype/islower_l.c b/newlib/libc/ctype/islower_l.c
index d1f3a82d8239..1c66ab48b931 100644
--- a/newlib/libc/ctype/islower_l.c
+++ b/newlib/libc/ctype/islower_l.c
@@ -6,5 +6,5 @@
 int
 islower_l (int c, struct __locale_t *locale)
 {
-  return (__locale_ctype_ptr_l (locale)[c+1] & (_U|_L)) == _L;
+  return (__locale_ctype_ptr_l (locale)[c+1] & (_ISupper|_ISlower)) == _ISlower;
 }
diff --git a/newlib/libc/ctype/isprint.c b/newlib/libc/ctype/isprint.c
index e34fbe28a2ac..7206047fb36b 100644
--- a/newlib/libc/ctype/isprint.c
+++ b/newlib/libc/ctype/isprint.c
@@ -59,7 +59,7 @@ No supporting OS subroutines are required.
 int
 isgraph (int c)
 {
-	return(__CTYPE_PTR[c+1] & (_P|_U|_L|_N));
+	return(__CTYPE_PTR[c+1] & (_ISpunct|_ISupper|_ISlower|_ISdigit));
 }
 
 
@@ -67,5 +67,5 @@ isgraph (int c)
 int
 isprint (int c)
 {
-	return(__CTYPE_PTR[c+1] & (_P|_U|_L|_N|_B));
+	return(__CTYPE_PTR[c+1] & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank));
 }
diff --git a/newlib/libc/ctype/isprint_l.c b/newlib/libc/ctype/isprint_l.c
index 535504f1406f..3efaa2f90e01 100644
--- a/newlib/libc/ctype/isprint_l.c
+++ b/newlib/libc/ctype/isprint_l.c
@@ -6,7 +6,7 @@
 int
 isgraph_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & (_P|_U|_L|_N);
+  return __locale_ctype_ptr_l (locale)[c+1] & (_ISpunct|_ISupper|_ISlower|_ISdigit);
 }
 
 #undef isprint_l
@@ -14,5 +14,5 @@ isgraph_l (int c, struct __locale_t *locale)
 int
 isprint_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & (_P|_U|_L|_N|_B);
+  return __locale_ctype_ptr_l (locale)[c+1] & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank);
 }
diff --git a/newlib/libc/ctype/ispunct.c b/newlib/libc/ctype/ispunct.c
index 9c5a3fcca762..35f7cc2d421b 100644
--- a/newlib/libc/ctype/ispunct.c
+++ b/newlib/libc/ctype/ispunct.c
@@ -47,5 +47,5 @@ No supporting OS subroutines are required.
 int
 ispunct (int c)
 {
-	return(__CTYPE_PTR[c+1] & _P);
+	return(__CTYPE_PTR[c+1] & _ISpunct);
 }
diff --git a/newlib/libc/ctype/ispunct_l.c b/newlib/libc/ctype/ispunct_l.c
index eeba1f5ae110..30c2b48d65ca 100644
--- a/newlib/libc/ctype/ispunct_l.c
+++ b/newlib/libc/ctype/ispunct_l.c
@@ -6,6 +6,6 @@
 int
 ispunct_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & _P;
+  return __locale_ctype_ptr_l (locale)[c+1] & _ISpunct;
 }
 
diff --git a/newlib/libc/ctype/isspace.c b/newlib/libc/ctype/isspace.c
index 0def2c0ce47f..8834d1901c6e 100644
--- a/newlib/libc/ctype/isspace.c
+++ b/newlib/libc/ctype/isspace.c
@@ -46,5 +46,5 @@ No supporting OS subroutines are required.
 int
 isspace (int c)
 {
-	return(__CTYPE_PTR[c+1] & _S);
+	return(__CTYPE_PTR[c+1] & _ISspace);
 }
diff --git a/newlib/libc/ctype/isspace_l.c b/newlib/libc/ctype/isspace_l.c
index bf4a36c3e90a..06227c87a052 100644
--- a/newlib/libc/ctype/isspace_l.c
+++ b/newlib/libc/ctype/isspace_l.c
@@ -6,6 +6,6 @@
 int
 isspace_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & _S;
+  return __locale_ctype_ptr_l (locale)[c+1] & _ISspace;
 }
 
diff --git a/newlib/libc/ctype/isupper.c b/newlib/libc/ctype/isupper.c
index aeed383ecdea..7d916d9f3c49 100644
--- a/newlib/libc/ctype/isupper.c
+++ b/newlib/libc/ctype/isupper.c
@@ -43,5 +43,5 @@ No supporting OS subroutines are required.
 int
 isupper (int c)
 {
-	return ((__CTYPE_PTR[c+1] & (_U|_L)) == _U);
+	return ((__CTYPE_PTR[c+1] & (_ISupper|_ISlower)) == _ISupper);
 }
diff --git a/newlib/libc/ctype/isupper_l.c b/newlib/libc/ctype/isupper_l.c
index eb473a7a1d54..1fdcf107277e 100644
--- a/newlib/libc/ctype/isupper_l.c
+++ b/newlib/libc/ctype/isupper_l.c
@@ -6,6 +6,6 @@
 int
 isupper_l (int c, struct __locale_t *locale)
 {
-  return (__locale_ctype_ptr_l (locale)[c+1] & (_U|_L)) == _U;
+  return (__locale_ctype_ptr_l (locale)[c+1] & (_ISupper|_ISlower)) == _ISupper;
 }
 
diff --git a/newlib/libc/ctype/isxdigit.c b/newlib/libc/ctype/isxdigit.c
index 2bfe18dbfab7..fb2a59ea801a 100644
--- a/newlib/libc/ctype/isxdigit.c
+++ b/newlib/libc/ctype/isxdigit.c
@@ -46,5 +46,5 @@ No supporting OS subroutines are required.
 int
 isxdigit (int c)
 {
-	return(__CTYPE_PTR[c+1] & ((_X)|(_N)));
+	return(__CTYPE_PTR[c+1] & ((_ISxdigit)|(_ISdigit)));
 }
diff --git a/newlib/libc/ctype/isxdigit_l.c b/newlib/libc/ctype/isxdigit_l.c
index 726db31903fb..bbae410c6cd8 100644
--- a/newlib/libc/ctype/isxdigit_l.c
+++ b/newlib/libc/ctype/isxdigit_l.c
@@ -6,6 +6,6 @@
 int
 isxdigit_l (int c, struct __locale_t *locale)
 {
-  return __locale_ctype_ptr_l (locale)[c+1] & ((_X)|(_N));
+  return __locale_ctype_ptr_l (locale)[c+1] & ((_ISxdigit)|(_ISdigit));
 }
 
diff --git a/newlib/libc/include/ctype.h b/newlib/libc/include/ctype.h
index 932a567e25db..366b35dc9f9d 100644
--- a/newlib/libc/include/ctype.h
+++ b/newlib/libc/include/ctype.h
@@ -57,14 +57,17 @@ extern int isascii_l (int __c, locale_t __l);
 extern int toascii_l (int __c, locale_t __l);
 #endif
 
-#define	_U	01
-#define	_L	02
-#define	_N	04
-#define	_S	010
-#define _P	020
-#define _C	040
-#define _X	0100
-#define	_B	0200
+enum
+{
+  _ISupper = 01,
+  _ISlower = 02,
+  _ISdigit = 04,
+  _ISspace = 010,
+  _ISpunct = 020,
+  _IScntrl = 040,
+  _ISxdigit = 0100,
+  _ISblank = 0200,
+};
 
 /* For C++ backward-compatibility only.  */
 extern	__IMPORT const char	_ctype_[];
@@ -89,22 +92,22 @@ const char *__locale_ctype_ptr (void);
    an out-of-bounds reference on a 64-bit machine.  */
 #define __ctype_lookup(__c) ((__CTYPE_PTR+sizeof(""[__c]))[(int)(__c)])
 
-#define	isalpha(__c)	(__ctype_lookup(__c)&(_U|_L))
-#define	isupper(__c)	((__ctype_lookup(__c)&(_U|_L))==_U)
-#define	islower(__c)	((__ctype_lookup(__c)&(_U|_L))==_L)
-#define	isdigit(__c)	(__ctype_lookup(__c)&_N)
-#define	isxdigit(__c)	(__ctype_lookup(__c)&(_X|_N))
-#define	isspace(__c)	(__ctype_lookup(__c)&_S)
-#define ispunct(__c)	(__ctype_lookup(__c)&_P)
-#define isalnum(__c)	(__ctype_lookup(__c)&(_U|_L|_N))
-#define isprint(__c)	(__ctype_lookup(__c)&(_P|_U|_L|_N|_B))
-#define	isgraph(__c)	(__ctype_lookup(__c)&(_P|_U|_L|_N))
-#define iscntrl(__c)	(__ctype_lookup(__c)&_C)
+#define	isalpha(__c)	(__ctype_lookup(__c) & (_ISupper|_ISlower))
+#define	isupper(__c)	((__ctype_lookup(__c) & (_ISupper|_ISlower)) == _ISupper)
+#define	islower(__c)	((__ctype_lookup(__c) & (_ISupper|_ISlower)) == _ISlower)
+#define	isdigit(__c)	(__ctype_lookup(__c) & _ISdigit)
+#define	isxdigit(__c)	(__ctype_lookup(__c) & (_ISxdigit|_ISdigit))
+#define	isspace(__c)	(__ctype_lookup(__c) & _ISspace)
+#define ispunct(__c)	(__ctype_lookup(__c) & _ISpunct)
+#define isalnum(__c)	(__ctype_lookup(__c) & (_ISupper|_ISlower|_ISdigit))
+#define isprint(__c)	(__ctype_lookup(__c) & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank))
+#define	isgraph(__c)	(__ctype_lookup(__c) & (_ISpunct|_ISupper|_ISlower|_ISdigit))
+#define iscntrl(__c)	(__ctype_lookup(__c) & _IScntrl)
 
 #if defined(__GNUC__) && __ISO_C_VISIBLE >= 1999
 #define isblank(__c) \
   __extension__ ({ __typeof__ (__c) __x = (__c);		\
-        (__ctype_lookup(__x)&_B) || (int) (__x) == '\t';})
+        (__ctype_lookup(__x)&_ISblank) || (int) (__x) == '\t';})
 #endif
 
 #if __POSIX_VISIBLE >= 200809
@@ -120,22 +123,22 @@ __locale_ctype_ptr_l(locale_t _l)
 #endif
 #define __ctype_lookup_l(__c,__l) ((__locale_ctype_ptr_l(__l)+sizeof(""[__c]))[(int)(__c)])
 
-#define	isalpha_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_U|_L))
-#define	isupper_l(__c,__l)	((__ctype_lookup_l(__c,__l)&(_U|_L))==_U)
-#define	islower_l(__c,__l)	((__ctype_lookup_l(__c,__l)&(_U|_L))==_L)
-#define	isdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_N)
-#define	isxdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_X|_N))
-#define	isspace_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_S)
-#define ispunct_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_P)
-#define isalnum_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_U|_L|_N))
-#define isprint_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_P|_U|_L|_N|_B))
-#define	isgraph_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_P|_U|_L|_N))
-#define iscntrl_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_C)
+#define	isalpha_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower))
+#define	isupper_l(__c,__l)	((__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower)) == _ISupper)
+#define	islower_l(__c,__l)	((__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower)) == _ISlower)
+#define	isdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISdigit)
+#define	isxdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISxdigit|_ISdigit))
+#define	isspace_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISspace)
+#define ispunct_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISpunct)
+#define isalnum_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower|_ISdigit))
+#define isprint_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank))
+#define	isgraph_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISpunct|_ISupper|_ISlower|_ISdigit))
+#define iscntrl_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _IScntrl)
 
 #if defined(__GNUC__)
 #define isblank_l(__c, __l) \
   __extension__ ({ __typeof__ (__c) __x = (__c);		\
-        (__ctype_lookup_l(__x,__l)&_B) || (int) (__x) == '\t';})
+        (__ctype_lookup_l(__x,__l)&_ISblank) || (int) (__x) == '\t';})
 #endif
 
 #endif /* __POSIX_VISIBLE >= 200809 */
-- 
2.33.0

Re: [PATCH v2] ctype: use less short names in public header

[Corinna Vinschen]

On Nov 10 20:37, Mike Frysinger wrote:
> We're seeing a build failure in GNU sim code which is using _P locally
> but the ctype.h define clashes with it.  Rename these to use the same
> symbols that glibc does.  They're a bit more verbose, but seems likely
> that we'll have fewer conflicts if glibc isn't seeing them.
> 
> However, these shortnames are still used internally by ctype modules
> to produce pretty concise source code, so move the short names to the
> internal ctype_.h where short name conflicts shouldn't show up.
> ---
>  newlib/libc/ctype/ctype_.h     | 10 +++++
>  newlib/libc/ctype/isalnum.c    |  2 +-
>  newlib/libc/ctype/isalnum_l.c  |  2 +-
>  newlib/libc/ctype/isalpha.c    |  2 +-
>  newlib/libc/ctype/isalpha_l.c  |  2 +-
>  newlib/libc/ctype/isblank.c    |  2 +-
>  newlib/libc/ctype/isblank_l.c  |  2 +-
>  newlib/libc/ctype/iscntrl.c    |  2 +-
>  newlib/libc/ctype/iscntrl_l.c  |  2 +-
>  newlib/libc/ctype/isdigit.c    |  2 +-
>  newlib/libc/ctype/isdigit_l.c  |  2 +-
>  newlib/libc/ctype/islower.c    |  2 +-
>  newlib/libc/ctype/islower_l.c  |  2 +-
>  newlib/libc/ctype/isprint.c    |  4 +-
>  newlib/libc/ctype/isprint_l.c  |  4 +-
>  newlib/libc/ctype/ispunct.c    |  2 +-
>  newlib/libc/ctype/ispunct_l.c  |  2 +-
>  newlib/libc/ctype/isspace.c    |  2 +-
>  newlib/libc/ctype/isspace_l.c  |  2 +-
>  newlib/libc/ctype/isupper.c    |  2 +-
>  newlib/libc/ctype/isupper_l.c  |  2 +-
>  newlib/libc/ctype/isxdigit.c   |  2 +-
>  newlib/libc/ctype/isxdigit_l.c |  2 +-
>  newlib/libc/include/ctype.h    | 67 ++++++++++++++++++----------------
>  24 files changed, 69 insertions(+), 56 deletions(-)

Good idea to move the _X macros to ctype_.h :)  Please push.


Thanks,
Corinna

Re: [PATCH v2] ctype: use less short names in public header

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2035 bytes --]

On 11 Nov 2021 11:35, Corinna Vinschen wrote:
> On Nov 10 20:37, Mike Frysinger wrote:
> > We're seeing a build failure in GNU sim code which is using _P locally
> > but the ctype.h define clashes with it.  Rename these to use the same
> > symbols that glibc does.  They're a bit more verbose, but seems likely
> > that we'll have fewer conflicts if glibc isn't seeing them.
> > 
> > However, these shortnames are still used internally by ctype modules
> > to produce pretty concise source code, so move the short names to the
> > internal ctype_.h where short name conflicts shouldn't show up.
> > ---
> >  newlib/libc/ctype/ctype_.h     | 10 +++++
> >  newlib/libc/ctype/isalnum.c    |  2 +-
> >  newlib/libc/ctype/isalnum_l.c  |  2 +-
> >  newlib/libc/ctype/isalpha.c    |  2 +-
> >  newlib/libc/ctype/isalpha_l.c  |  2 +-
> >  newlib/libc/ctype/isblank.c    |  2 +-
> >  newlib/libc/ctype/isblank_l.c  |  2 +-
> >  newlib/libc/ctype/iscntrl.c    |  2 +-
> >  newlib/libc/ctype/iscntrl_l.c  |  2 +-
> >  newlib/libc/ctype/isdigit.c    |  2 +-
> >  newlib/libc/ctype/isdigit_l.c  |  2 +-
> >  newlib/libc/ctype/islower.c    |  2 +-
> >  newlib/libc/ctype/islower_l.c  |  2 +-
> >  newlib/libc/ctype/isprint.c    |  4 +-
> >  newlib/libc/ctype/isprint_l.c  |  4 +-
> >  newlib/libc/ctype/ispunct.c    |  2 +-
> >  newlib/libc/ctype/ispunct_l.c  |  2 +-
> >  newlib/libc/ctype/isspace.c    |  2 +-
> >  newlib/libc/ctype/isspace_l.c  |  2 +-
> >  newlib/libc/ctype/isupper.c    |  2 +-
> >  newlib/libc/ctype/isupper_l.c  |  2 +-
> >  newlib/libc/ctype/isxdigit.c   |  2 +-
> >  newlib/libc/ctype/isxdigit_l.c |  2 +-
> >  newlib/libc/include/ctype.h    | 67 ++++++++++++++++++----------------
> >  24 files changed, 69 insertions(+), 56 deletions(-)
> 
> Good idea to move the _X macros to ctype_.h :)  Please push.

i pushed this since it's standalone now.  not sure if you're also saying
"define _COMPILING_NEWLIB for all targets when compiling" is OK, so i haven't
pushed that yet.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] ctype: use less short names in public header

[Corinna Vinschen]

On Nov 11 17:28, Mike Frysinger wrote:
> On 11 Nov 2021 11:35, Corinna Vinschen wrote:
> > On Nov 10 20:37, Mike Frysinger wrote:
> > > We're seeing a build failure in GNU sim code which is using _P locally
> > > but the ctype.h define clashes with it.  Rename these to use the same
> > > symbols that glibc does.  They're a bit more verbose, but seems likely
> > > that we'll have fewer conflicts if glibc isn't seeing them.
> > > 
> > > However, these shortnames are still used internally by ctype modules
> > > to produce pretty concise source code, so move the short names to the
> > > internal ctype_.h where short name conflicts shouldn't show up.
> > > ---
> > >  newlib/libc/ctype/ctype_.h     | 10 +++++
> > >  newlib/libc/ctype/isalnum.c    |  2 +-
> > >  newlib/libc/ctype/isalnum_l.c  |  2 +-
> > >  newlib/libc/ctype/isalpha.c    |  2 +-
> > >  newlib/libc/ctype/isalpha_l.c  |  2 +-
> > >  newlib/libc/ctype/isblank.c    |  2 +-
> > >  newlib/libc/ctype/isblank_l.c  |  2 +-
> > >  newlib/libc/ctype/iscntrl.c    |  2 +-
> > >  newlib/libc/ctype/iscntrl_l.c  |  2 +-
> > >  newlib/libc/ctype/isdigit.c    |  2 +-
> > >  newlib/libc/ctype/isdigit_l.c  |  2 +-
> > >  newlib/libc/ctype/islower.c    |  2 +-
> > >  newlib/libc/ctype/islower_l.c  |  2 +-
> > >  newlib/libc/ctype/isprint.c    |  4 +-
> > >  newlib/libc/ctype/isprint_l.c  |  4 +-
> > >  newlib/libc/ctype/ispunct.c    |  2 +-
> > >  newlib/libc/ctype/ispunct_l.c  |  2 +-
> > >  newlib/libc/ctype/isspace.c    |  2 +-
> > >  newlib/libc/ctype/isspace_l.c  |  2 +-
> > >  newlib/libc/ctype/isupper.c    |  2 +-
> > >  newlib/libc/ctype/isupper_l.c  |  2 +-
> > >  newlib/libc/ctype/isxdigit.c   |  2 +-
> > >  newlib/libc/ctype/isxdigit_l.c |  2 +-
> > >  newlib/libc/include/ctype.h    | 67 ++++++++++++++++++----------------
> > >  24 files changed, 69 insertions(+), 56 deletions(-)
> > 
> > Good idea to move the _X macros to ctype_.h :)  Please push.
> 
> i pushed this since it's standalone now.  not sure if you're also saying
> "define _COMPILING_NEWLIB for all targets when compiling" is OK, so i haven't
> pushed that yet.

Oh, that was implicitly clear to me, given the original dependency.
So, yeah, please push.


Thanks,
Corinna

Re: [PATCH v2] ctype: use less short names in public header

[Brian Inglis]

On 2021-11-12 03:27, Corinna Vinschen wrote:
> On Nov 11 17:28, Mike Frysinger wrote:
>> On 11 Nov 2021 11:35, Corinna Vinschen wrote:
>>> On Nov 10 20:37, Mike Frysinger wrote:
>>>> We're seeing a build failure in GNU sim code which is using _P locally
>>>> but the ctype.h define clashes with it.  Rename these to use the same
>>>> symbols that glibc does.  They're a bit more verbose, but seems likely
>>>> that we'll have fewer conflicts if glibc isn't seeing them.
>>>>
>>>> However, these shortnames are still used internally by ctype modules
>>>> to produce pretty concise source code, so move the short names to the
>>>> internal ctype_.h where short name conflicts shouldn't show up.
>>>> ---
>>>>   newlib/libc/ctype/ctype_.h     | 10 +++++
>>>>   newlib/libc/ctype/isalnum.c    |  2 +-
>>>>   newlib/libc/ctype/isalnum_l.c  |  2 +-
>>>>   newlib/libc/ctype/isalpha.c    |  2 +-
>>>>   newlib/libc/ctype/isalpha_l.c  |  2 +-
>>>>   newlib/libc/ctype/isblank.c    |  2 +-
>>>>   newlib/libc/ctype/isblank_l.c  |  2 +-
>>>>   newlib/libc/ctype/iscntrl.c    |  2 +-
>>>>   newlib/libc/ctype/iscntrl_l.c  |  2 +-
>>>>   newlib/libc/ctype/isdigit.c    |  2 +-
>>>>   newlib/libc/ctype/isdigit_l.c  |  2 +-
>>>>   newlib/libc/ctype/islower.c    |  2 +-
>>>>   newlib/libc/ctype/islower_l.c  |  2 +-
>>>>   newlib/libc/ctype/isprint.c    |  4 +-
>>>>   newlib/libc/ctype/isprint_l.c  |  4 +-
>>>>   newlib/libc/ctype/ispunct.c    |  2 +-
>>>>   newlib/libc/ctype/ispunct_l.c  |  2 +-
>>>>   newlib/libc/ctype/isspace.c    |  2 +-
>>>>   newlib/libc/ctype/isspace_l.c  |  2 +-
>>>>   newlib/libc/ctype/isupper.c    |  2 +-
>>>>   newlib/libc/ctype/isupper_l.c  |  2 +-
>>>>   newlib/libc/ctype/isxdigit.c   |  2 +-
>>>>   newlib/libc/ctype/isxdigit_l.c |  2 +-
>>>>   newlib/libc/include/ctype.h    | 67 ++++++++++++++++++----------------
>>>>   24 files changed, 69 insertions(+), 56 deletions(-)
>>>
>>> Good idea to move the _X macros to ctype_.h :)  Please push.
>>
>> i pushed this since it's standalone now.  not sure if you're also saying
>> "define _COMPILING_NEWLIB for all targets when compiling" is OK, so i haven't
>> pushed that yet.
> 
> Oh, that was implicitly clear to me, given the original dependency.
> So, yeah, please push.

+1
I've got a package build *NOT* failing with a redefinition of a couple 
of those short symbols to macros!

I will remind the author of the reserved symbols issue, and submit an 
upstream patch.

I will also report not failing as an upstream gcc error, as I don't 
think the compiler should just warn about those types of redefinitions.

But it would be good to see those short symbols lengthened and hidden 
from normal usage, as I have also seen those kinds of short symbols _? 
used as parameter and argument names in various sources.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

Re: [PATCH 2/2] ctype: use less short names in public header

[Richard Earnshaw]

This is wrong and breaks all old versions of C++.

The GNU sim code should not be using reserved names (those starting _) 
in normal source code.  Such names are reserved to the implementation.

R.

On 09/11/2021 01:24, Mike Frysinger wrote:
> We're seeing a build failure in GNU sim code which is using _P locally
> but the ctype.h define clashes with it.  Rename these to use the same
> symbols that glibc does.  They're a bit more verbose, but seems likely
> that we'll have fewer conflicts if glibc isn't seeing them.
> 
> However, these shortnames are still used internally by ctype modules
> to produce pretty concise source code, so use _COMPILING_NEWLIB to
> keep them around when compiling newlib itself where we have better
> control over short name conflicts.
> ---
>   newlib/libc/include/ctype.h | 77 ++++++++++++++++++++++---------------
>   1 file changed, 45 insertions(+), 32 deletions(-)
> 
> diff --git a/newlib/libc/include/ctype.h b/newlib/libc/include/ctype.h
> index 932a567e25db..f2a4368da5d2 100644
> --- a/newlib/libc/include/ctype.h
> +++ b/newlib/libc/include/ctype.h
> @@ -57,14 +57,27 @@ extern int isascii_l (int __c, locale_t __l);
>   extern int toascii_l (int __c, locale_t __l);
>   #endif
>   
> -#define	_U	01
> -#define	_L	02
> -#define	_N	04
> -#define	_S	010
> -#define _P	020
> -#define _C	040
> -#define _X	0100
> -#define	_B	0200
> +enum
> +{
> +  _ISupper = 01,
> +  _ISlower = 02,
> +  _ISdigit = 04,
> +  _ISspace = 010,
> +  _ISpunct = 020,
> +  _IScntrl = 040,
> +  _ISxdigit = 0100,
> +  _ISblank = 0200,
> +};
> +#ifdef _COMPILING_NEWLIB
> +# define _U _ISupper
> +# define _L _ISlower
> +# define _N _ISdigit
> +# define _S _ISspace
> +# define _P _ISpunct
> +# define _C _IScntrl
> +# define _X _ISxdigit
> +# define _B _ISblank
> +#endif
>   
>   /* For C++ backward-compatibility only.  */
>   extern	__IMPORT const char	_ctype_[];
> @@ -89,22 +102,22 @@ const char *__locale_ctype_ptr (void);
>      an out-of-bounds reference on a 64-bit machine.  */
>   #define __ctype_lookup(__c) ((__CTYPE_PTR+sizeof(""[__c]))[(int)(__c)])
>   
> -#define	isalpha(__c)	(__ctype_lookup(__c)&(_U|_L))
> -#define	isupper(__c)	((__ctype_lookup(__c)&(_U|_L))==_U)
> -#define	islower(__c)	((__ctype_lookup(__c)&(_U|_L))==_L)
> -#define	isdigit(__c)	(__ctype_lookup(__c)&_N)
> -#define	isxdigit(__c)	(__ctype_lookup(__c)&(_X|_N))
> -#define	isspace(__c)	(__ctype_lookup(__c)&_S)
> -#define ispunct(__c)	(__ctype_lookup(__c)&_P)
> -#define isalnum(__c)	(__ctype_lookup(__c)&(_U|_L|_N))
> -#define isprint(__c)	(__ctype_lookup(__c)&(_P|_U|_L|_N|_B))
> -#define	isgraph(__c)	(__ctype_lookup(__c)&(_P|_U|_L|_N))
> -#define iscntrl(__c)	(__ctype_lookup(__c)&_C)
> +#define	isalpha(__c)	(__ctype_lookup(__c) & (_ISupper|_ISlower))
> +#define	isupper(__c)	((__ctype_lookup(__c) & (_ISupper|_ISlower)) == _ISupper)
> +#define	islower(__c)	((__ctype_lookup(__c) & (_ISupper|_ISlower)) == _ISlower)
> +#define	isdigit(__c)	(__ctype_lookup(__c) & _ISdigit)
> +#define	isxdigit(__c)	(__ctype_lookup(__c) & (_ISxdigit|_ISdigit))
> +#define	isspace(__c)	(__ctype_lookup(__c) & _ISspace)
> +#define ispunct(__c)	(__ctype_lookup(__c) & _ISpunct)
> +#define isalnum(__c)	(__ctype_lookup(__c) & (_ISupper|_ISlower|_ISdigit))
> +#define isprint(__c)	(__ctype_lookup(__c) & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank))
> +#define	isgraph(__c)	(__ctype_lookup(__c) & (_ISpunct|_ISupper|_ISlower|_ISdigit))
> +#define iscntrl(__c)	(__ctype_lookup(__c) & _IScntrl)
>   
>   #if defined(__GNUC__) && __ISO_C_VISIBLE >= 1999
>   #define isblank(__c) \
>     __extension__ ({ __typeof__ (__c) __x = (__c);		\
> -        (__ctype_lookup(__x)&_B) || (int) (__x) == '\t';})
> +        (__ctype_lookup(__x)&_ISblank) || (int) (__x) == '\t';})
>   #endif
>   
>   #if __POSIX_VISIBLE >= 200809
> @@ -120,22 +133,22 @@ __locale_ctype_ptr_l(locale_t _l)
>   #endif
>   #define __ctype_lookup_l(__c,__l) ((__locale_ctype_ptr_l(__l)+sizeof(""[__c]))[(int)(__c)])
>   
> -#define	isalpha_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_U|_L))
> -#define	isupper_l(__c,__l)	((__ctype_lookup_l(__c,__l)&(_U|_L))==_U)
> -#define	islower_l(__c,__l)	((__ctype_lookup_l(__c,__l)&(_U|_L))==_L)
> -#define	isdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_N)
> -#define	isxdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_X|_N))
> -#define	isspace_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_S)
> -#define ispunct_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_P)
> -#define isalnum_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_U|_L|_N))
> -#define isprint_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_P|_U|_L|_N|_B))
> -#define	isgraph_l(__c,__l)	(__ctype_lookup_l(__c,__l)&(_P|_U|_L|_N))
> -#define iscntrl_l(__c,__l)	(__ctype_lookup_l(__c,__l)&_C)
> +#define	isalpha_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower))
> +#define	isupper_l(__c,__l)	((__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower)) == _ISupper)
> +#define	islower_l(__c,__l)	((__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower)) == _ISlower)
> +#define	isdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISdigit)
> +#define	isxdigit_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISxdigit|_ISdigit))
> +#define	isspace_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISspace)
> +#define ispunct_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _ISpunct)
> +#define isalnum_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISupper|_ISlower|_ISdigit))
> +#define isprint_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISpunct|_ISupper|_ISlower|_ISdigit|_ISblank))
> +#define	isgraph_l(__c,__l)	(__ctype_lookup_l(__c,__l) & (_ISpunct|_ISupper|_ISlower|_ISdigit))
> +#define iscntrl_l(__c,__l)	(__ctype_lookup_l(__c,__l) & _IScntrl)
>   
>   #if defined(__GNUC__)
>   #define isblank_l(__c, __l) \
>     __extension__ ({ __typeof__ (__c) __x = (__c);		\
> -        (__ctype_lookup_l(__x,__l)&_B) || (int) (__x) == '\t';})
> +        (__ctype_lookup_l(__x,__l)&_ISblank) || (int) (__x) == '\t';})
>   #endif
>   
>   #endif /* __POSIX_VISIBLE >= 200809 */
> 

Re: [PATCH 2/2] ctype: use less short names in public header

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1591 bytes --]

On 23 Nov 2021 15:09, Richard Earnshaw wrote:
> This is wrong and breaks all old versions of C++.

this is a bit vague.  it would help if you provided details as to what broke.
i doubt this broke all old versions of C++ everywhere.

i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
and its hardcoding of newlib's internal ctype define names.
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0

if you're talking about something else, please state so clearly.

> The GNU sim code should not be using reserved names (those starting _) 
> in normal source code.  Such names are reserved to the implementation.

that's not really a good reason to go pooping all over the namespace.

we can maintain backwards compat here for C++ code fairly easily:

--- a/newlib/libc/include/ctype.h
+++ b/newlib/libc/include/ctype.h
@@ -71,6 +71,16 @@ enum
 
 /* For C++ backward-compatibility only.  */
 extern	__IMPORT const char	_ctype_[];
+#ifdef __cplusplus
+# define _U _ISupper
+# define _L _ISlower
+# define _N _ISdigit
+# define _S _ISspace
+# define _P _ISpunct
+# define _C _IScntrl
+# define _X _ISxdigit
+# define _B _ISblank
+#endif
 
 #ifdef __HAVE_LOCALE_INFO__
 const char *__locale_ctype_ptr (void);

considering the numerical value is part of the ABI, not the name, libstdc++
could have inlined the constant values instead.  i wonder how long of a version
skew is reasonable if we wanted to transition it to the new names to match what
glibc uses.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] ctype: use less short names in public header

[Richard Earnshaw]

On 24/11/2021 04:15, Mike Frysinger wrote:
> On 23 Nov 2021 15:09, Richard Earnshaw wrote:
>> This is wrong and breaks all old versions of C++.
> 
> this is a bit vague.  it would help if you provided details as to what broke.
> i doubt this broke all old versions of C++ everywhere.

My apologies, I did mean to say GNU C++.
> 
> i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
> and its hardcoding of newlib's internal ctype define names.
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0
> 
> if you're talking about something else, please state so clearly.
> 
>> The GNU sim code should not be using reserved names (those starting _) 
>> in normal source code.  Such names are reserved to the implementation.
> 
> that's not really a good reason to go pooping all over the namespace.
> 
It's not 'pooping' on the namespace.  Identifiers starting with _ are
all reserved, one way or another; some for the compiler, some for the
library implementation and some for future changes to the standard.
Applications have no business defining such names.

> we can maintain backwards compat here for C++ code fairly easily:
> 
> --- a/newlib/libc/include/ctype.h
> +++ b/newlib/libc/include/ctype.h
> @@ -71,6 +71,16 @@ enum
>  
>  /* For C++ backward-compatibility only.  */
>  extern	__IMPORT const char	_ctype_[];
> +#ifdef __cplusplus
> +# define _U _ISupper
> +# define _L _ISlower
> +# define _N _ISdigit
> +# define _S _ISspace
> +# define _P _ISpunct
> +# define _C _IScntrl
> +# define _X _ISxdigit
> +# define _B _ISblank
> +#endif


>  
>  #ifdef __HAVE_LOCALE_INFO__
>  const char *__locale_ctype_ptr (void);
> 
> considering the numerical value is part of the ABI, not the name, libstdc++
> could have inlined the constant values instead.  i wonder how long of a version
> skew is reasonable if we wanted to transition it to the new names to match what
> glibc uses.


I think you'll find that the BSD libraries have been using _[ULNSPCXB]
since pretty much forever.  Why do we need to be compatible with glibc?

R.

> -mike
> 

Re: [PATCH 2/2] ctype: use less short names in public header

[Richard Earnshaw]

On 24/11/2021 10:58, Richard Earnshaw wrote:
> On 24/11/2021 04:15, Mike Frysinger wrote:
>> On 23 Nov 2021 15:09, Richard Earnshaw wrote:
>>> This is wrong and breaks all old versions of C++.
>>
>> this is a bit vague.  it would help if you provided details as to what broke.
>> i doubt this broke all old versions of C++ everywhere.
> 
> My apologies, I did mean to say GNU C++.
>>
>> i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
>> and its hardcoding of newlib's internal ctype define names.
>> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0
>>
>> if you're talking about something else, please state so clearly.
>>
>>> The GNU sim code should not be using reserved names (those starting _) 
>>> in normal source code.  Such names are reserved to the implementation.
>>
>> that's not really a good reason to go pooping all over the namespace.
>>
> It's not 'pooping' on the namespace.  Identifiers starting with _ are
> all reserved, one way or another; some for the compiler, some for the
> library implementation and some for future changes to the standard.
> Applications have no business defining such names.
> 
>> we can maintain backwards compat here for C++ code fairly easily:
>>
>> --- a/newlib/libc/include/ctype.h
>> +++ b/newlib/libc/include/ctype.h
>> @@ -71,6 +71,16 @@ enum
>>  
>>  /* For C++ backward-compatibility only.  */
>>  extern	__IMPORT const char	_ctype_[];
>> +#ifdef __cplusplus
>> +# define _U _ISupper
>> +# define _L _ISlower
>> +# define _N _ISdigit
>> +# define _S _ISspace
>> +# define _P _ISpunct
>> +# define _C _IScntrl
>> +# define _X _ISxdigit
>> +# define _B _ISblank
>> +#endif
> 
> 
>>  
>>  #ifdef __HAVE_LOCALE_INFO__
>>  const char *__locale_ctype_ptr (void);
>>
>> considering the numerical value is part of the ABI, not the name, libstdc++
>> could have inlined the constant values instead.  i wonder how long of a version
>> skew is reasonable if we wanted to transition it to the new names to match what
>> glibc uses.
> 
> 
> I think you'll find that the BSD libraries have been using _[ULNSPCXB]
> since pretty much forever.  Why do we need to be compatible with glibc?

Eg: http://www.retro11.de/ouxr/43bsd/usr/src/include/ctype.h.html

R.

> 
> R.
> 
>> -mike
>>
> 

Re: [PATCH 2/2] ctype: use less short names in public header

[Jonathan Wakely]

On 23/11/21 23:15 -0500, Mike Frysinger wrote:
>On 23 Nov 2021 15:09, Richard Earnshaw wrote:
>> This is wrong and breaks all old versions of C++.
>
>this is a bit vague.  it would help if you provided details as to what broke.
>i doubt this broke all old versions of C++ everywhere.
>
>i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
>and its hardcoding of newlib's internal ctype define names.
>https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0

Yes, you were CC'd on the GCC bug slightly before Richard sent his
email to this list:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c16

>if you're talking about something else, please state so clearly.
>
>> The GNU sim code should not be using reserved names (those starting _)
>> in normal source code.  Such names are reserved to the implementation.
>
>that's not really a good reason to go pooping all over the namespace.
>
>we can maintain backwards compat here for C++ code fairly easily:

Yes, or only do that for GCC < 12, as I suggested in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c19

#if defined(__GNUC__) && defined(__cplusplus)
# if __GNUC__ < 12

The libstdc++ code on trunk uses the new _ISupper names.

I have no opinion on how long you should keep such backwards
compatibility around. Whatever time limit you set, at some point it
will make a new newlib release unusable with past G++ versions.

But either way, the GNU sim code needed to be fixed. Your commit msg
for that was misleading:

     Some C libraries export _P symbols in their headers (like older
     newlib and its ctype.h), so use P_ instead to avoid conflicts.

It's irrelevant whether they use it or not: _P is a reserved name
meaning they *could* use it, and so anything outside "the
implementation" MUST NOT use it.

This isn't "that particular name happens to clash with a particular
header", it's "that name causes undefined behaviour, period".

Re: [PATCH 2/2] ctype: use less short names in public header

[Jonathan Wakely]

On 09/11/21 19:18 -0500, Mike Frysinger wrote:
>On 09 Nov 2021 12:38, Corinna Vinschen wrote:
>> On Nov  8 20:24, Mike Frysinger wrote:
>> > We're seeing a build failure in GNU sim code which is using _P locally
>> > but the ctype.h define clashes with it.  Rename these to use the same
>> > symbols that glibc does.  They're a bit more verbose, but seems likely
>> > that we'll have fewer conflicts if glibc isn't seeing them.
>>                                      ^^^^^
>> Mixing newlib and glibc?  That's just a typo, I guess?
>
>i meant glibc here.  these are the same symbol names that glibc is using,
>and it's not seeing conflicts in the wider ecosystem.  so if they aren't
>seeing issues, it's likely newlib won't either if it uses the same names.

There's a reason you don't see conflicts with those names:

They are reserved names, just like _P.

Any user code that conflicts with those names has a bug and needs
fixing.

Re: [PATCH 2/2] ctype: use less short names in public header

[Corinna Vinschen]

On Nov 30 12:01, Jonathan Wakely wrote:
> On 23/11/21 23:15 -0500, Mike Frysinger wrote:
> > On 23 Nov 2021 15:09, Richard Earnshaw wrote:
> > > This is wrong and breaks all old versions of C++.
> > 
> > this is a bit vague.  it would help if you provided details as to what broke.
> > i doubt this broke all old versions of C++ everywhere.
> > 
> > i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
> > and its hardcoding of newlib's internal ctype define names.
> > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0
> 
> Yes, you were CC'd on the GCC bug slightly before Richard sent his
> email to this list:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c16
> 
> > if you're talking about something else, please state so clearly.
> > 
> > > The GNU sim code should not be using reserved names (those starting _)
> > > in normal source code.  Such names are reserved to the implementation.
> > 
> > that's not really a good reason to go pooping all over the namespace.
> > 
> > we can maintain backwards compat here for C++ code fairly easily:
> 
> Yes, or only do that for GCC < 12, as I suggested in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c19
> 
> #if defined(__GNUC__) && defined(__cplusplus)
> # if __GNUC__ < 12
> 
> The libstdc++ code on trunk uses the new _ISupper names.
> 
> I have no opinion on how long you should keep such backwards
> compatibility around. Whatever time limit you set, at some point it
> will make a new newlib release unusable with past G++ versions.

Is there a good reason to revert these patches in newlib?  I see the
problem but I'm unclear on how problematic the change is in real life.


Thanks,
Corinna

Re: [PATCH 2/2] ctype: use less short names in public header

[Jonathan Wakely]

On Tue, 30 Nov 2021 at 15:14, Corinna Vinschen <vinschen@redhat.com> wrote:
>
> On Nov 30 12:01, Jonathan Wakely wrote:
> > On 23/11/21 23:15 -0500, Mike Frysinger wrote:
> > > On 23 Nov 2021 15:09, Richard Earnshaw wrote:
> > > > This is wrong and breaks all old versions of C++.
> > >
> > > this is a bit vague.  it would help if you provided details as to what broke.
> > > i doubt this broke all old versions of C++ everywhere.
> > >
> > > i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
> > > and its hardcoding of newlib's internal ctype define names.
> > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0
> >
> > Yes, you were CC'd on the GCC bug slightly before Richard sent his
> > email to this list:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c16
> >
> > > if you're talking about something else, please state so clearly.
> > >
> > > > The GNU sim code should not be using reserved names (those starting _)
> > > > in normal source code.  Such names are reserved to the implementation.
> > >
> > > that's not really a good reason to go pooping all over the namespace.
> > >
> > > we can maintain backwards compat here for C++ code fairly easily:
> >
> > Yes, or only do that for GCC < 12, as I suggested in
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c19
> >
> > #if defined(__GNUC__) && defined(__cplusplus)
> > # if __GNUC__ < 12
> >
> > The libstdc++ code on trunk uses the new _ISupper names.
> >
> > I have no opinion on how long you should keep such backwards
> > compatibility around. Whatever time limit you set, at some point it
> > will make a new newlib release unusable with past G++ versions.
>
> Is there a good reason to revert these patches in newlib?  I see the
> problem but I'm unclear on how problematic the change is in real life.

You cannot use newlib from Git to build any released version of GCC.

Is building newlib from Git only supported when using GCC trunk, or is
it supposed to build with e.g. GCC 11.2.0 from July this year? If yes,
then newlib needs changes (whether reverting the change entirely, or
just making another change to restore the old names in addition to the
new ones).

Re: [PATCH 2/2] ctype: use less short names in public header

[Jonathan Wakely]

On Tue, 30 Nov 2021 at 17:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>
> On Tue, 30 Nov 2021 at 15:14, Corinna Vinschen <vinschen@redhat.com> wrote:
> >
> > On Nov 30 12:01, Jonathan Wakely wrote:
> > > On 23/11/21 23:15 -0500, Mike Frysinger wrote:
> > > > On 23 Nov 2021 15:09, Richard Earnshaw wrote:
> > > > > This is wrong and breaks all old versions of C++.
> > > >
> > > > this is a bit vague.  it would help if you provided details as to what broke.
> > > > i doubt this broke all old versions of C++ everywhere.
> > > >
> > > > i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
> > > > and its hardcoding of newlib's internal ctype define names.
> > > > https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0
> > >
> > > Yes, you were CC'd on the GCC bug slightly before Richard sent his
> > > email to this list:
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c16
> > >
> > > > if you're talking about something else, please state so clearly.
> > > >
> > > > > The GNU sim code should not be using reserved names (those starting _)
> > > > > in normal source code.  Such names are reserved to the implementation.
> > > >
> > > > that's not really a good reason to go pooping all over the namespace.
> > > >
> > > > we can maintain backwards compat here for C++ code fairly easily:
> > >
> > > Yes, or only do that for GCC < 12, as I suggested in
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c19
> > >
> > > #if defined(__GNUC__) && defined(__cplusplus)
> > > # if __GNUC__ < 12
> > >
> > > The libstdc++ code on trunk uses the new _ISupper names.
> > >
> > > I have no opinion on how long you should keep such backwards
> > > compatibility around. Whatever time limit you set, at some point it
> > > will make a new newlib release unusable with past G++ versions.
> >
> > Is there a good reason to revert these patches in newlib?  I see the
> > problem but I'm unclear on how problematic the change is in real life.
>
> You cannot use newlib from Git to build any released version of GCC.
>
> Is building newlib from Git only supported when using GCC trunk, or is

Oops, I mean building *against* newlib from Git, not building newlib
itself. You can still build newlib itself, because it doesn't use C++.
But you can't build a GCC 11.2.0 compiler that uses the latest newlib
from Git as its libc.

> it supposed to build with e.g. GCC 11.2.0 from July this year? If yes,
> then newlib needs changes (whether reverting the change entirely, or
> just making another change to restore the old names in addition to the
> new ones).

Re: [PATCH 2/2] ctype: use less short names in public header

[Richard Earnshaw]

On 30/11/2021 17:15, Jonathan Wakely wrote:
> On Tue, 30 Nov 2021 at 17:12, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> On Tue, 30 Nov 2021 at 15:14, Corinna Vinschen <vinschen@redhat.com> wrote:
>>>
>>> On Nov 30 12:01, Jonathan Wakely wrote:
>>>> On 23/11/21 23:15 -0500, Mike Frysinger wrote:
>>>>> On 23 Nov 2021 15:09, Richard Earnshaw wrote:
>>>>>> This is wrong and breaks all old versions of C++.
>>>>>
>>>>> this is a bit vague.  it would help if you provided details as to what broke.
>>>>> i doubt this broke all old versions of C++ everywhere.
>>>>>
>>>>> i'm guessing you're referring to the GNU C++ (libstdc++) library specifically
>>>>> and its hardcoding of newlib's internal ctype define names.
>>>>> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/config/os/newlib/ctype_base.h;hb=releases/gcc-11.2.0
>>>>
>>>> Yes, you were CC'd on the GCC bug slightly before Richard sent his
>>>> email to this list:
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c16
>>>>
>>>>> if you're talking about something else, please state so clearly.
>>>>>
>>>>>> The GNU sim code should not be using reserved names (those starting _)
>>>>>> in normal source code.  Such names are reserved to the implementation.
>>>>>
>>>>> that's not really a good reason to go pooping all over the namespace.
>>>>>
>>>>> we can maintain backwards compat here for C++ code fairly easily:
>>>>
>>>> Yes, or only do that for GCC < 12, as I suggested in
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103305#c19
>>>>
>>>> #if defined(__GNUC__) && defined(__cplusplus)
>>>> # if __GNUC__ < 12
>>>>
>>>> The libstdc++ code on trunk uses the new _ISupper names.
>>>>
>>>> I have no opinion on how long you should keep such backwards
>>>> compatibility around. Whatever time limit you set, at some point it
>>>> will make a new newlib release unusable with past G++ versions.
>>>
>>> Is there a good reason to revert these patches in newlib?  I see the
>>> problem but I'm unclear on how problematic the change is in real life.
>>
>> You cannot use newlib from Git to build any released version of GCC.
>>
>> Is building newlib from Git only supported when using GCC trunk, or is
> 
> Oops, I mean building *against* newlib from Git, not building newlib
> itself. You can still build newlib itself, because it doesn't use C++.
> But you can't build a GCC 11.2.0 compiler that uses the latest newlib
> from Git as its libc.
> 
>> it supposed to build with e.g. GCC 11.2.0 from July this year? If yes,
>> then newlib needs changes (whether reverting the change entirely, or
>> just making another change to restore the old names in addition to the
>> new ones).
> 

My concern is that the proposed workaround may break other (probably 
buggy) apps that have been relying on the old BSD internal API for 30 
odd years.  The proposed workaround only solves the issue for G++.

R.

Re: [PATCH 2/2] ctype: use less short names in public header

[Corinna Vinschen]

On Nov 30 17:52, Richard Earnshaw wrote:
> On 30/11/2021 17:15, Jonathan Wakely wrote:
> > On Tue, 30 Nov 2021 at 17:12, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > 
> > > On Tue, 30 Nov 2021 at 15:14, Corinna Vinschen <vinschen@redhat.com> wrote:
> > > > Is there a good reason to revert these patches in newlib?  I see the
> > > > problem but I'm unclear on how problematic the change is in real life.
> > > 
> > > You cannot use newlib from Git to build any released version of GCC.
> > > 
> > > Is building newlib from Git only supported when using GCC trunk, or is
> > 
> > Oops, I mean building *against* newlib from Git, not building newlib
> > itself. You can still build newlib itself, because it doesn't use C++.
> > But you can't build a GCC 11.2.0 compiler that uses the latest newlib
> > from Git as its libc.
> > 
> > > it supposed to build with e.g. GCC 11.2.0 from July this year? If yes,
> > > then newlib needs changes (whether reverting the change entirely, or
> > > just making another change to restore the old names in addition to the
> > > new ones).
> > 
> 
> My concern is that the proposed workaround may break other (probably buggy)
> apps that have been relying on the old BSD internal API for 30 odd years.
> The proposed workaround only solves the issue for G++.

I'm inclined to revert 3ba1bd0d9db, given it solves a problem which
isn't actually a problem in newlib, but in an application not following
the standards in terms of reserved symbols.

I'm discussing this with Jeff, stay tuned.


Corinna

Re: [PATCH 2/2] ctype: use less short names in public header

[Corinna Vinschen]

On Dec  2 11:27, Corinna Vinschen wrote:
> On Nov 30 17:52, Richard Earnshaw wrote:
> > On 30/11/2021 17:15, Jonathan Wakely wrote:
> > > On Tue, 30 Nov 2021 at 17:12, Jonathan Wakely <jwakely@redhat.com> wrote:
> > > > 
> > > > On Tue, 30 Nov 2021 at 15:14, Corinna Vinschen <vinschen@redhat.com> wrote:
> > > > > Is there a good reason to revert these patches in newlib?  I see the
> > > > > problem but I'm unclear on how problematic the change is in real life.
> > > > 
> > > > You cannot use newlib from Git to build any released version of GCC.
> > > > 
> > > > Is building newlib from Git only supported when using GCC trunk, or is
> > > 
> > > Oops, I mean building *against* newlib from Git, not building newlib
> > > itself. You can still build newlib itself, because it doesn't use C++.
> > > But you can't build a GCC 11.2.0 compiler that uses the latest newlib
> > > from Git as its libc.
> > > 
> > > > it supposed to build with e.g. GCC 11.2.0 from July this year? If yes,
> > > > then newlib needs changes (whether reverting the change entirely, or
> > > > just making another change to restore the old names in addition to the
> > > > new ones).
> > > 
> > 
> > My concern is that the proposed workaround may break other (probably buggy)
> > apps that have been relying on the old BSD internal API for 30 odd years.
> > The proposed workaround only solves the issue for G++.
> 
> I'm inclined to revert 3ba1bd0d9db, given it solves a problem which
> isn't actually a problem in newlib, but in an application not following
> the standards in terms of reserved symbols.
> 
> I'm discussing this with Jeff, stay tuned.

Reverted.


Corinna

Re: [PATCH 2/2] ctype: use less short names in public header

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 403 bytes --]

On 30 Nov 2021 17:52, Richard Earnshaw wrote:
> My concern is that the proposed workaround may break other (probably 
> buggy) apps that have been relying on the old BSD internal API for 30 
> odd years.  The proposed workaround only solves the issue for G++.

those programs, if they exist, don't build on glibc or other POSIX libs
either.  so i doubt such apps really exist in practice.
-mike

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]