public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
@ 2023-09-28 16:37 Dimitry Andric
  2023-10-26 17:00 ` [PING][PATCH] " Dimitry Andric
  2024-01-30 16:19 ` [PATCH] " Jonathan Wakely
  0 siblings, 2 replies; 7+ messages in thread
From: Dimitry Andric @ 2023-09-28 16:37 UTC (permalink / raw)
  To: gcc-patches; +Cc: Gerald Pfeifer

Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632

When building gcc's C++ sources against recent libc++, the poisoning of
the ctype macros due to including safe-ctype.h before including C++
standard headers such as <list>, <map>, etc, causes many compilation
errors, similar to:

 In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
 In file included from /home/dim/src/gcc/master/gcc/system.h:233:
 In file included from /usr/include/c++/v1/vector:321:
 In file included from
 /usr/include/c++/v1/__format/formatter_bool.h:20:
 In file included from
 /usr/include/c++/v1/__format/formatter_integral.h:32:
 In file included from /usr/include/c++/v1/locale:202:
 /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
 only applies to structs, variables, functions, and namespaces
   546 |     _LIBCPP_INLINE_VISIBILITY
       |     ^
 /usr/include/c++/v1/__config:813:37: note: expanded from macro
 '_LIBCPP_INLINE_VISIBILITY'
   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
       |                                     ^
 /usr/include/c++/v1/__config:792:26: note: expanded from macro
 '_LIBCPP_HIDE_FROM_ABI'
   792 |
   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
 _LIBCPP_VERSIONED_IDENTIFIER))))
       |                          ^
 In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
 In file included from /home/dim/src/gcc/master/gcc/system.h:233:
 In file included from /usr/include/c++/v1/vector:321:
 In file included from
 /usr/include/c++/v1/__format/formatter_bool.h:20:
 In file included from
 /usr/include/c++/v1/__format/formatter_integral.h:32:
 In file included from /usr/include/c++/v1/locale:202:
 /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
 declaration list
   547 |     char_type toupper(char_type __c) const
       |                                     ^
 /usr/include/c++/v1/__locale:553:48: error: too many arguments
 provided to function-like macro invocation
   553 |     const char_type* toupper(char_type* __low, const
   char_type* __high) const
       |                                                ^
 /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
 macro 'toupper' defined here
   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
       |         ^

This is because libc++ uses different transitive includes than
libstdc++, and some of those transitive includes pull in various ctype
declarations (typically via <locale>).

There was already a special case for including <string> before
safe-ctype.h, so move the rest of the C++ standard header includes to
the same location, to fix the problem.

Signed-off-by: Dimitry Andric <dimitry@andric.com>
---
gcc/system.h | 39 ++++++++++++++++++---------------------
1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/gcc/system.h b/gcc/system.h
index e924152ad4c..7a516b11438 100644
--- a/gcc/system.h
+++ b/gcc/system.h
@@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
#undef fread_unlocked
#undef fwrite_unlocked

-/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
-   the ctype macros through safe-ctype.h */
-
-#ifdef __cplusplus
-#ifdef INCLUDE_STRING
-# include <string>
-#endif
-#endif
-
-/* There are an extraordinary number of issues with <ctype.h>.
-   The last straw is that it varies with the locale.  Use libiberty's
-   replacement instead.  */
-#include "safe-ctype.h"
-
-#include <sys/types.h>
-
-#include <errno.h>
-
-#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
-extern int errno;
-#endif
+/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
+   poisoning the ctype macros through safe-ctype.h */

#ifdef __cplusplus
#if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
@@ -229,6 +210,9 @@ extern int errno;
#ifdef INCLUDE_SET
# include <set>
#endif
+#ifdef INCLUDE_STRING
+# include <string>
+#endif
#ifdef INCLUDE_VECTOR
# include <vector>
#endif
@@ -245,6 +229,19 @@ extern int errno;
# include <type_traits>
#endif

+/* There are an extraordinary number of issues with <ctype.h>.
+   The last straw is that it varies with the locale.  Use libiberty's
+   replacement instead.  */
+#include "safe-ctype.h"
+
+#include <sys/types.h>
+
+#include <errno.h>
+
+#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
+extern int errno;
+#endif
+
/* Some of glibc's string inlines cause warnings.  Plus we'd rather
   rely on (and therefore test) GCC's string builtins.  */
