[PATCH] #undef isblank before def or decl in libstdc++ headers

[PATCH] #undef isblank before def or decl in libstdc++ headers

[Olivier Hainque]

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

Hello,

The attached patch helps fix a build failure of libstdc++
on some variants of VxWorks where the system headers expose
an "isblank" macro.

I understand this kind of thing normally is handled through
fixincludes, however fixincludes on VxWorks is very tricky and
we already have

  libstdc++-v3/include/c_global/cctype:#undef isblank
  libstdc++-v3/include/tr1/cctype:#undef isblank

so the suggestion here is to simply do the same in a couple
more places.

I checked that it gets us through the observed build failure
for VxWorks, then bootstrapped and regtested on native 64bit
linux.

Ok to commit?

Thanks in advance,

With Kind Regards,

Olivier

2021-12-07  Olivier Hainque  <hainque@adacore.com>

libstdc++-v3/

	* include/bits/locale_facets.h: #undef isblank before
	providing a definition.
	* libstdc++-v3/include/bits/localefwd.h: Likewise.


[-- Attachment #2: 0001-Add-undef-isblank-before-definition-or-decl-in-bits-.patch --]
[-- Type: application/octet-stream, Size: 1974 bytes --]

From 6e2b18025476ba5acf929eb7a313305ff282efe3 Mon Sep 17 00:00:00 2001
From: Olivier Hainque <hainque@adacore.com>
Date: Fri, 19 Nov 2021 15:20:45 +0000
Subject: [PATCH] Add #undef isblank before definition or decl in bits headers

As already done in

  libstdc++-v3/include/c_global/cctype:#undef isblank
  libstdc++-v3/include/tr1/cctype:#undef isblank

Fixes build failures for old VxWorks variants where the system
headers provide a macro version (observed on 6.9).

2021-12-07  Olivier Hainque  <hainque@adacore.com>

libstdc++-v3/

	* include/bits/locale_facets.h: #undef isblank before
	providing a definition.
	* libstdc++-v3/include/bits/localefwd.h: Likewise.

Part of UB12-004 (vxworks ports transition to gcc-11)

Change-Id: Ib682f663a833d02e814c3ac9905e81096c361601
(cherry picked from commit 6bad427c07cb22983b419a85cf9116f03e954d47)
---
 libstdc++-v3/include/bits/locale_facets.h | 1 +
 libstdc++-v3/include/bits/localefwd.h     | 1 +
 2 files changed, 2 insertions(+)

diff --git a/libstdc++-v3/include/bits/locale_facets.h b/libstdc++-v3/include/bits/locale_facets.h
index 5ca431e1a25..9e1d33f84e7 100644
--- a/libstdc++-v3/include/bits/locale_facets.h
+++ b/libstdc++-v3/include/bits/locale_facets.h
@@ -2663,6 +2663,7 @@ _GLIBCXX_END_NAMESPACE_LDBL
 
 #if __cplusplus >= 201103L
   /// Convenience interface to ctype.is(ctype_base::blank, __c).
+  #undef isblank
   template<typename _CharT>
     inline bool
     isblank(_CharT __c, const locale& __loc)
diff --git a/libstdc++-v3/include/bits/localefwd.h b/libstdc++-v3/include/bits/localefwd.h
index 9bc35e9761e..8d86386d2e5 100644
--- a/libstdc++-v3/include/bits/localefwd.h
+++ b/libstdc++-v3/include/bits/localefwd.h
@@ -108,6 +108,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     isgraph(_CharT, const locale&);
 
 #if __cplusplus >= 201103L
+  #undef isblank
   template<typename _CharT>
     bool
     isblank(_CharT, const locale&);
-- 
2.25.1

Re: [PATCH] #undef isblank before def or decl in libstdc++ headers

[Jonathan Wakely]

On Fri, 10 Dec 2021 at 17:07, Olivier Hainque via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Hello,
>
> The attached patch helps fix a build failure of libstdc++
> on some variants of VxWorks where the system headers expose
> an "isblank" macro.
>
> I understand this kind of thing normally is handled through
> fixincludes, however fixincludes on VxWorks is very tricky and
> we already have
>
>   libstdc++-v3/include/c_global/cctype:#undef isblank
>   libstdc++-v3/include/tr1/cctype:#undef isblank
>
> so the suggestion here is to simply do the same in a couple
> more places.

Both of those places already include <cctype> so I think we should just do this:

--- a/libstdc++-v3/include/c_global/cctype
+++ b/libstdc++-v3/include/c_global/cctype
@@ -78,10 +78,10 @@ namespace std

#if __cplusplus >= 201103L

-#ifdef _GLIBCXX_USE_C99_CTYPE_TR1
-
#undef isblank

+#ifdef _GLIBCXX_USE_C99_CTYPE_TR1
+
namespace std
{
  using ::isblank;

I'm curious why _GLIBCXX_USE_C99_CTYPE_TR1 is not defined if VxWorks
has isblank, the configure check is:

  # Check for the existence of <ctype.h> functions.
  AC_CACHE_CHECK([for ISO C99 support to TR1 in <ctype.h>],
  glibcxx_cv_c99_ctype_tr1, [
  AC_TRY_COMPILE([#include <ctype.h>],
         [int ch;
          int ret;
          ret = isblank(ch);
         ],[glibcxx_cv_c99_ctype_tr1=yes],
           [glibcxx_cv_c99_ctype_tr1=no])
  ])
  if test x"$glibcxx_cv_c99_ctype_tr1" = x"yes"; then
    AC_DEFINE(_GLIBCXX_USE_C99_CTYPE_TR1, 1,
          [Define if C99 functions in <ctype.h> should be imported in
          <tr1/cctype> in namespace std::tr1.])
  fi

In any case, undef'ing it unconditionally in <cctype> should work, and
avoids having to do it in multiple places (we still need it in
<tr1/cctype> because that is used in C++98 code, whereas the other
definitions of functions called "isblank" were added for C++11).

Re: [PATCH] #undef isblank before def or decl in libstdc++ headers

[Olivier Hainque]

(Thanks for your feedback Jonathan)

> On 10 Dec 2021, at 19:24, Jonathan Wakely <jwakely@redhat.com> wrote:
> 
> I'm curious why _GLIBCXX_USE_C99_CTYPE_TR1 is not defined if VxWorks
> has isblank, the configure check is:

Oh, hmm, very good point. The reason was that the definition
of isblank is conditioned on _C99/_HAS_C9X as well, so the need
for which we had introduced the definition in os_defines.h
would better be generalized.

	* config/vxworks.h (VXWORKS_OS_CPP_BUILTINS): Define
	_C99 for C++.

--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -328,6 +328,10 @@ extern void vxworks_asm_out_destructor (rtx symbol, int priority);
 	   if (!flag_isoc99 && !c_dialect_cxx())			\
              builtin_define ("_ALLOW_KEYWORD_MACROS");			\
         }								\
+      /* C++ support relies on C99 features.  Make sure they are	\
+	 exposed by the system headers.  */				\
+      if (c_dialect_cxx())						\
+	builtin_define("_C99");						\
     }									\
   while (0)


Works with the two libstdc++ changes reverted, and gives
"configure" a better view of what's there.

Makes sense?

Re: [PATCH] #undef isblank before def or decl in libstdc++ headers

[Jonathan Wakely]

On Sat, 11 Dec 2021, 10:56 Olivier Hainque via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> (Thanks for your feedback Jonathan)
>
> > On 10 Dec 2021, at 19:24, Jonathan Wakely <jwakely@redhat.com> wrote:
> >
> > I'm curious why _GLIBCXX_USE_C99_CTYPE_TR1 is not defined if VxWorks
> > has isblank, the configure check is:
>
> Oh, hmm, very good point. The reason was that the definition
> of isblank is conditioned on _C99/_HAS_C9X as well, so the need
> for which we had introduced the definition in os_defines.h
> would better be generalized.
>
>         * config/vxworks.h (VXWORKS_OS_CPP_BUILTINS): Define
>         _C99 for C++.
>
> --- a/gcc/config/vxworks.h
> +++ b/gcc/config/vxworks.h
> @@ -328,6 +328,10 @@ extern void vxworks_asm_out_destructor (rtx symbol,
> int priority);
>            if (!flag_isoc99 && !c_dialect_cxx())                        \
>               builtin_define ("_ALLOW_KEYWORD_MACROS");                 \
>          }                                                              \
> +      /* C++ support relies on C99 features.  Make sure they are       \
> +        exposed by the system headers.  */                             \
> +      if (c_dialect_cxx())                                             \
> +       builtin_define("_C99");                                         \
>      }                                                                  \
>    while (0)
>
>
> Works with the two libstdc++ changes reverted, and gives
> "configure" a better view of what's there.
>
> Makes sense?


