Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Gerald Pfeifer]

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

Hi Matthew,

On Mon, 4 Feb 2019, Matthew Bauer wrote:
> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
> have changed their ctype.h definition. It was updated in their intree
> libstdc++-v3 but not in the GCC one. My understanding is this is a
> straightforward rewrite. I've attached my own patch, but the file can
> be obtained directly here:
> 
> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
> 
> With the attached patch, libstdc++-v3 can succesfully be built with
> NetBSD headers (along with --disable-libcilkrts).

I noticed this has not been applied yet, nor seen a follow-up?, and also 
noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.

Let me re-address this to libstdc++@gcc.gnu.org in the hope the 
maintainers there will have a look.

Gerald

[-- Attachment #2: Type: text/x-patch, Size: 5009 bytes --]

diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
index ff3ec893974..21eccf9fde1 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   /// @brief  Base class for ctype.
   struct ctype_base
   {
-    // Non-standard typedefs.
-    typedef const unsigned char*	__to_type;
 
     // NB: Offsets into ctype<char>::_M_table force a particular size
     // on the mask type. Because of this, we don't use an enum.
-    typedef unsigned char      	mask;
 
 #ifndef _CTYPE_U
-    static const mask upper    	= _U;
-    static const mask lower 	= _L;
-    static const mask alpha 	= _U | _L;
-    static const mask digit 	= _N;
-    static const mask xdigit 	= _N | _X;
-    static const mask space 	= _S;
-    static const mask print 	= _P | _U | _L | _N | _B;
-    static const mask graph 	= _P | _U | _L | _N;
-    static const mask cntrl 	= _C;
-    static const mask punct 	= _P;
-    static const mask alnum 	= _U | _L | _N;
+    // Non-standard typedefs.
+    typedef const unsigned char*	__to_type;
+
+    typedef unsigned char	mask;
+
+    static const mask upper	= _U;
+    static const mask lower	= _L;
+    static const mask alpha	= _U | _L;
+    static const mask digit	= _N;
+    static const mask xdigit	= _N | _X;
+    static const mask space	= _S;
+    static const mask print	= _P | _U | _L | _N | _B;
+    static const mask graph	= _P | _U | _L | _N;
+    static const mask cntrl	= _C;
+    static const mask punct	= _P;
+    static const mask alnum	= _U | _L | _N;
 #else
-    static const mask upper    	= _CTYPE_U;
-    static const mask lower 	= _CTYPE_L;
-    static const mask alpha 	= _CTYPE_U | _CTYPE_L;
-    static const mask digit 	= _CTYPE_N;
-    static const mask xdigit 	= _CTYPE_N | _CTYPE_X;
-    static const mask space 	= _CTYPE_S;
-    static const mask print 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B;
-    static const mask graph 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N;
-    static const mask cntrl 	= _CTYPE_C;
-    static const mask punct 	= _CTYPE_P;
-    static const mask alnum 	= _CTYPE_U | _CTYPE_L | _CTYPE_N;
+    typedef const unsigned short*	__to_type;
+
+    typedef unsigned short	mask;
+
+    static const mask upper	= _CTYPE_U;
+    static const mask lower	= _CTYPE_L;
+    static const mask alpha	= _CTYPE_A;
+    static const mask digit	= _CTYPE_D;
+    static const mask xdigit	= _CTYPE_X;
+    static const mask space	= _CTYPE_S;
+    static const mask print	= _CTYPE_R;
+    static const mask graph	= _CTYPE_G;
+    static const mask cntrl	= _CTYPE_C;
+    static const mask punct	= _CTYPE_P;
+    static const mask alnum	= _CTYPE_A | _CTYPE_D;
 #endif
 #if __cplusplus >= 201103L
-    static const mask blank 	= space;
+    static const mask blank	= space;
 #endif
   };
 
diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
index ed3b7cd0d6a..33358e8f5d8 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
@@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 // Information as gleaned from /usr/include/ctype.h
 
-  extern "C" const u_int8_t _C_ctype_[];
-
   const ctype_base::mask*
   ctype<char>::classic_table() throw()
-  { return _C_ctype_ + 1; }
+  { return _C_ctype_tab_ + 1; }
 
   ctype<char>::ctype(__c_locale, const mask* __table, bool __del,
 		     size_t __refs)
@@ -69,14 +67,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   char
   ctype<char>::do_toupper(char __c) const
-  { return ::toupper((int) __c); }
+  { return ::toupper((int)(unsigned char) __c); }
 
   const char*
   ctype<char>::do_toupper(char* __low, const char* __high) const
   {
     while (__low < __high)
       {
-	*__low = ::toupper((int) *__low);
+	*__low = ::toupper((int)(unsigned char) *__low);
 	++__low;
       }
     return __high;
@@ -84,14 +82,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   char
   ctype<char>::do_tolower(char __c) const
-  { return ::tolower((int) __c); }
+  { return ::tolower((int)(unsigned char) __c); }
 
   const char*
   ctype<char>::do_tolower(char* __low, const char* __high) const
   {
     while (__low < __high)
       {
-	*__low = ::tolower((int) *__low);
+	*__low = ::tolower((int)(unsigned char) *__low);
 	++__low;
       }
     return __high;
diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h
index ace1120fba2..3234ce17c70 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h
@@ -48,7 +48,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   is(const char* __low, const char* __high, mask* __vec) const
   {
     while (__low < __high)
-      *__vec++ = _M_table[*__low++];
+      *__vec++ = _M_table[(unsigned char)*__low++];
     return __high;
   }
 

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1016 bytes --]

On 22.12.2019 00:36, Gerald Pfeifer wrote:
> Hi Matthew,
> 
> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>> have changed their ctype.h definition. It was updated in their intree
>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>> straightforward rewrite. I've attached my own patch, but the file can
>> be obtained directly here:
>>
>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>
>> With the attached patch, libstdc++-v3 can succesfully be built with
>> NetBSD headers (along with --disable-libcilkrts).
> 
> I noticed this has not been applied yet, nor seen a follow-up?, and also 
> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
> 
> Let me re-address this to libstdc++@gcc.gnu.org in the hope the 
> maintainers there will have a look.
> 
> Gerald
> 

We (in NetBSD) wait for this to be merged.


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

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On Sat, 21 Dec 2019 at 23:37, Gerald Pfeifer <gerald@pfeifer.com> wrote:
>
> Hi Matthew,
>
> On Mon, 4 Feb 2019, Matthew Bauer wrote:
> > The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
> > have changed their ctype.h definition. It was updated in their intree
> > libstdc++-v3 but not in the GCC one. My understanding is this is a
> > straightforward rewrite. I've attached my own patch, but the file can
> > be obtained directly here:
> >
> > http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
> >
> > With the attached patch, libstdc++-v3 can succesfully be built with
> > NetBSD headers (along with --disable-libcilkrts).
>
> I noticed this has not been applied yet, nor seen a follow-up?, and also
> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.

That's why it was ignored then.

> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
> maintainers there will have a look.

I'll take a look ASAP.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>Hi Matthew,
>
>On Mon, 4 Feb 2019, Matthew Bauer wrote:
>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>> have changed their ctype.h definition. It was updated in their intree
>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>> straightforward rewrite. I've attached my own patch, but the file can
>> be obtained directly here:
>>
>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>
>> With the attached patch, libstdc++-v3 can succesfully be built with
>> NetBSD headers (along with --disable-libcilkrts).
>
>I noticed this has not been applied yet, nor seen a follow-up?, and also
>noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>
>Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>maintainers there will have a look.
>
>Gerald

>diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>index ff3ec893974..21eccf9fde1 100644
>--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   /// @brief  Base class for ctype.
>   struct ctype_base
>   {
>-    // Non-standard typedefs.
>-    typedef const unsigned char*	__to_type;
> 
>     // NB: Offsets into ctype<char>::_M_table force a particular size
>     // on the mask type. Because of this, we don't use an enum.
>-    typedef unsigned char      	mask;
> 
> #ifndef _CTYPE_U
>-    static const mask upper    	= _U;
>-    static const mask lower 	= _L;
>-    static const mask alpha 	= _U | _L;
>-    static const mask digit 	= _N;
>-    static const mask xdigit 	= _N | _X;
>-    static const mask space 	= _S;
>-    static const mask print 	= _P | _U | _L | _N | _B;
>-    static const mask graph 	= _P | _U | _L | _N;
>-    static const mask cntrl 	= _C;
>-    static const mask punct 	= _P;
>-    static const mask alnum 	= _U | _L | _N;
>+    // Non-standard typedefs.
>+    typedef const unsigned char*	__to_type;
>+
>+    typedef unsigned char	mask;
>+
>+    static const mask upper	= _U;
>+    static const mask lower	= _L;
>+    static const mask alpha	= _U | _L;
>+    static const mask digit	= _N;
>+    static const mask xdigit	= _N | _X;
>+    static const mask space	= _S;
>+    static const mask print	= _P | _U | _L | _N | _B;
>+    static const mask graph	= _P | _U | _L | _N;
>+    static const mask cntrl	= _C;
>+    static const mask punct	= _P;
>+    static const mask alnum	= _U | _L | _N;
> #else
>-    static const mask upper    	= _CTYPE_U;
>-    static const mask lower 	= _CTYPE_L;
>-    static const mask alpha 	= _CTYPE_U | _CTYPE_L;
>-    static const mask digit 	= _CTYPE_N;
>-    static const mask xdigit 	= _CTYPE_N | _CTYPE_X;
>-    static const mask space 	= _CTYPE_S;
>-    static const mask print 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B;
>-    static const mask graph 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N;
>-    static const mask cntrl 	= _CTYPE_C;
>-    static const mask punct 	= _CTYPE_P;
>-    static const mask alnum 	= _CTYPE_U | _CTYPE_L | _CTYPE_N;
>+    typedef const unsigned short*	__to_type;
>+
>+    typedef unsigned short	mask;

I seem to recall looking at this previously and noting that the change
to ctype_base::mask is an ABI break. It means that code compiled with
old versions of GCC or on old versions of NetBSD will not be ABI
compatible with code compiled by a new GCC on a new version of NetBSD.

If the NetBSD maintainers are OK with that, then we can go ahead and
change it.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>Hi Matthew,
>
>On Mon, 4 Feb 2019, Matthew Bauer wrote:
>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>> have changed their ctype.h definition. It was updated in their intree
>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>> straightforward rewrite. I've attached my own patch, but the file can
>> be obtained directly here:
>>
>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>
>> With the attached patch, libstdc++-v3 can succesfully be built with
>> NetBSD headers (along with --disable-libcilkrts).
>
>I noticed this has not been applied yet, nor seen a follow-up?, and also
>noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>
>Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>maintainers there will have a look.
>
>Gerald

>diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>index ff3ec893974..21eccf9fde1 100644
>--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>   /// @brief  Base class for ctype.
>   struct ctype_base
>   {
>-    // Non-standard typedefs.
>-    typedef const unsigned char*	__to_type;
> 
>     // NB: Offsets into ctype<char>::_M_table force a particular size
>     // on the mask type. Because of this, we don't use an enum.
>-    typedef unsigned char      	mask;
> 
> #ifndef _CTYPE_U
>-    static const mask upper    	= _U;
>-    static const mask lower 	= _L;
>-    static const mask alpha 	= _U | _L;
>-    static const mask digit 	= _N;
>-    static const mask xdigit 	= _N | _X;
>-    static const mask space 	= _S;
>-    static const mask print 	= _P | _U | _L | _N | _B;
>-    static const mask graph 	= _P | _U | _L | _N;
>-    static const mask cntrl 	= _C;
>-    static const mask punct 	= _P;
>-    static const mask alnum 	= _U | _L | _N;
>+    // Non-standard typedefs.
>+    typedef const unsigned char*	__to_type;
>+
>+    typedef unsigned char	mask;
>+
>+    static const mask upper	= _U;
>+    static const mask lower	= _L;
>+    static const mask alpha	= _U | _L;
>+    static const mask digit	= _N;
>+    static const mask xdigit	= _N | _X;
>+    static const mask space	= _S;
>+    static const mask print	= _P | _U | _L | _N | _B;
>+    static const mask graph	= _P | _U | _L | _N;
>+    static const mask cntrl	= _C;
>+    static const mask punct	= _P;
>+    static const mask alnum	= _U | _L | _N;
> #else
>-    static const mask upper    	= _CTYPE_U;
>-    static const mask lower 	= _CTYPE_L;
>-    static const mask alpha 	= _CTYPE_U | _CTYPE_L;
>-    static const mask digit 	= _CTYPE_N;
>-    static const mask xdigit 	= _CTYPE_N | _CTYPE_X;
>-    static const mask space 	= _CTYPE_S;
>-    static const mask print 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B;
>-    static const mask graph 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N;
>-    static const mask cntrl 	= _CTYPE_C;
>-    static const mask punct 	= _CTYPE_P;
>-    static const mask alnum 	= _CTYPE_U | _CTYPE_L | _CTYPE_N;
>+    typedef const unsigned short*	__to_type;
>+
>+    typedef unsigned short	mask;
>+
>+    static const mask upper	= _CTYPE_U;
>+    static const mask lower	= _CTYPE_L;
>+    static const mask alpha	= _CTYPE_A;
>+    static const mask digit	= _CTYPE_D;
>+    static const mask xdigit	= _CTYPE_X;
>+    static const mask space	= _CTYPE_S;
>+    static const mask print	= _CTYPE_R;
>+    static const mask graph	= _CTYPE_G;
>+    static const mask cntrl	= _CTYPE_C;
>+    static const mask punct	= _CTYPE_P;
>+    static const mask alnum	= _CTYPE_A | _CTYPE_D;
> #endif
> #if __cplusplus >= 201103L
>-    static const mask blank 	= space;
>+    static const mask blank	= space;
> #endif
>   };
> 
>diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>index ed3b7cd0d6a..33358e8f5d8 100644
>--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>@@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> 
> // Information as gleaned from /usr/include/ctype.h
> 
>-  extern "C" const u_int8_t _C_ctype_[];
>-
>   const ctype_base::mask*
>   ctype<char>::classic_table() throw()
>-  { return _C_ctype_ + 1; }
>+  { return _C_ctype_tab_ + 1; }

The first patch attached to PR 64271 (i.e.
https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that
differently. Is it safe to make this change unconditionally?

Who authored these patches? We don't seem to have a changelog, not
even an author's name and email address, as required for any patch.

Here was my previous review, where I mentioned the ABI break:
https://gcc.gnu.org/ml/libstdc++/2014-12/msg00069.html
I didn't get a reply.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 4477 bytes --]

On 06.01.2020 16:24, Jonathan Wakely wrote:
> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>> Hi Matthew,
>>
>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>> have changed their ctype.h definition. It was updated in their intree
>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>> straightforward rewrite. I've attached my own patch, but the file can
>>> be obtained directly here:
>>>
>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>
>>>
>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>> NetBSD headers (along with --disable-libcilkrts).
>>
>> I noticed this has not been applied yet, nor seen a follow-up?, and also
>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>
>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>> maintainers there will have a look.
>>
>> Gerald
> 
>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> index ff3ec893974..21eccf9fde1 100644
>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>   /// @brief  Base class for ctype.
>>   struct ctype_base
>>   {
>> -    // Non-standard typedefs.
>> -    typedef const unsigned char*    __to_type;
>>
>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>     // on the mask type. Because of this, we don't use an enum.
>> -    typedef unsigned char          mask;
>>
>> #ifndef _CTYPE_U
>> -    static const mask upper        = _U;
>> -    static const mask lower     = _L;
>> -    static const mask alpha     = _U | _L;
>> -    static const mask digit     = _N;
>> -    static const mask xdigit     = _N | _X;
>> -    static const mask space     = _S;
>> -    static const mask print     = _P | _U | _L | _N | _B;
>> -    static const mask graph     = _P | _U | _L | _N;
>> -    static const mask cntrl     = _C;
>> -    static const mask punct     = _P;
>> -    static const mask alnum     = _U | _L | _N;
>> +    // Non-standard typedefs.
>> +    typedef const unsigned char*    __to_type;
>> +
>> +    typedef unsigned char    mask;
>> +
>> +    static const mask upper    = _U;
>> +    static const mask lower    = _L;
>> +    static const mask alpha    = _U | _L;
>> +    static const mask digit    = _N;
>> +    static const mask xdigit    = _N | _X;
>> +    static const mask space    = _S;
>> +    static const mask print    = _P | _U | _L | _N | _B;
>> +    static const mask graph    = _P | _U | _L | _N;
>> +    static const mask cntrl    = _C;
>> +    static const mask punct    = _P;
>> +    static const mask alnum    = _U | _L | _N;
>> #else
>> -    static const mask upper        = _CTYPE_U;
>> -    static const mask lower     = _CTYPE_L;
>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>> -    static const mask digit     = _CTYPE_N;
>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>> -    static const mask space     = _CTYPE_S;
>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>> _CTYPE_N | _CTYPE_B;
>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>> _CTYPE_N;
>> -    static const mask cntrl     = _CTYPE_C;
>> -    static const mask punct     = _CTYPE_P;
>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>> +    typedef const unsigned short*    __to_type;
>> +
>> +    typedef unsigned short    mask;
> 
> I seem to recall looking at this previously and noting that the change
> to ctype_base::mask is an ABI break. It means that code compiled with
> old versions of GCC or on old versions of NetBSD will not be ABI
> compatible with code compiled by a new GCC on a new version of NetBSD.
> 
> If the NetBSD maintainers are OK with that, then we can go ahead and
> change it.
> 
> 

We are fine with ABI breaks as we bump libstdc++ major on each upgrade
in base.


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

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 6083 bytes --]

On 06.01.2020 16:34, Jonathan Wakely wrote:
> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>> Hi Matthew,
>>
>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>> have changed their ctype.h definition. It was updated in their intree
>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>> straightforward rewrite. I've attached my own patch, but the file can
>>> be obtained directly here:
>>>
>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>
>>>
>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>> NetBSD headers (along with --disable-libcilkrts).
>>
>> I noticed this has not been applied yet, nor seen a follow-up?, and also
>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>
>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>> maintainers there will have a look.
>>
>> Gerald
> 
>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> index ff3ec893974..21eccf9fde1 100644
>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>   /// @brief  Base class for ctype.
>>   struct ctype_base
>>   {
>> -    // Non-standard typedefs.
>> -    typedef const unsigned char*    __to_type;
>>
>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>     // on the mask type. Because of this, we don't use an enum.
>> -    typedef unsigned char          mask;
>>
>> #ifndef _CTYPE_U
>> -    static const mask upper        = _U;
>> -    static const mask lower     = _L;
>> -    static const mask alpha     = _U | _L;
>> -    static const mask digit     = _N;
>> -    static const mask xdigit     = _N | _X;
>> -    static const mask space     = _S;
>> -    static const mask print     = _P | _U | _L | _N | _B;
>> -    static const mask graph     = _P | _U | _L | _N;
>> -    static const mask cntrl     = _C;
>> -    static const mask punct     = _P;
>> -    static const mask alnum     = _U | _L | _N;
>> +    // Non-standard typedefs.
>> +    typedef const unsigned char*    __to_type;
>> +
>> +    typedef unsigned char    mask;
>> +
>> +    static const mask upper    = _U;
>> +    static const mask lower    = _L;
>> +    static const mask alpha    = _U | _L;
>> +    static const mask digit    = _N;
>> +    static const mask xdigit    = _N | _X;
>> +    static const mask space    = _S;
>> +    static const mask print    = _P | _U | _L | _N | _B;
>> +    static const mask graph    = _P | _U | _L | _N;
>> +    static const mask cntrl    = _C;
>> +    static const mask punct    = _P;
>> +    static const mask alnum    = _U | _L | _N;
>> #else
>> -    static const mask upper        = _CTYPE_U;
>> -    static const mask lower     = _CTYPE_L;
>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>> -    static const mask digit     = _CTYPE_N;
>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>> -    static const mask space     = _CTYPE_S;
>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>> _CTYPE_N | _CTYPE_B;
>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>> _CTYPE_N;
>> -    static const mask cntrl     = _CTYPE_C;
>> -    static const mask punct     = _CTYPE_P;
>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>> +    typedef const unsigned short*    __to_type;
>> +
>> +    typedef unsigned short    mask;
>> +
>> +    static const mask upper    = _CTYPE_U;
>> +    static const mask lower    = _CTYPE_L;
>> +    static const mask alpha    = _CTYPE_A;
>> +    static const mask digit    = _CTYPE_D;
>> +    static const mask xdigit    = _CTYPE_X;
>> +    static const mask space    = _CTYPE_S;
>> +    static const mask print    = _CTYPE_R;
>> +    static const mask graph    = _CTYPE_G;
>> +    static const mask cntrl    = _CTYPE_C;
>> +    static const mask punct    = _CTYPE_P;
>> +    static const mask alnum    = _CTYPE_A | _CTYPE_D;
>> #endif
>> #if __cplusplus >= 201103L
>> -    static const mask blank     = space;
>> +    static const mask blank    = space;
>> #endif
>>   };
>>
>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>> index ed3b7cd0d6a..33358e8f5d8 100644
>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>> @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>
>> // Information as gleaned from /usr/include/ctype.h
>>
>> -  extern "C" const u_int8_t _C_ctype_[];
>> -
>>   const ctype_base::mask*
>>   ctype<char>::classic_table() throw()
>> -  { return _C_ctype_ + 1; }
>> +  { return _C_ctype_tab_ + 1; }
> 
> The first patch attached to PR 64271 (i.e.
> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that
> differently. Is it safe to make this change unconditionally?
> 

This file is NetBSD only (at least in theory) and we likely can do the
right think without conditional build here.

> Who authored these patches? We don't seem to have a changelog, not
> even an author's name and email address, as required for any patch.
> 
> Here was my previous review, where I mentioned the ABI break:
> https://gcc.gnu.org/ml/libstdc++/2014-12/msg00069.html
> I didn't get a reply.
> 

Adding Maya, who can hopefully shed some light on it.


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

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 06/01/20 23:20 +0100, Kamil Rytarowski wrote:
>On 06.01.2020 16:24, Jonathan Wakely wrote:
>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>>> Hi Matthew,
>>>
>>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>>> have changed their ctype.h definition. It was updated in their intree
>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>>> straightforward rewrite. I've attached my own patch, but the file can
>>>> be obtained directly here:
>>>>
>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>>
>>>>
>>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>>> NetBSD headers (along with --disable-libcilkrts).
>>>
>>> I noticed this has not been applied yet, nor seen a follow-up?, and also
>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>>
>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>>> maintainers there will have a look.
>>>
>>> Gerald
>>
>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> index ff3ec893974..21eccf9fde1 100644
>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>   /// @brief  Base class for ctype.
>>>   struct ctype_base
>>>   {
>>> -    // Non-standard typedefs.
>>> -    typedef const unsigned char*    __to_type;
>>>
>>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>>     // on the mask type. Because of this, we don't use an enum.
>>> -    typedef unsigned char          mask;
>>>
>>> #ifndef _CTYPE_U
>>> -    static const mask upper        = _U;
>>> -    static const mask lower     = _L;
>>> -    static const mask alpha     = _U | _L;
>>> -    static const mask digit     = _N;
>>> -    static const mask xdigit     = _N | _X;
>>> -    static const mask space     = _S;
>>> -    static const mask print     = _P | _U | _L | _N | _B;
>>> -    static const mask graph     = _P | _U | _L | _N;
>>> -    static const mask cntrl     = _C;
>>> -    static const mask punct     = _P;
>>> -    static const mask alnum     = _U | _L | _N;
>>> +    // Non-standard typedefs.
>>> +    typedef const unsigned char*    __to_type;
>>> +
>>> +    typedef unsigned char    mask;
>>> +
>>> +    static const mask upper    = _U;
>>> +    static const mask lower    = _L;
>>> +    static const mask alpha    = _U | _L;
>>> +    static const mask digit    = _N;
>>> +    static const mask xdigit    = _N | _X;
>>> +    static const mask space    = _S;
>>> +    static const mask print    = _P | _U | _L | _N | _B;
>>> +    static const mask graph    = _P | _U | _L | _N;
>>> +    static const mask cntrl    = _C;
>>> +    static const mask punct    = _P;
>>> +    static const mask alnum    = _U | _L | _N;
>>> #else
>>> -    static const mask upper        = _CTYPE_U;
>>> -    static const mask lower     = _CTYPE_L;
>>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>>> -    static const mask digit     = _CTYPE_N;
>>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>>> -    static const mask space     = _CTYPE_S;
>>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>> _CTYPE_N | _CTYPE_B;
>>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>> _CTYPE_N;
>>> -    static const mask cntrl     = _CTYPE_C;
>>> -    static const mask punct     = _CTYPE_P;
>>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>>> +    typedef const unsigned short*    __to_type;
>>> +
>>> +    typedef unsigned short    mask;
>>
>> I seem to recall looking at this previously and noting that the change
>> to ctype_base::mask is an ABI break. It means that code compiled with
>> old versions of GCC or on old versions of NetBSD will not be ABI
>> compatible with code compiled by a new GCC on a new version of NetBSD.
>>
>> If the NetBSD maintainers are OK with that, then we can go ahead and
>> change it.
>>
>>
>
>We are fine with ABI breaks as we bump libstdc++ major on each upgrade
>in base.

That affects the libstdc++ built from NetBSD ports collection, but if
somebody builds GCC themselves using the upstream sources (which is
what these patches are trying to change) then they don't get that
bumped major version.

So if somebody builds GCC 9 themselves, and then builds GCC 10
themselves on the same machine, they'll get two versions of
libstdc++.so that are not ABI compatible.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 07/01/20 00:18 +0100, Kamil Rytarowski wrote:
>On 06.01.2020 16:34, Jonathan Wakely wrote:
>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>>> Hi Matthew,
>>>
>>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>>> have changed their ctype.h definition. It was updated in their intree
>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>>> straightforward rewrite. I've attached my own patch, but the file can
>>>> be obtained directly here:
>>>>
>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>>
>>>>
>>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>>> NetBSD headers (along with --disable-libcilkrts).
>>>
>>> I noticed this has not been applied yet, nor seen a follow-up?, and also
>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>>
>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>>> maintainers there will have a look.
>>>
>>> Gerald
>>
>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> index ff3ec893974..21eccf9fde1 100644
>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>   /// @brief  Base class for ctype.
>>>   struct ctype_base
>>>   {
>>> -    // Non-standard typedefs.
>>> -    typedef const unsigned char*    __to_type;
>>>
>>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>>     // on the mask type. Because of this, we don't use an enum.
>>> -    typedef unsigned char          mask;
>>>
>>> #ifndef _CTYPE_U
>>> -    static const mask upper        = _U;
>>> -    static const mask lower     = _L;
>>> -    static const mask alpha     = _U | _L;
>>> -    static const mask digit     = _N;
>>> -    static const mask xdigit     = _N | _X;
>>> -    static const mask space     = _S;
>>> -    static const mask print     = _P | _U | _L | _N | _B;
>>> -    static const mask graph     = _P | _U | _L | _N;
>>> -    static const mask cntrl     = _C;
>>> -    static const mask punct     = _P;
>>> -    static const mask alnum     = _U | _L | _N;
>>> +    // Non-standard typedefs.
>>> +    typedef const unsigned char*    __to_type;
>>> +
>>> +    typedef unsigned char    mask;
>>> +
>>> +    static const mask upper    = _U;
>>> +    static const mask lower    = _L;
>>> +    static const mask alpha    = _U | _L;
>>> +    static const mask digit    = _N;
>>> +    static const mask xdigit    = _N | _X;
>>> +    static const mask space    = _S;
>>> +    static const mask print    = _P | _U | _L | _N | _B;
>>> +    static const mask graph    = _P | _U | _L | _N;
>>> +    static const mask cntrl    = _C;
>>> +    static const mask punct    = _P;
>>> +    static const mask alnum    = _U | _L | _N;
>>> #else
>>> -    static const mask upper        = _CTYPE_U;
>>> -    static const mask lower     = _CTYPE_L;
>>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>>> -    static const mask digit     = _CTYPE_N;
>>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>>> -    static const mask space     = _CTYPE_S;
>>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>> _CTYPE_N | _CTYPE_B;
>>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>> _CTYPE_N;
>>> -    static const mask cntrl     = _CTYPE_C;
>>> -    static const mask punct     = _CTYPE_P;
>>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>>> +    typedef const unsigned short*    __to_type;
>>> +
>>> +    typedef unsigned short    mask;
>>> +
>>> +    static const mask upper    = _CTYPE_U;
>>> +    static const mask lower    = _CTYPE_L;
>>> +    static const mask alpha    = _CTYPE_A;
>>> +    static const mask digit    = _CTYPE_D;
>>> +    static const mask xdigit    = _CTYPE_X;
>>> +    static const mask space    = _CTYPE_S;
>>> +    static const mask print    = _CTYPE_R;
>>> +    static const mask graph    = _CTYPE_G;
>>> +    static const mask cntrl    = _CTYPE_C;
>>> +    static const mask punct    = _CTYPE_P;
>>> +    static const mask alnum    = _CTYPE_A | _CTYPE_D;
>>> #endif
>>> #if __cplusplus >= 201103L
>>> -    static const mask blank     = space;
>>> +    static const mask blank    = space;
>>> #endif
>>>   };
>>>
>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>> index ed3b7cd0d6a..33358e8f5d8 100644
>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>> @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>
>>> // Information as gleaned from /usr/include/ctype.h
>>>
>>> -  extern "C" const u_int8_t _C_ctype_[];
>>> -
>>>   const ctype_base::mask*
>>>   ctype<char>::classic_table() throw()
>>> -  { return _C_ctype_ + 1; }
>>> +  { return _C_ctype_tab_ + 1; }
>>
>> The first patch attached to PR 64271 (i.e.
>> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that
>> differently. Is it safe to make this change unconditionally?
>>
>
>This file is NetBSD only (at least in theory) and we likely can do the
>right think without conditional build here.

Yes, it's NetBSD-specific, but it's used for multiple versions of
NetBSD.

The patch in PR 64271 only uses _C_ctype_tab_+1 when _CTYPE_BL is
defined, i.e. for newer versions of NetBSD. For older versions, it
still uses _C_ctype_.

So my question is whether _C_ctype_tab_+1 does the right thing for all
versions of NetBSD, or only for newer ones.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 5416 bytes --]

On 07.01.2020 01:26, Jonathan Wakely wrote:
> On 06/01/20 23:20 +0100, Kamil Rytarowski wrote:
>> On 06.01.2020 16:24, Jonathan Wakely wrote:
>>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>>>> Hi Matthew,
>>>>
>>>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>>>> have changed their ctype.h definition. It was updated in their intree
>>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>>>> straightforward rewrite. I've attached my own patch, but the file can
>>>>> be obtained directly here:
>>>>>
>>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>
>>>>>
>>>>>
>>>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>>>> NetBSD headers (along with --disable-libcilkrts).
>>>>
>>>> I noticed this has not been applied yet, nor seen a follow-up?, and
>>>> also
>>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>>>
>>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>>>> maintainers there will have a look.
>>>>
>>>> Gerald
>>>
>>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> index ff3ec893974..21eccf9fde1 100644
>>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>   /// @brief  Base class for ctype.
>>>>   struct ctype_base
>>>>   {
>>>> -    // Non-standard typedefs.
>>>> -    typedef const unsigned char*    __to_type;
>>>>
>>>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>>>     // on the mask type. Because of this, we don't use an enum.
>>>> -    typedef unsigned char          mask;
>>>>
>>>> #ifndef _CTYPE_U
>>>> -    static const mask upper        = _U;
>>>> -    static const mask lower     = _L;
>>>> -    static const mask alpha     = _U | _L;
>>>> -    static const mask digit     = _N;
>>>> -    static const mask xdigit     = _N | _X;
>>>> -    static const mask space     = _S;
>>>> -    static const mask print     = _P | _U | _L | _N | _B;
>>>> -    static const mask graph     = _P | _U | _L | _N;
>>>> -    static const mask cntrl     = _C;
>>>> -    static const mask punct     = _P;
>>>> -    static const mask alnum     = _U | _L | _N;
>>>> +    // Non-standard typedefs.
>>>> +    typedef const unsigned char*    __to_type;
>>>> +
>>>> +    typedef unsigned char    mask;
>>>> +
>>>> +    static const mask upper    = _U;
>>>> +    static const mask lower    = _L;
>>>> +    static const mask alpha    = _U | _L;
>>>> +    static const mask digit    = _N;
>>>> +    static const mask xdigit    = _N | _X;
>>>> +    static const mask space    = _S;
>>>> +    static const mask print    = _P | _U | _L | _N | _B;
>>>> +    static const mask graph    = _P | _U | _L | _N;
>>>> +    static const mask cntrl    = _C;
>>>> +    static const mask punct    = _P;
>>>> +    static const mask alnum    = _U | _L | _N;
>>>> #else
>>>> -    static const mask upper        = _CTYPE_U;
>>>> -    static const mask lower     = _CTYPE_L;
>>>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>>>> -    static const mask digit     = _CTYPE_N;
>>>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>>>> -    static const mask space     = _CTYPE_S;
>>>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>> _CTYPE_N | _CTYPE_B;
>>>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>> _CTYPE_N;
>>>> -    static const mask cntrl     = _CTYPE_C;
>>>> -    static const mask punct     = _CTYPE_P;
>>>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>>>> +    typedef const unsigned short*    __to_type;
>>>> +
>>>> +    typedef unsigned short    mask;
>>>
>>> I seem to recall looking at this previously and noting that the change
>>> to ctype_base::mask is an ABI break. It means that code compiled with
>>> old versions of GCC or on old versions of NetBSD will not be ABI
>>> compatible with code compiled by a new GCC on a new version of NetBSD.
>>>
>>> If the NetBSD maintainers are OK with that, then we can go ahead and
>>> change it.
>>>
>>>
>>
>> We are fine with ABI breaks as we bump libstdc++ major on each upgrade
>> in base.
> 
> That affects the libstdc++ built from NetBSD ports collection, but if
> somebody builds GCC themselves using the upstream sources (which is
> what these patches are trying to change) then they don't get that
> bumped major version.
> 
> So if somebody builds GCC 9 themselves, and then builds GCC 10
> themselves on the same machine, they'll get two versions of
> libstdc++.so that are not ABI compatible.
> 
> 

I know, but we don't support this in NetBSD. We ship with our own major
numbers for GCC. If someone wants to maintain out of base or out of
pkgsrc it will have likely more problems than ABI breakage.


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

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 6926 bytes --]

On 07.01.2020 01:28, Jonathan Wakely wrote:
> On 07/01/20 00:18 +0100, Kamil Rytarowski wrote:
>> On 06.01.2020 16:34, Jonathan Wakely wrote:
>>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>>>> Hi Matthew,
>>>>
>>>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>>>> have changed their ctype.h definition. It was updated in their intree
>>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>>>> straightforward rewrite. I've attached my own patch, but the file can
>>>>> be obtained directly here:
>>>>>
>>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>
>>>>>
>>>>>
>>>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>>>> NetBSD headers (along with --disable-libcilkrts).
>>>>
>>>> I noticed this has not been applied yet, nor seen a follow-up?, and
>>>> also
>>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>>>
>>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>>>> maintainers there will have a look.
>>>>
>>>> Gerald
>>>
>>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> index ff3ec893974..21eccf9fde1 100644
>>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>   /// @brief  Base class for ctype.
>>>>   struct ctype_base
>>>>   {
>>>> -    // Non-standard typedefs.
>>>> -    typedef const unsigned char*    __to_type;
>>>>
>>>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>>>     // on the mask type. Because of this, we don't use an enum.
>>>> -    typedef unsigned char          mask;
>>>>
>>>> #ifndef _CTYPE_U
>>>> -    static const mask upper        = _U;
>>>> -    static const mask lower     = _L;
>>>> -    static const mask alpha     = _U | _L;
>>>> -    static const mask digit     = _N;
>>>> -    static const mask xdigit     = _N | _X;
>>>> -    static const mask space     = _S;
>>>> -    static const mask print     = _P | _U | _L | _N | _B;
>>>> -    static const mask graph     = _P | _U | _L | _N;
>>>> -    static const mask cntrl     = _C;
>>>> -    static const mask punct     = _P;
>>>> -    static const mask alnum     = _U | _L | _N;
>>>> +    // Non-standard typedefs.
>>>> +    typedef const unsigned char*    __to_type;
>>>> +
>>>> +    typedef unsigned char    mask;
>>>> +
>>>> +    static const mask upper    = _U;
>>>> +    static const mask lower    = _L;
>>>> +    static const mask alpha    = _U | _L;
>>>> +    static const mask digit    = _N;
>>>> +    static const mask xdigit    = _N | _X;
>>>> +    static const mask space    = _S;
>>>> +    static const mask print    = _P | _U | _L | _N | _B;
>>>> +    static const mask graph    = _P | _U | _L | _N;
>>>> +    static const mask cntrl    = _C;
>>>> +    static const mask punct    = _P;
>>>> +    static const mask alnum    = _U | _L | _N;
>>>> #else
>>>> -    static const mask upper        = _CTYPE_U;
>>>> -    static const mask lower     = _CTYPE_L;
>>>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>>>> -    static const mask digit     = _CTYPE_N;
>>>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>>>> -    static const mask space     = _CTYPE_S;
>>>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>> _CTYPE_N | _CTYPE_B;
>>>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>> _CTYPE_N;
>>>> -    static const mask cntrl     = _CTYPE_C;
>>>> -    static const mask punct     = _CTYPE_P;
>>>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>>>> +    typedef const unsigned short*    __to_type;
>>>> +
>>>> +    typedef unsigned short    mask;
>>>> +
>>>> +    static const mask upper    = _CTYPE_U;
>>>> +    static const mask lower    = _CTYPE_L;
>>>> +    static const mask alpha    = _CTYPE_A;
>>>> +    static const mask digit    = _CTYPE_D;
>>>> +    static const mask xdigit    = _CTYPE_X;
>>>> +    static const mask space    = _CTYPE_S;
>>>> +    static const mask print    = _CTYPE_R;
>>>> +    static const mask graph    = _CTYPE_G;
>>>> +    static const mask cntrl    = _CTYPE_C;
>>>> +    static const mask punct    = _CTYPE_P;
>>>> +    static const mask alnum    = _CTYPE_A | _CTYPE_D;
>>>> #endif
>>>> #if __cplusplus >= 201103L
>>>> -    static const mask blank     = space;
>>>> +    static const mask blank    = space;
>>>> #endif
>>>>   };
>>>>
>>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>>> index ed3b7cd0d6a..33358e8f5d8 100644
>>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
>>>> @@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>
>>>> // Information as gleaned from /usr/include/ctype.h
>>>>
>>>> -  extern "C" const u_int8_t _C_ctype_[];
>>>> -
>>>>   const ctype_base::mask*
>>>>   ctype<char>::classic_table() throw()
>>>> -  { return _C_ctype_ + 1; }
>>>> +  { return _C_ctype_tab_ + 1; }
>>>
>>> The first patch attached to PR 64271 (i.e.
>>> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34254 ) does that
>>> differently. Is it safe to make this change unconditionally?
>>>
>>
>> This file is NetBSD only (at least in theory) and we likely can do the
>> right think without conditional build here.
> 
> Yes, it's NetBSD-specific, but it's used for multiple versions of
> NetBSD.

This change was introduced in 2013 and we do not support such old
snapshots. If GCC wants to support older releases there it would be futile.

> 
> The patch in PR 64271 only uses _C_ctype_tab_+1 when _CTYPE_BL is
> defined, i.e. for newer versions of NetBSD. For older versions, it
> still uses _C_ctype_.
> 
> So my question is whether _C_ctype_tab_+1 does the right thing for all
> versions of NetBSD, or only for newer ones.
> 

The right thing today is to go unconditionally for _C_ctype_tab_ and
abandon _C_ctype_ entirely.

_C_ctype_ was u8 and did not support all inline semantics.

We still keep _C_ctype_ binary compat for the time being (until next
libc major bump), but it will go away in future (no timeline when).


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

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 07/01/20 01:30 +0100, Kamil Rytarowski wrote:
>On 07.01.2020 01:26, Jonathan Wakely wrote:
>> On 06/01/20 23:20 +0100, Kamil Rytarowski wrote:
>>> On 06.01.2020 16:24, Jonathan Wakely wrote:
>>>> On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>>>>> Hi Matthew,
>>>>>
>>>>> On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>>>>> The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>>>>> have changed their ctype.h definition. It was updated in their intree
>>>>>> libstdc++-v3 but not in the GCC one. My understanding is this is a
>>>>>> straightforward rewrite. I've attached my own patch, but the file can
>>>>>> be obtained directly here:
>>>>>>
>>>>>> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>>
>>>>>>
>>>>>>
>>>>>> With the attached patch, libstdc++-v3 can succesfully be built with
>>>>>> NetBSD headers (along with --disable-libcilkrts).
>>>>>
>>>>> I noticed this has not been applied yet, nor seen a follow-up?, and
>>>>> also
>>>>> noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>>>>
>>>>> Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>>>>> maintainers there will have a look.
>>>>>
>>>>> Gerald
>>>>
>>>>> diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>> b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>> index ff3ec893974..21eccf9fde1 100644
>>>>> --- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>> +++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>> @@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>>   /// @brief  Base class for ctype.
>>>>>   struct ctype_base
>>>>>   {
>>>>> -    // Non-standard typedefs.
>>>>> -    typedef const unsigned char*    __to_type;
>>>>>
>>>>>     // NB: Offsets into ctype<char>::_M_table force a particular size
>>>>>     // on the mask type. Because of this, we don't use an enum.
>>>>> -    typedef unsigned char          mask;
>>>>>
>>>>> #ifndef _CTYPE_U
>>>>> -    static const mask upper        = _U;
>>>>> -    static const mask lower     = _L;
>>>>> -    static const mask alpha     = _U | _L;
>>>>> -    static const mask digit     = _N;
>>>>> -    static const mask xdigit     = _N | _X;
>>>>> -    static const mask space     = _S;
>>>>> -    static const mask print     = _P | _U | _L | _N | _B;
>>>>> -    static const mask graph     = _P | _U | _L | _N;
>>>>> -    static const mask cntrl     = _C;
>>>>> -    static const mask punct     = _P;
>>>>> -    static const mask alnum     = _U | _L | _N;
>>>>> +    // Non-standard typedefs.
>>>>> +    typedef const unsigned char*    __to_type;
>>>>> +
>>>>> +    typedef unsigned char    mask;
>>>>> +
>>>>> +    static const mask upper    = _U;
>>>>> +    static const mask lower    = _L;
>>>>> +    static const mask alpha    = _U | _L;
>>>>> +    static const mask digit    = _N;
>>>>> +    static const mask xdigit    = _N | _X;
>>>>> +    static const mask space    = _S;
>>>>> +    static const mask print    = _P | _U | _L | _N | _B;
>>>>> +    static const mask graph    = _P | _U | _L | _N;
>>>>> +    static const mask cntrl    = _C;
>>>>> +    static const mask punct    = _P;
>>>>> +    static const mask alnum    = _U | _L | _N;
>>>>> #else
>>>>> -    static const mask upper        = _CTYPE_U;
>>>>> -    static const mask lower     = _CTYPE_L;
>>>>> -    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>>>>> -    static const mask digit     = _CTYPE_N;
>>>>> -    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>>>>> -    static const mask space     = _CTYPE_S;
>>>>> -    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>>> _CTYPE_N | _CTYPE_B;
>>>>> -    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>>> _CTYPE_N;
>>>>> -    static const mask cntrl     = _CTYPE_C;
>>>>> -    static const mask punct     = _CTYPE_P;
>>>>> -    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>>>>> +    typedef const unsigned short*    __to_type;
>>>>> +
>>>>> +    typedef unsigned short    mask;
>>>>
>>>> I seem to recall looking at this previously and noting that the change
>>>> to ctype_base::mask is an ABI break. It means that code compiled with
>>>> old versions of GCC or on old versions of NetBSD will not be ABI
>>>> compatible with code compiled by a new GCC on a new version of NetBSD.
>>>>
>>>> If the NetBSD maintainers are OK with that, then we can go ahead and
>>>> change it.
>>>>
>>>>
>>>
>>> We are fine with ABI breaks as we bump libstdc++ major on each upgrade
>>> in base.
>>
>> That affects the libstdc++ built from NetBSD ports collection, but if
>> somebody builds GCC themselves using the upstream sources (which is
>> what these patches are trying to change) then they don't get that
>> bumped major version.
>>
>> So if somebody builds GCC 9 themselves, and then builds GCC 10
>> themselves on the same machine, they'll get two versions of
>> libstdc++.so that are not ABI compatible.
>>
>>
>
>I know, but we don't support this in NetBSD. We ship with our own major
>numbers for GCC. If someone wants to maintain out of base or out of
>pkgsrc it will have likely more problems than ABI breakage.

But that's exactly what upstream GCC sources are (out of pkgsrc), and
that's where I'm being asked to include this patch.

NetBSD is free to not support it, of course, but we (GCC) have
different schedules and different priorities.

And if older versions of NetBSD are not supported, can we just drop
the `#ifndef _CTYPE_U` branch in config/os/bsd/netbsd/ctype_base.h ?

It seems wrong to keep support for old systems in one file and drop it
in another file.

I think we need the netbsd target maintainers (CC'd) to decide whether
GCC should still support older releases or drop that support for GCC
10. Usually I'd say we need a period of deprecation, but if GCC
doesn't currently build on NetBSD then maybe that's unnecessary.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 07/01/20 15:40 +0000, Jonathan Wakely wrote:
>On 07/01/20 01:30 +0100, Kamil Rytarowski wrote:
>>On 07.01.2020 01:26, Jonathan Wakely wrote:
>>>On 06/01/20 23:20 +0100, Kamil Rytarowski wrote:
>>>>On 06.01.2020 16:24, Jonathan Wakely wrote:
>>>>>On 22/12/19 09:36 +1000, Gerald Pfeifer wrote:
>>>>>>Hi Matthew,
>>>>>>
>>>>>>On Mon, 4 Feb 2019, Matthew Bauer wrote:
>>>>>>>The ctype_base.h file in libstdc++-v3 is out of date for NetBSD. They
>>>>>>>have changed their ctype.h definition. It was updated in their intree
>>>>>>>libstdc++-v3 but not in the GCC one. My understanding is this is a
>>>>>>>straightforward rewrite. I've attached my own patch, but the file can
>>>>>>>be obtained directly here:
>>>>>>>
>>>>>>>http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/dist/libstdc%2b%2b-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>With the attached patch, libstdc++-v3 can succesfully be built with
>>>>>>>NetBSD headers (along with --disable-libcilkrts).
>>>>>>
>>>>>>I noticed this has not been applied yet, nor seen a follow-up?, and
>>>>>>also
>>>>>>noticed it went to the gcc-patches list, but not libstdc++@gcc.gnu.org.
>>>>>>
>>>>>>Let me re-address this to libstdc++@gcc.gnu.org in the hope the
>>>>>>maintainers there will have a look.
>>>>>>
>>>>>>Gerald
>>>>>
>>>>>>diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>>b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>>index ff3ec893974..21eccf9fde1 100644
>>>>>>--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>>+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
>>>>>>@@ -38,40 +38,46 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>>>>>>  /// @brief  Base class for ctype.
>>>>>>  struct ctype_base
>>>>>>  {
>>>>>>-    // Non-standard typedefs.
>>>>>>-    typedef const unsigned char*    __to_type;
>>>>>>
>>>>>>    // NB: Offsets into ctype<char>::_M_table force a particular size
>>>>>>    // on the mask type. Because of this, we don't use an enum.
>>>>>>-    typedef unsigned char          mask;
>>>>>>
>>>>>>#ifndef _CTYPE_U
>>>>>>-    static const mask upper        = _U;
>>>>>>-    static const mask lower     = _L;
>>>>>>-    static const mask alpha     = _U | _L;
>>>>>>-    static const mask digit     = _N;
>>>>>>-    static const mask xdigit     = _N | _X;
>>>>>>-    static const mask space     = _S;
>>>>>>-    static const mask print     = _P | _U | _L | _N | _B;
>>>>>>-    static const mask graph     = _P | _U | _L | _N;
>>>>>>-    static const mask cntrl     = _C;
>>>>>>-    static const mask punct     = _P;
>>>>>>-    static const mask alnum     = _U | _L | _N;
>>>>>>+    // Non-standard typedefs.
>>>>>>+    typedef const unsigned char*    __to_type;
>>>>>>+
>>>>>>+    typedef unsigned char    mask;
>>>>>>+
>>>>>>+    static const mask upper    = _U;
>>>>>>+    static const mask lower    = _L;
>>>>>>+    static const mask alpha    = _U | _L;
>>>>>>+    static const mask digit    = _N;
>>>>>>+    static const mask xdigit    = _N | _X;
>>>>>>+    static const mask space    = _S;
>>>>>>+    static const mask print    = _P | _U | _L | _N | _B;
>>>>>>+    static const mask graph    = _P | _U | _L | _N;
>>>>>>+    static const mask cntrl    = _C;
>>>>>>+    static const mask punct    = _P;
>>>>>>+    static const mask alnum    = _U | _L | _N;
>>>>>>#else
>>>>>>-    static const mask upper        = _CTYPE_U;
>>>>>>-    static const mask lower     = _CTYPE_L;
>>>>>>-    static const mask alpha     = _CTYPE_U | _CTYPE_L;
>>>>>>-    static const mask digit     = _CTYPE_N;
>>>>>>-    static const mask xdigit     = _CTYPE_N | _CTYPE_X;
>>>>>>-    static const mask space     = _CTYPE_S;
>>>>>>-    static const mask print     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>>>>_CTYPE_N | _CTYPE_B;
>>>>>>-    static const mask graph     = _CTYPE_P | _CTYPE_U | _CTYPE_L |
>>>>>>_CTYPE_N;
>>>>>>-    static const mask cntrl     = _CTYPE_C;
>>>>>>-    static const mask punct     = _CTYPE_P;
>>>>>>-    static const mask alnum     = _CTYPE_U | _CTYPE_L | _CTYPE_N;
>>>>>>+    typedef const unsigned short*    __to_type;
>>>>>>+
>>>>>>+    typedef unsigned short    mask;
>>>>>
>>>>>I seem to recall looking at this previously and noting that the change
>>>>>to ctype_base::mask is an ABI break. It means that code compiled with
>>>>>old versions of GCC or on old versions of NetBSD will not be ABI
>>>>>compatible with code compiled by a new GCC on a new version of NetBSD.
>>>>>
>>>>>If the NetBSD maintainers are OK with that, then we can go ahead and
>>>>>change it.
>>>>>
>>>>>
>>>>
>>>>We are fine with ABI breaks as we bump libstdc++ major on each upgrade
>>>>in base.
>>>
>>>That affects the libstdc++ built from NetBSD ports collection, but if
>>>somebody builds GCC themselves using the upstream sources (which is
>>>what these patches are trying to change) then they don't get that
>>>bumped major version.
>>>
>>>So if somebody builds GCC 9 themselves, and then builds GCC 10
>>>themselves on the same machine, they'll get two versions of
>>>libstdc++.so that are not ABI compatible.
>>>
>>>
>>
>>I know, but we don't support this in NetBSD. We ship with our own major
>>numbers for GCC. If someone wants to maintain out of base or out of
>>pkgsrc it will have likely more problems than ABI breakage.
>
>But that's exactly what upstream GCC sources are (out of pkgsrc), and
>that's where I'm being asked to include this patch.
>
>NetBSD is free to not support it, of course, but we (GCC) have
>different schedules and different priorities.
>
>And if older versions of NetBSD are not supported, can we just drop
>the `#ifndef _CTYPE_U` branch in config/os/bsd/netbsd/ctype_base.h ?
>
>It seems wrong to keep support for old systems in one file and drop it
>in another file.

For Jason and Krister's benefit, that last comment was referring to
an earlier suggestion to not try to support old NetBSD releases, see
https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html

>I think we need the netbsd target maintainers (CC'd) to decide whether
>GCC should still support older releases or drop that support for GCC
>10. Usually I'd say we need a period of deprecation, but if GCC
>doesn't currently build on NetBSD then maybe that's unnecessary.
>
>
>

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jason Thorpe]

> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> 
> For Jason and Krister's benefit, that last comment was referring to
> an earlier suggestion to not try to support old NetBSD releases, see
> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html
> 
>> I think we need the netbsd target maintainers (CC'd) to decide whether
>> GCC should still support older releases or drop that support for GCC
>> 10. Usually I'd say we need a period of deprecation, but if GCC
>> doesn't currently build on NetBSD then maybe that's unnecessary.

The affected NetBSD versions are NetBSD 6 and earlier, which are EOL from the NetBSD perspective, so I think this is OK.

-- thorpej

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

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

On 07/01/20 12:44 -0800, Jason Thorpe wrote:
>
>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
>>
>> For Jason and Krister's benefit, that last comment was referring to
>> an earlier suggestion to not try to support old NetBSD releases, see
>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html
>>
>>> I think we need the netbsd target maintainers (CC'd) to decide whether
>>> GCC should still support older releases or drop that support for GCC
>>> 10. Usually I'd say we need a period of deprecation, but if GCC
>>> doesn't currently build on NetBSD then maybe that's unnecessary.
>
>The affected NetBSD versions are NetBSD 6 and earlier, which are EOL from the NetBSD perspective, so I think this is OK.

So is this patch OK then?

Could somebody please test it on NetBSD? (Ideally on the oldest
supported release, but anything is better than nothing).

This differs from the patches posted by using _CTYPE_BL for the
isblank class, which seems better than using _CTYPE_S.



[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 5678 bytes --]

commit 67c5a9a8c7e689bb2208d12e5459c1e207cbbd4c
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Fri Jan 10 16:01:19 2020 +0000

    libstdc++: std::ctype fixes for recent versions of NetBSD
    
    This removes support for EOL versions of NetBSD and syncs the
    definitions with patches from NetBSD upstream.
    
    2020-01-10  Kai-Uwe Eckhardt  <kuehro@gmx.de>
                Matthew Bauer  <mjbauer95@gmail.com>
                Jonathan Wakely  <jwakely@redhat.com>
    
            PR bootstrap/64271 (partial)
            * config/os/bsd/netbsd/ctype_base.h (ctype_base::mask): Change type
            to unsigned short.
            (ctype_base::alpha, ctype_base::digit, ctype_base::xdigit)
            (ctype_base::print, ctype_base::graph, ctype_base::alnum): Sync
            definitions with NetBSD upstream.
            (ctype_base::blank): Use _CTYPE_BL.
            * config/os/bsd/netbsd/ctype_configure_char.cc (_C_ctype_): Remove
            Declaration.
            (ctype<char>::classic_table): Use _C_ctype_tab_ instead of _C_ctype_.
            (ctype<char>::do_toupper, ctype<char>::do_tolower): Cast char
            parameters to unsigned char.
            * config/os/bsd/netbsd/ctype_inline.h (ctype<char>::is): Likewise.

diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
index 340bd8e3d4e..28bf232ba83 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_base.h
@@ -43,35 +43,21 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
     // NB: Offsets into ctype<char>::_M_table force a particular size
     // on the mask type. Because of this, we don't use an enum.
-    typedef unsigned char      	mask;
+    typedef unsigned short      	mask;
 
-#ifndef _CTYPE_U
-    static const mask upper    	= _U;
-    static const mask lower 	= _L;
-    static const mask alpha 	= _U | _L;
-    static const mask digit 	= _N;
-    static const mask xdigit 	= _N | _X;
-    static const mask space 	= _S;
-    static const mask print 	= _P | _U | _L | _N | _B;
-    static const mask graph 	= _P | _U | _L | _N;
-    static const mask cntrl 	= _C;
-    static const mask punct 	= _P;
-    static const mask alnum 	= _U | _L | _N;
-#else
-    static const mask upper    	= _CTYPE_U;
-    static const mask lower 	= _CTYPE_L;
-    static const mask alpha 	= _CTYPE_U | _CTYPE_L;
-    static const mask digit 	= _CTYPE_N;
-    static const mask xdigit 	= _CTYPE_N | _CTYPE_X;
-    static const mask space 	= _CTYPE_S;
-    static const mask print 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N | _CTYPE_B;
-    static const mask graph 	= _CTYPE_P | _CTYPE_U | _CTYPE_L | _CTYPE_N;
-    static const mask cntrl 	= _CTYPE_C;
-    static const mask punct 	= _CTYPE_P;
-    static const mask alnum 	= _CTYPE_U | _CTYPE_L | _CTYPE_N;
-#endif
+    static const mask upper	= _CTYPE_U;
+    static const mask lower	= _CTYPE_L;
+    static const mask alpha	= _CTYPE_A;
+    static const mask digit	= _CTYPE_D;
+    static const mask xdigit	= _CTYPE_X;
+    static const mask space	= _CTYPE_S;
+    static const mask print	= _CTYPE_R;
+    static const mask graph	= _CTYPE_G;
+    static const mask cntrl	= _CTYPE_C;
+    static const mask punct	= _CTYPE_P;
+    static const mask alnum	= _CTYPE_A | _CTYPE_D;
 #if __cplusplus >= 201103L
-    static const mask blank 	= space;
+    static const mask blank	= _CTYPE_BL;
 #endif
   };
 
diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
index 0680bb350c3..86f7828517d 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_configure_char.cc
@@ -38,11 +38,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
 // Information as gleaned from /usr/include/ctype.h
 
-  extern "C" const u_int8_t _C_ctype_[];
-
   const ctype_base::mask*
   ctype<char>::classic_table() throw()
-  { return _C_ctype_ + 1; }
+  { return _C_ctype_tab_ + 1; }
 
   ctype<char>::ctype(__c_locale, const mask* __table, bool __del,
 		     size_t __refs)
@@ -69,14 +67,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   char
   ctype<char>::do_toupper(char __c) const
-  { return ::toupper((int) __c); }
+  { return ::toupper((int)(unsigned char) __c); }
 
   const char*
   ctype<char>::do_toupper(char* __low, const char* __high) const
   {
     while (__low < __high)
       {
-	*__low = ::toupper((int) *__low);
+	*__low = ::toupper((int)(unsigned char) *__low);
 	++__low;
       }
     return __high;
@@ -84,14 +82,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   char
   ctype<char>::do_tolower(char __c) const
-  { return ::tolower((int) __c); }
+  { return ::tolower((int)(unsigned char) __c); }
 
   const char*
   ctype<char>::do_tolower(char* __low, const char* __high) const
   {
     while (__low < __high)
       {
-	*__low = ::tolower((int) *__low);
+	*__low = ::tolower((int)(unsigned char) *__low);
 	++__low;
       }
     return __high;
diff --git a/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h b/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h
index 6395433e1ac..4536690b6d8 100644
--- a/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h
+++ b/libstdc++-v3/config/os/bsd/netbsd/ctype_inline.h
@@ -48,7 +48,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   is(const char* __low, const char* __high, mask* __vec) const
   {
     while (__low < __high)
-      *__vec++ = _M_table[*__low++];
+      *__vec++ = _M_table[(unsigned char)*__low++];
     return __high;
   }
 

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1477 bytes --]

On 10.01.2020 17:11, Jonathan Wakely wrote:
> On 07/01/20 12:44 -0800, Jason Thorpe wrote:
>>
>>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>
>>> For Jason and Krister's benefit, that last comment was referring to
>>> an earlier suggestion to not try to support old NetBSD releases, see
>>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html
>>>
>>>> I think we need the netbsd target maintainers (CC'd) to decide whether
>>>> GCC should still support older releases or drop that support for GCC
>>>> 10. Usually I'd say we need a period of deprecation, but if GCC
>>>> doesn't currently build on NetBSD then maybe that's unnecessary.
>>
>> The affected NetBSD versions are NetBSD 6 and earlier, which are EOL
>> from the NetBSD perspective, so I think this is OK.
> 
> So is this patch OK then?
> 

Looks good to me.

> Could somebody please test it on NetBSD? (Ideally on the oldest
> supported release, but anything is better than nothing).
> 

Works for me on:

$ uname -a
NetBSD chieftec 9.99.37 NetBSD 9.99.37 (GENERIC) #5: Mon Jan 13 15:39:58
CET 2020
root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64

I see no reason why would it break on older releases, but I don't have
them handy for tests.

> This differs from the patches posted by using _CTYPE_BL for the
> isblank class, which seems better than using _CTYPE_S.
> 
> 

_CTYPE_BL is the right bit for isblank().


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

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jason Thorpe]

> On Jan 14, 2020, at 8:49 AM, Kamil Rytarowski <n54@gmx.com> wrote:
> 
> On 10.01.2020 17:11, Jonathan Wakely wrote:
>> On 07/01/20 12:44 -0800, Jason Thorpe wrote:
>>> 
>>>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>> 
>>>> For Jason and Krister's benefit, that last comment was referring to
>>>> an earlier suggestion to not try to support old NetBSD releases, see
>>>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html
>>>> 
>>>>> I think we need the netbsd target maintainers (CC'd) to decide whether
>>>>> GCC should still support older releases or drop that support for GCC
>>>>> 10. Usually I'd say we need a period of deprecation, but if GCC
>>>>> doesn't currently build on NetBSD then maybe that's unnecessary.
>>> 
>>> The affected NetBSD versions are NetBSD 6 and earlier, which are EOL
>>> from the NetBSD perspective, so I think this is OK.
>> 
>> So is this patch OK then?
>> 
> 
> Looks good to me.

Now that we have Kamil's confirmation that it's working for him, it also gets a thumbs-up from me.

Thanks!

> 
>> Could somebody please test it on NetBSD? (Ideally on the oldest
>> supported release, but anything is better than nothing).
>> 
> 
> Works for me on:
> 
> $ uname -a
> NetBSD chieftec 9.99.37 NetBSD 9.99.37 (GENERIC) #5: Mon Jan 13 15:39:58
> CET 2020
> root@chieftec:/public/netbsd-root/sys/arch/amd64/compile/GENERIC amd64
> 
> I see no reason why would it break on older releases, but I don't have
> them handy for tests.
> 
>> This differs from the patches posted by using _CTYPE_BL for the
>> isblank class, which seems better than using _CTYPE_S.
>> 
>> 
> 
> _CTYPE_BL is the right bit for isblank().
> 

-- thorpej

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jonathan Wakely]

On 14/01/20 08:56 -0800, Jason Thorpe wrote:
>
>
>> On Jan 14, 2020, at 8:49 AM, Kamil Rytarowski <n54@gmx.com> wrote:
>>
>> On 10.01.2020 17:11, Jonathan Wakely wrote:
>>> On 07/01/20 12:44 -0800, Jason Thorpe wrote:
>>>>
>>>>> On Jan 7, 2020, at 7:43 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
>>>>>
>>>>> For Jason and Krister's benefit, that last comment was referring to
>>>>> an earlier suggestion to not try to support old NetBSD releases, see
>>>>> https://gcc.gnu.org/ml/libstdc++/2020-01/msg00026.html
>>>>>
>>>>>> I think we need the netbsd target maintainers (CC'd) to decide whether
>>>>>> GCC should still support older releases or drop that support for GCC
>>>>>> 10. Usually I'd say we need a period of deprecation, but if GCC
>>>>>> doesn't currently build on NetBSD then maybe that's unnecessary.
>>>>
>>>> The affected NetBSD versions are NetBSD 6 and earlier, which are EOL
>>>> from the NetBSD perspective, so I think this is OK.
>>>
>>> So is this patch OK then?
>>>
>>
>> Looks good to me.
>
>Now that we have Kamil's confirmation that it's working for him, it also gets a thumbs-up from me.
>
>Thanks!

Thanks to everyone for their feedback. I've pushed the patch to GCC
master now (I think it's reasonable to consider "doesn't build on
NetBSD anymore" as a regression and so fixable at this point in GCC's
development).

Please let me know of any problems you find with the changes.

Re: [PATCH] libstdcxx: Update ctype_base.h from NetBSD upstream

[Jason Thorpe]

> On Jan 16, 2020, at 9:24 AM, Jonathan Wakely <jwakely@redhat.com> wrote:
> 
> Thanks to everyone for their feedback. I've pushed the patch to GCC
> master now (I think it's reasonable to consider "doesn't build on
> NetBSD anymore" as a regression and so fixable at this point in GCC's
> development).
> 
> Please let me know of any problems you find with the changes.

Great, thanks Jonathan!

-- thorpej