#define __NO_STRING_INLINES
-- 
2.42.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2023-09-28 16:37 [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning Dimitry Andric
@ 2023-10-26 17:00 ` Dimitry Andric
  2023-10-29 13:08   ` Sam James
  2024-01-30 16:19 ` [PATCH] " Jonathan Wakely
  1 sibling, 1 reply; 7+ messages in thread
From: Dimitry Andric @ 2023-10-26 17:00 UTC (permalink / raw)
  To: gcc-patches; +Cc: Gerald Pfeifer

Ping. It would be nice to get this QoL fix in.

-Dimitry

> On 28 Sep 2023, at 18:37, Dimitry Andric <dimitry@andric.com> wrote:
> 
> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
> 
> When building gcc's C++ sources against recent libc++, the poisoning of
> the ctype macros due to including safe-ctype.h before including C++
> standard headers such as <list>, <map>, etc, causes many compilation
> errors, similar to:
> 
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
> only applies to structs, variables, functions, and namespaces
>   546 |     _LIBCPP_INLINE_VISIBILITY
>       |     ^
> /usr/include/c++/v1/__config:813:37: note: expanded from macro
> '_LIBCPP_INLINE_VISIBILITY'
>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>       |                                     ^
> /usr/include/c++/v1/__config:792:26: note: expanded from macro
> '_LIBCPP_HIDE_FROM_ABI'
>   792 |
>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
> _LIBCPP_VERSIONED_IDENTIFIER))))
>       |                          ^
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
> declaration list
>   547 |     char_type toupper(char_type __c) const
>       |                                     ^
> /usr/include/c++/v1/__locale:553:48: error: too many arguments
> provided to function-like macro invocation
>   553 |     const char_type* toupper(char_type* __low, const
>   char_type* __high) const
>       |                                                ^
> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
> macro 'toupper' defined here
>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>       |         ^
> 
> This is because libc++ uses different transitive includes than
> libstdc++, and some of those transitive includes pull in various ctype
> declarations (typically via <locale>).
> 
> There was already a special case for including <string> before
> safe-ctype.h, so move the rest of the C++ standard header includes to
> the same location, to fix the problem.
> 
> Signed-off-by: Dimitry Andric <dimitry@andric.com>
> ---
> gcc/system.h | 39 ++++++++++++++++++---------------------
> 1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/gcc/system.h b/gcc/system.h
> index e924152ad4c..7a516b11438 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
> #undef fread_unlocked
> #undef fwrite_unlocked
> 
> -/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
> -   the ctype macros through safe-ctype.h */
> -
> -#ifdef __cplusplus
> -#ifdef INCLUDE_STRING
> -# include <string>
> -#endif
> -#endif
> -
> -/* There are an extraordinary number of issues with <ctype.h>.
> -   The last straw is that it varies with the locale.  Use libiberty's
> -   replacement instead.  */
> -#include "safe-ctype.h"
> -
> -#include <sys/types.h>
> -
> -#include <errno.h>
> -
> -#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
> -extern int errno;
> -#endif
> +/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
> +   poisoning the ctype macros through safe-ctype.h */
> 
> #ifdef __cplusplus
> #if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
> @@ -229,6 +210,9 @@ extern int errno;
> #ifdef INCLUDE_SET
> # include <set>
> #endif
> +#ifdef INCLUDE_STRING
> +# include <string>
> +#endif
> #ifdef INCLUDE_VECTOR
> # include <vector>
> #endif
> @@ -245,6 +229,19 @@ extern int errno;
> # include <type_traits>
> #endif
> 
> +/* There are an extraordinary number of issues with <ctype.h>.
> +   The last straw is that it varies with the locale.  Use libiberty's
> +   replacement instead.  */
> +#include "safe-ctype.h"
> +
> +#include <sys/types.h>
> +
> +#include <errno.h>
> +
> +#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
> +extern int errno;
> +#endif
> +
> /* Some of glibc's string inlines cause warnings.  Plus we'd rather
>   rely on (and therefore test) GCC's string builtins.  */
> #define __NO_STRING_INLINES
> -- 
> 2.42.0
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2023-10-26 17:00 ` [PING][PATCH] " Dimitry Andric
@ 2023-10-29 13:08   ` Sam James
  2024-01-20  9:17     ` Sam James
  0 siblings, 1 reply; 7+ messages in thread
From: Sam James @ 2023-10-29 13:08 UTC (permalink / raw)
  To: Dimitry Andric; +Cc: Gerald Pfeifer, gcc-patches, Jeff Law


Dimitry Andric <dimitry@andric.com> writes:

> Ping. It would be nice to get this QoL fix in.
>

Yes please - we've been using this in Gentoo since around when it was
first posted. No complaints.

I cannot approve but it looks good to me.

> -Dimitry
>
>> On 28 Sep 2023, at 18:37, Dimitry Andric <dimitry@andric.com> wrote:
>> 
>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>> 
>> When building gcc's C++ sources against recent libc++, the poisoning of
>> the ctype macros due to including safe-ctype.h before including C++
>> standard headers such as <list>, <map>, etc, causes many compilation
>> errors, similar to:
>> 
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>> only applies to structs, variables, functions, and namespaces
>>   546 |     _LIBCPP_INLINE_VISIBILITY
>>       |     ^
>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>> '_LIBCPP_INLINE_VISIBILITY'
>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>       |                                     ^
>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>> '_LIBCPP_HIDE_FROM_ABI'
>>   792 |
>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>> _LIBCPP_VERSIONED_IDENTIFIER))))
>>       |                          ^
>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>> In file included from /usr/include/c++/v1/vector:321:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>> In file included from
>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>> In file included from /usr/include/c++/v1/locale:202:
>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>> declaration list
>>   547 |     char_type toupper(char_type __c) const
>>       |                                     ^
>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>> provided to function-like macro invocation
>>   553 |     const char_type* toupper(char_type* __low, const
>>   char_type* __high) const
>>       |                                                ^
>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>> macro 'toupper' defined here
>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>       |         ^
>> 
>> This is because libc++ uses different transitive includes than
>> libstdc++, and some of those transitive includes pull in various ctype
>> declarations (typically via <locale>).
>> 
>> There was already a special case for including <string> before
>> safe-ctype.h, so move the rest of the C++ standard header includes to
>> the same location, to fix the problem.
>> 
>> Signed-off-by: Dimitry Andric <dimitry@andric.com>
>> ---
>> gcc/system.h | 39 ++++++++++++++++++---------------------
>> 1 file changed, 18 insertions(+), 21 deletions(-)
>> 
>> diff --git a/gcc/system.h b/gcc/system.h
>> index e924152ad4c..7a516b11438 100644
>> --- a/gcc/system.h
>> +++ b/gcc/system.h
>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>> #undef fread_unlocked
>> #undef fwrite_unlocked
>> 
>> -/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
>> -   the ctype macros through safe-ctype.h */
>> -
>> -#ifdef __cplusplus
>> -#ifdef INCLUDE_STRING
>> -# include <string>
>> -#endif
>> -#endif
>> -
>> -/* There are an extraordinary number of issues with <ctype.h>.
>> -   The last straw is that it varies with the locale.  Use libiberty's
>> -   replacement instead.  */
>> -#include "safe-ctype.h"
>> -
>> -#include <sys/types.h>
>> -
>> -#include <errno.h>
>> -
>> -#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>> -extern int errno;
>> -#endif
>> +/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>> +   poisoning the ctype macros through safe-ctype.h */
>> 
>> #ifdef __cplusplus
>> #if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>> @@ -229,6 +210,9 @@ extern int errno;
>> #ifdef INCLUDE_SET
>> # include <set>
>> #endif
>> +#ifdef INCLUDE_STRING
>> +# include <string>
>> +#endif
>> #ifdef INCLUDE_VECTOR
>> # include <vector>
>> #endif
>> @@ -245,6 +229,19 @@ extern int errno;
>> # include <type_traits>
>> #endif
>> 
>> +/* There are an extraordinary number of issues with <ctype.h>.
>> +   The last straw is that it varies with the locale.  Use libiberty's
>> +   replacement instead.  */
>> +#include "safe-ctype.h"
>> +
>> +#include <sys/types.h>
>> +
>> +#include <errno.h>
>> +
>> +#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>> +extern int errno;
>> +#endif
>> +
>> /* Some of glibc's string inlines cause warnings.  Plus we'd rather
>>   rely on (and therefore test) GCC's string builtins.  */
>> #define __NO_STRING_INLINES
>> -- 
>> 2.42.0
>> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2023-10-29 13:08   ` Sam James
@ 2024-01-20  9:17     ` Sam James
  2024-01-30 15:07       ` Sam James
  0 siblings, 1 reply; 7+ messages in thread
From: Sam James @ 2024-01-20  9:17 UTC (permalink / raw)
  To: Sam James; +Cc: Dimitry Andric, Gerald Pfeifer, gcc-patches, Jeff Law


Sam James <sam@gentoo.org> writes:

> Dimitry Andric <dimitry@andric.com> writes:
>
>> Ping. It would be nice to get this QoL fix in.
>>
>
> Yes please - we've been using this in Gentoo since around when it was
> first posted. No complaints.
>
> I cannot approve but it looks good to me.

Ping.

>
>> -Dimitry
>>
>>> On 28 Sep 2023, at 18:37, Dimitry Andric <dimitry@andric.com> wrote:
>>> 
>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>> 
>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>> the ctype macros due to including safe-ctype.h before including C++
>>> standard headers such as <list>, <map>, etc, causes many compilation
>>> errors, similar to:
>>> 
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>> only applies to structs, variables, functions, and namespaces
>>>   546 |     _LIBCPP_INLINE_VISIBILITY
>>>       |     ^
>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>> '_LIBCPP_INLINE_VISIBILITY'
>>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>>       |                                     ^
>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>> '_LIBCPP_HIDE_FROM_ABI'
>>>   792 |
>>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>> _LIBCPP_VERSIONED_IDENTIFIER))))
>>>       |                          ^
>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>> In file included from /usr/include/c++/v1/vector:321:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>> In file included from
>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>> In file included from /usr/include/c++/v1/locale:202:
>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>> declaration list
>>>   547 |     char_type toupper(char_type __c) const
>>>       |                                     ^
>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>> provided to function-like macro invocation
>>>   553 |     const char_type* toupper(char_type* __low, const
>>>   char_type* __high) const
>>>       |                                                ^
>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>> macro 'toupper' defined here
>>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>>       |         ^
>>> 
>>> This is because libc++ uses different transitive includes than
>>> libstdc++, and some of those transitive includes pull in various ctype
>>> declarations (typically via <locale>).
>>> 
>>> There was already a special case for including <string> before
>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>> the same location, to fix the problem.
>>> 
>>> Signed-off-by: Dimitry Andric <dimitry@andric.com>
>>> ---
>>> gcc/system.h | 39 ++++++++++++++++++---------------------
>>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>> 
>>> diff --git a/gcc/system.h b/gcc/system.h
>>> index e924152ad4c..7a516b11438 100644
>>> --- a/gcc/system.h
>>> +++ b/gcc/system.h
>>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>>> #undef fread_unlocked
>>> #undef fwrite_unlocked
>>> 
>>> -/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
>>> -   the ctype macros through safe-ctype.h */
>>> -
>>> -#ifdef __cplusplus
>>> -#ifdef INCLUDE_STRING
>>> -# include <string>
>>> -#endif
>>> -#endif
>>> -
>>> -/* There are an extraordinary number of issues with <ctype.h>.
>>> -   The last straw is that it varies with the locale.  Use libiberty's
>>> -   replacement instead.  */
>>> -#include "safe-ctype.h"
>>> -
>>> -#include <sys/types.h>
>>> -
>>> -#include <errno.h>
>>> -
>>> -#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>>> -extern int errno;
>>> -#endif
>>> +/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>>> +   poisoning the ctype macros through safe-ctype.h */
>>> 
>>> #ifdef __cplusplus
>>> #if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>>> @@ -229,6 +210,9 @@ extern int errno;
>>> #ifdef INCLUDE_SET
>>> # include <set>
>>> #endif
>>> +#ifdef INCLUDE_STRING
>>> +# include <string>
>>> +#endif
>>> #ifdef INCLUDE_VECTOR
>>> # include <vector>
>>> #endif
>>> @@ -245,6 +229,19 @@ extern int errno;
>>> # include <type_traits>
>>> #endif
>>> 
>>> +/* There are an extraordinary number of issues with <ctype.h>.
>>> +   The last straw is that it varies with the locale.  Use libiberty's
>>> +   replacement instead.  */
>>> +#include "safe-ctype.h"
>>> +
>>> +#include <sys/types.h>
>>> +
>>> +#include <errno.h>
>>> +
>>> +#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>>> +extern int errno;
>>> +#endif
>>> +
>>> /* Some of glibc's string inlines cause warnings.  Plus we'd rather
>>>   rely on (and therefore test) GCC's string builtins.  */
>>> #define __NO_STRING_INLINES
>>> -- 
>>> 2.42.0
>>> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-01-20  9:17     ` Sam James
@ 2024-01-30 15:07       ` Sam James
  2024-01-30 20:27         ` Sam James
  0 siblings, 1 reply; 7+ messages in thread