Yes. I can't approve patches outside libstdc++, but that looks definitely
correct for C++11 and later, because C++11 incorporates the whole C99
library by reference. So if that macro is needed to get the C99 library
(because the vxworks libc doesn't check the__cplusplus macro and enable C99
features that way), then I agree _C99 should be defined for C++11. Defining
it for C++11 is sufficient to solve the isblank problem, because
std::isblank is only declared for C++11 and later. (std::tr1::isblank is
declared for C++98 if the C library supports it, but nobody really cares
about TR1 nowadays, and probably hardly anybody cares about C++98).

Defining _C99 is not strictly correct for C++98 mode, because C++98
incorporates the C89 library by reference. But as you noted in the earlier
patch, libstdc++ likes to have full C99 facilities available even for C++98
mode (so it can use them for std::tr1 features, among other things). So I
think defining it even for C++98 is fine too.

Re: [PATCH] #undef isblank before def or decl in libstdc++ headers

[Olivier Hainque]

> On 11 Dec 2021, at 18:09, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> 
>         * config/vxworks.h (VXWORKS_OS_CPP_BUILTINS): Define
>         _C99 for C++.
> 
> Makes sense?
> 
> Yes. I can't approve patches outside libstdc++, but that looks definitely correct for C++11 and later, because C++11 incorporates the whole C99 library by reference. So if that macro is needed to get the C99 library (because the vxworks libc doesn't check the__cplusplus macro and enable C99 features that way), then I agree _C99 should be defined for C++11. Defining it for C++11 is sufficient to solve the isblank problem, because std::isblank is only declared for C++11 and later. (std::tr1::isblank is declared for C++98 if the C library supports it, but nobody really cares about TR1 nowadays, and probably hardly anybody cares about C++98).
> 
> Defining _C99 is not strictly correct for C++98 mode, because C++98 incorporates the C89 library by reference. But as you noted in the earlier patch, libstdc++ likes to have full C99 facilities available even for C++98 mode (so it can use them for std::tr1 features, among other things). So I think defining it even for C++98 is fine too.

Testing looks good so far and libstdc++ configure is a lot
happier indeed.

Inspecting the logs spotted an inaccuracy in another area,
which I'll address separately.

Thanks a lot for your feedback!

With Kind Regards,

Olivier