* [PATCH] #undef isblank before def or decl in libstdc++ headers
@ 2021-12-10 17:06 Olivier Hainque
2021-12-10 18:24 ` Jonathan Wakely
0 siblings, 1 reply; 5+ messages in thread
From: Olivier Hainque @ 2021-12-10 17:06 UTC (permalink / raw)
To: libstdc++, gcc-patches; +Cc: 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] #undef isblank before def or decl in libstdc++ headers
2021-12-10 17:06 [PATCH] #undef isblank before def or decl in libstdc++ headers Olivier Hainque
@ 2021-12-10 18:24 ` Jonathan Wakely
2021-12-11 10:55 ` Olivier Hainque
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2021-12-10 18:24 UTC (permalink / raw)
To: Olivier Hainque; +Cc: libstdc++, gcc-patches
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).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] #undef isblank before def or decl in libstdc++ headers
2021-12-10 18:24 ` Jonathan Wakely
@ 2021-12-11 10:55 ` Olivier Hainque
2021-12-11 17:09 ` Jonathan Wakely
0 siblings, 1 reply; 5+ messages in thread
From: Olivier Hainque @ 2021-12-11 10:55 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Olivier Hainque, libstdc++, gcc-patches
(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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] #undef isblank before def or decl in libstdc++ headers
2021-12-11 10:55 ` Olivier Hainque
@ 2021-12-11 17:09 ` Jonathan Wakely
2021-12-13 11:09 ` Olivier Hainque
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Wakely @ 2021-12-11 17:09 UTC (permalink / raw)
To: Olivier Hainque; +Cc: Jonathan Wakely, libstdc++, gcc-patches
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] #undef isblank before def or decl in libstdc++ headers
2021-12-11 17:09 ` Jonathan Wakely
@ 2021-12-13 11:09 ` Olivier Hainque
0 siblings, 0 replies; 5+ messages in thread
From: Olivier Hainque @ 2021-12-13 11:09 UTC (permalink / raw)
To: Jonathan Wakely; +Cc: Olivier Hainque, Jonathan Wakely, libstdc++, gcc-patches
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-13 11:09 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-10 17:06 [PATCH] #undef isblank before def or decl in libstdc++ headers Olivier Hainque
2021-12-10 18:24 ` Jonathan Wakely
2021-12-11 10:55 ` Olivier Hainque
2021-12-11 17:09 ` Jonathan Wakely
2021-12-13 11:09 ` Olivier Hainque
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).