From: Sam James @ 2024-01-30 15:07 UTC (permalink / raw)
  To: Sam James
  Cc: Dimitry Andric, Gerald Pfeifer, gcc-patches, Jeff Law,
	Arsen Arsenović

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


Sam James <sam@gentoo.org> writes:

> Sam James <sam@gentoo.org> writes:
>
>> Dimitry Andric <dimitry@andric.com> writes:
>>
>>> Ping. It would be nice to get this QoL fix in.
>>>
>>
>> Yes please - we've been using this in Gentoo since around when it was
>> first posted. No complaints.
>>
>> I cannot approve but it looks good to me.
>
> Ping.

Ping. This is straightforward, I just can't merge it myself.

>
>>
>>> -Dimitry
>>>
>>>> On 28 Sep 2023, at 18:37, Dimitry Andric <dimitry@andric.com> wrote:
>>>> 
>>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>>> 
>>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>>> the ctype macros due to including safe-ctype.h before including C++
>>>> standard headers such as <list>, <map>, etc, causes many compilation
>>>> errors, similar to:
>>>> 
>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>> In file included from /usr/include/c++/v1/vector:321:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>> In file included from /usr/include/c++/v1/locale:202:
>>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>>> only applies to structs, variables, functions, and namespaces
>>>>   546 |     _LIBCPP_INLINE_VISIBILITY
>>>>       |     ^
>>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>>> '_LIBCPP_INLINE_VISIBILITY'
>>>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>>>       |                                     ^
>>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>>> '_LIBCPP_HIDE_FROM_ABI'
>>>>   792 |
>>>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>>> _LIBCPP_VERSIONED_IDENTIFIER))))
>>>>       |                          ^
>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>> In file included from /usr/include/c++/v1/vector:321:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>> In file included from
>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>> In file included from /usr/include/c++/v1/locale:202:
>>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>>> declaration list
>>>>   547 |     char_type toupper(char_type __c) const
>>>>       |                                     ^
>>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>>> provided to function-like macro invocation
>>>>   553 |     const char_type* toupper(char_type* __low, const
>>>>   char_type* __high) const
>>>>       |                                                ^
>>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>>> macro 'toupper' defined here
>>>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>>>       |         ^
>>>> 
>>>> This is because libc++ uses different transitive includes than
>>>> libstdc++, and some of those transitive includes pull in various ctype
>>>> declarations (typically via <locale>).
>>>> 
>>>> There was already a special case for including <string> before
>>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>>> the same location, to fix the problem.
>>>> 
>>>> Signed-off-by: Dimitry Andric <dimitry@andric.com>
>>>> ---
>>>> gcc/system.h | 39 ++++++++++++++++++---------------------
>>>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>>> 
>>>> diff --git a/gcc/system.h b/gcc/system.h
>>>> index e924152ad4c..7a516b11438 100644
>>>> --- a/gcc/system.h
>>>> +++ b/gcc/system.h
>>>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>>>> #undef fread_unlocked
>>>> #undef fwrite_unlocked
>>>> 
>>>> -/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
>>>> -   the ctype macros through safe-ctype.h */
>>>> -
>>>> -#ifdef __cplusplus
>>>> -#ifdef INCLUDE_STRING
>>>> -# include <string>
>>>> -#endif
>>>> -#endif
>>>> -
>>>> -/* There are an extraordinary number of issues with <ctype.h>.
>>>> -   The last straw is that it varies with the locale.  Use libiberty's
>>>> -   replacement instead.  */
>>>> -#include "safe-ctype.h"
>>>> -
>>>> -#include <sys/types.h>
>>>> -
>>>> -#include <errno.h>
>>>> -
>>>> -#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>>>> -extern int errno;
>>>> -#endif
>>>> +/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>>>> +   poisoning the ctype macros through safe-ctype.h */
>>>> 
>>>> #ifdef __cplusplus
>>>> #if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>>>> @@ -229,6 +210,9 @@ extern int errno;
>>>> #ifdef INCLUDE_SET
>>>> # include <set>
>>>> #endif
>>>> +#ifdef INCLUDE_STRING
>>>> +# include <string>
>>>> +#endif
>>>> #ifdef INCLUDE_VECTOR
>>>> # include <vector>
>>>> #endif
>>>> @@ -245,6 +229,19 @@ extern int errno;
>>>> # include <type_traits>
>>>> #endif
>>>> 
>>>> +/* There are an extraordinary number of issues with <ctype.h>.
>>>> +   The last straw is that it varies with the locale.  Use libiberty's
>>>> +   replacement instead.  */
>>>> +#include "safe-ctype.h"
>>>> +
>>>> +#include <sys/types.h>
>>>> +
>>>> +#include <errno.h>
>>>> +
>>>> +#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>>>> +extern int errno;
>>>> +#endif
>>>> +
>>>> /* Some of glibc's string inlines cause warnings.  Plus we'd rather
>>>>   rely on (and therefore test) GCC's string builtins.  */
>>>> #define __NO_STRING_INLINES
>>>> -- 
>>>> 2.42.0
>>>> 


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2023-09-28 16:37 [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning Dimitry Andric
  2023-10-26 17:00 ` [PING][PATCH] " Dimitry Andric
@ 2024-01-30 16:19 ` Jonathan Wakely
  1 sibling, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2024-01-30 16:19 UTC (permalink / raw)
  To: Dimitry Andric; +Cc: gcc-patches, Gerald Pfeifer

On 28/09/23 18:37 +0200, Dimitry Andric wrote:
>Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>
>When building gcc's C++ sources against recent libc++, the poisoning of
>the ctype macros due to including safe-ctype.h before including C++
>standard headers such as <list>, <map>, etc, causes many compilation
>errors, similar to:
>
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
> only applies to structs, variables, functions, and namespaces
>   546 |     _LIBCPP_INLINE_VISIBILITY
>       |     ^
> /usr/include/c++/v1/__config:813:37: note: expanded from macro
> '_LIBCPP_INLINE_VISIBILITY'
>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>       |                                     ^
> /usr/include/c++/v1/__config:792:26: note: expanded from macro
> '_LIBCPP_HIDE_FROM_ABI'
>   792 |
>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
> _LIBCPP_VERSIONED_IDENTIFIER))))
>       |                          ^
> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
> In file included from /usr/include/c++/v1/vector:321:
> In file included from
> /usr/include/c++/v1/__format/formatter_bool.h:20:
> In file included from
> /usr/include/c++/v1/__format/formatter_integral.h:32:
> In file included from /usr/include/c++/v1/locale:202:
> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
> declaration list
>   547 |     char_type toupper(char_type __c) const
>       |                                     ^
> /usr/include/c++/v1/__locale:553:48: error: too many arguments
> provided to function-like macro invocation
>   553 |     const char_type* toupper(char_type* __low, const
>   char_type* __high) const
>       |                                                ^
> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
> macro 'toupper' defined here
>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>       |         ^
>
>This is because libc++ uses different transitive includes than
>libstdc++, and some of those transitive includes pull in various ctype
>declarations (typically via <locale>).
>
>There was already a special case for including <string> before
>safe-ctype.h, so move the rest of the C++ standard header includes to
>the same location, to fix the problem.

