Re: [PING][PATCH] Include safe-ctype.h after C++ standard headers, to avoid over-poisoning

[Sam James <sam@gentoo.org>]

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
>>>