I can't approve this either, but FWIW it looks good to me.


>Signed-off-by: Dimitry Andric <dimitry@andric.com>
>---
>gcc/system.h | 39 ++++++++++++++++++---------------------
>1 file changed, 18 insertions(+), 21 deletions(-)
>
>diff --git a/gcc/system.h b/gcc/system.h
>index e924152ad4c..7a516b11438 100644
>--- a/gcc/system.h
>+++ b/gcc/system.h
>@@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>#undef fread_unlocked
>#undef fwrite_unlocked
>
>-/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
>-   the ctype macros through safe-ctype.h */
>-
>-#ifdef __cplusplus
>-#ifdef INCLUDE_STRING
>-# include <string>
>-#endif
>-#endif
>-
>-/* There are an extraordinary number of issues with <ctype.h>.
>-   The last straw is that it varies with the locale.  Use libiberty's
>-   replacement instead.  */
>-#include "safe-ctype.h"
>-
>-#include <sys/types.h>
>-
>-#include <errno.h>
>-
>-#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>-extern int errno;
>-#endif
>+/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>+   poisoning the ctype macros through safe-ctype.h */
>
>#ifdef __cplusplus
>#if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>@@ -229,6 +210,9 @@ extern int errno;
>#ifdef INCLUDE_SET
># include <set>
>#endif
>+#ifdef INCLUDE_STRING
>+# include <string>
>+#endif
>#ifdef INCLUDE_VECTOR
># include <vector>
>#endif
>@@ -245,6 +229,19 @@ extern int errno;
># include <type_traits>
>#endif
>
>+/* There are an extraordinary number of issues with <ctype.h>.
>+   The last straw is that it varies with the locale.  Use libiberty's
>+   replacement instead.  */
>+#include "safe-ctype.h"
>+
>+#include <sys/types.h>
>+
>+#include <errno.h>
>+
>+#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>+extern int errno;
>+#endif
>+
>/* Some of glibc's string inlines cause warnings.  Plus we'd rather
>   rely on (and therefore test) GCC's string builtins.  */
>#define __NO_STRING_INLINES


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning
  2024-01-30 15:07       ` Sam James
@ 2024-01-30 20:27         ` Sam James
  0 siblings, 0 replies; 7+ messages in thread
From: Sam James @ 2024-01-30 20:27 UTC (permalink / raw)
  To: Sam James
  Cc: Dimitry Andric, Gerald Pfeifer, gcc-patches, Jeff Law,
	Arsen Arsenović


Sam James <sam@gentoo.org> writes:

> [[PGP Signed Part:Undecided]]
>
> Sam James <sam@gentoo.org> writes:
>
>> Sam James <sam@gentoo.org> writes:
>>
>>> Dimitry Andric <dimitry@andric.com> writes:
>>>
>>>> Ping. It would be nice to get this QoL fix in.
>>>>
>>>
>>> Yes please - we've been using this in Gentoo since around when it was
>>> first posted. No complaints.
>>>
>>> I cannot approve but it looks good to me.
>>
>> Ping.
>
> Ping. This is straightforward, I just can't merge it myself.
>
>>

We discussed it on IRC and Iain is going to test it on Darwin, I'm going
to test it on the cfarm on platforms the patch was not already exposed
to, after Jeff raised some concerns.

I also have an additional hunk or two I'm testing for JIT.

>>>
>>>> -Dimitry
>>>>
>>>>> On 28 Sep 2023, at 18:37, Dimitry Andric <dimitry@andric.com> wrote:
>>>>> 
>>>>> Ref: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111632
>>>>> 
>>>>> When building gcc's C++ sources against recent libc++, the poisoning of
>>>>> the ctype macros due to including safe-ctype.h before including C++
>>>>> standard headers such as <list>, <map>, etc, causes many compilation
>>>>> errors, similar to:
>>>>> 
>>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>>> In file included from /usr/include/c++/v1/vector:321:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>>> In file included from /usr/include/c++/v1/locale:202:
>>>>> /usr/include/c++/v1/__locale:546:5: error: '__abi_tag__' attribute
>>>>> only applies to structs, variables, functions, and namespaces
>>>>>   546 |     _LIBCPP_INLINE_VISIBILITY
>>>>>       |     ^
>>>>> /usr/include/c++/v1/__config:813:37: note: expanded from macro
>>>>> '_LIBCPP_INLINE_VISIBILITY'
>>>>>   813 | #  define _LIBCPP_INLINE_VISIBILITY _LIBCPP_HIDE_FROM_ABI
>>>>>       |                                     ^
>>>>> /usr/include/c++/v1/__config:792:26: note: expanded from macro
>>>>> '_LIBCPP_HIDE_FROM_ABI'
>>>>>   792 |
>>>>>   __attribute__((__abi_tag__(_LIBCPP_TOSTRING(
>>>>> _LIBCPP_VERSIONED_IDENTIFIER))))
>>>>>       |                          ^
>>>>> In file included from /home/dim/src/gcc/master/gcc/gensupport.cc:23:
>>>>> In file included from /home/dim/src/gcc/master/gcc/system.h:233:
>>>>> In file included from /usr/include/c++/v1/vector:321:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_bool.h:20:
>>>>> In file included from
>>>>> /usr/include/c++/v1/__format/formatter_integral.h:32:
>>>>> In file included from /usr/include/c++/v1/locale:202:
>>>>> /usr/include/c++/v1/__locale:547:37: error: expected ';' at end of
>>>>> declaration list
>>>>>   547 |     char_type toupper(char_type __c) const
>>>>>       |                                     ^
>>>>> /usr/include/c++/v1/__locale:553:48: error: too many arguments
>>>>> provided to function-like macro invocation
>>>>>   553 |     const char_type* toupper(char_type* __low, const
>>>>>   char_type* __high) const
>>>>>       |                                                ^
>>>>> /home/dim/src/gcc/master/gcc/../include/safe-ctype.h:146:9: note:
>>>>> macro 'toupper' defined here
>>>>>   146 | #define toupper(c) do_not_use_toupper_with_safe_ctype
>>>>>       |         ^
>>>>> 
>>>>> This is because libc++ uses different transitive includes than
>>>>> libstdc++, and some of those transitive includes pull in various ctype
>>>>> declarations (typically via <locale>).
>>>>> 
>>>>> There was already a special case for including <string> before
>>>>> safe-ctype.h, so move the rest of the C++ standard header includes to
>>>>> the same location, to fix the problem.
>>>>> 
>>>>> Signed-off-by: Dimitry Andric <dimitry@andric.com>
>>>>> ---
>>>>> gcc/system.h | 39 ++++++++++++++++++---------------------
>>>>> 1 file changed, 18 insertions(+), 21 deletions(-)
>>>>> 
>>>>> diff --git a/gcc/system.h b/gcc/system.h
>>>>> index e924152ad4c..7a516b11438 100644
>>>>> --- a/gcc/system.h
>>>>> +++ b/gcc/system.h
>>>>> @@ -194,27 +194,8 @@ extern int fprintf_unlocked (FILE *, const char *, ...);
>>>>> #undef fread_unlocked
>>>>> #undef fwrite_unlocked
>>>>> 
>>>>> -/* Include <string> before "safe-ctype.h" to avoid GCC poisoning
>>>>> -   the ctype macros through safe-ctype.h */
>>>>> -
>>>>> -#ifdef __cplusplus
>>>>> -#ifdef INCLUDE_STRING
>>>>> -# include <string>
>>>>> -#endif
>>>>> -#endif
>>>>> -
>>>>> -/* There are an extraordinary number of issues with <ctype.h>.
>>>>> -   The last straw is that it varies with the locale.  Use libiberty's
>>>>> -   replacement instead.  */
>>>>> -#include "safe-ctype.h"
>>>>> -
>>>>> -#include <sys/types.h>
>>>>> -
>>>>> -#include <errno.h>
>>>>> -
>>>>> -#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>>>>> -extern int errno;
>>>>> -#endif
>>>>> +/* Include C++ standard headers before "safe-ctype.h" to avoid GCC
>>>>> +   poisoning the ctype macros through safe-ctype.h */
>>>>> 
>>>>> #ifdef __cplusplus
>>>>> #if defined (INCLUDE_ALGORITHM) || !defined (HAVE_SWAP_IN_UTILITY)
>>>>> @@ -229,6 +210,9 @@ extern int errno;
>>>>> #ifdef INCLUDE_SET
>>>>> # include <set>
>>>>> #endif
>>>>> +#ifdef INCLUDE_STRING
>>>>> +# include <string>
>>>>> +#endif
>>>>> #ifdef INCLUDE_VECTOR
>>>>> # include <vector>
>>>>> #endif
>>>>> @@ -245,6 +229,19 @@ extern int errno;
>>>>> # include <type_traits>
>>>>> #endif
>>>>> 
>>>>> +/* There are an extraordinary number of issues with <ctype.h>.
>>>>> +   The last straw is that it varies with the locale.  Use libiberty's
>>>>> +   replacement instead.  */
>>>>> +#include "safe-ctype.h"
>>>>> +
>>>>> +#include <sys/types.h>
>>>>> +
>>>>> +#include <errno.h>
>>>>> +
>>>>> +#if !defined (errno) && defined (HAVE_DECL_ERRNO) && !HAVE_DECL_ERRNO
>>>>> +extern int errno;
>>>>> +#endif
>>>>> +
>>>>> /* Some of glibc's string inlines cause warnings.  Plus we'd rather
>>>>>   rely on (and therefore test) GCC's string builtins.  */
>>>>> #define __NO_STRING_INLINES
>>>>> -- 
>>>>> 2.42.0
>>>>> 
>
> [[End of PGP Signed Part]]


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-01-30 20:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-28 16:37 [PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning Dimitry Andric
2023-10-26 17:00 ` [PING][PATCH] " Dimitry Andric
2023-10-29 13:08   ` Sam James
2024-01-20  9:17     ` Sam James
2024-01-30 15:07       ` Sam James
2024-01-30 20:27         ` Sam James
2024-01-30 16:19 ` [PATCH] " Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).