Re: [PATCH] Define _C99 in libstdc++ vxworks/os_defines.h

2021-12-10 Thread Olivier Hainque via Gcc-patches



> On 10 Dec 2021, at 16:42, Jonathan Wakely  wrote:
> 
> 
> OK to commit then, thanks.
> 
> The comment is a bit misleading though:
> 
> +// libstdc++ relies on C99 features for virtually all versions of C++,
> +// up to at least C++98.
> +#undef _C99
> +#define _C99 1
> 
> The "up to" seems backwards, I'd expect it to say "down to" or "as far
> back as at least C++98".
> 
> But there's nothing older than C++98 anyway, so this just means "all
> versions", there's no "virtually" about it.
> 
> Please just say "libstdc++ relies on C99 features for all versions of C++".

Sure, thanks Jonathan. Yes, I agree the comment is confused.
I hesitated between a couple of perspectives and just mixed
things up.

Thanks for your prompt review!

Olivier


Re: [PATCH] Define _C99 in libstdc++ vxworks/os_defines.h

2021-12-10 Thread Jonathan Wakely via Gcc-patches
On Fri, 10 Dec 2021 at 15:16, Rasmus Villemoes via Libstdc++
 wrote:
>
> On 10/12/2021 16.06, Olivier Hainque wrote:
> > Hello,
> >
> > The attached patch for libstdc++ / VxWorks helps building
> > the library for old versions of the OS, as witnessed with
> > VxWorks 6.9 in particular.
> >
> > It explicitly requests C99 features from old system headers,
> > on which libstc++ relies since at least c++98. The specific
> > issue that exposed this was a failure to compile
> >
> >   libstdc++-v3/src/c++17/floating_to_chars.cc
> >
> > for VxWorks 6.9 with a batch of errors such as:
> >
> >   error: 'FP_NAN' was not declared in this scope
> >
> > The missing definitions are provided by the
> > system headers with guards on _HAS_C9X, which gets
> > internally defined when _C99 is.
> >
> > Ok to commit?
>
> Yes, we've observed that error as well. We have just patched our vxworks
> 5.5 headers to provide FP_NAN et al. There's no test of definedness or
> other uses of a _C99 macro anywhere, so ok by me, as it shouldn't change
> anything on our end.
>
> Rasmus

OK to commit then, thanks.

The comment is a bit misleading though:

+// libstdc++ relies on C99 features for virtually all versions of C++,
+// up to at least C++98.
+#undef _C99
+#define _C99 1

The "up to" seems backwards, I'd expect it to say "down to" or "as far
back as at least C++98".

But there's nothing older than C++98 anyway, so this just means "all
versions", there's no "virtually" about it.

Please just say "libstdc++ relies on C99 features for all versions of C++".


Re: [PATCH] Define _C99 in libstdc++ vxworks/os_defines.h

2021-12-10 Thread Rasmus Villemoes via Gcc-patches
On 10/12/2021 16.06, Olivier Hainque wrote:
> Hello,
> 
> The attached patch for libstdc++ / VxWorks helps building
> the library for old versions of the OS, as witnessed with
> VxWorks 6.9 in particular.
> 
> It explicitly requests C99 features from old system headers,
> on which libstc++ relies since at least c++98. The specific
> issue that exposed this was a failure to compile
> 
>   libstdc++-v3/src/c++17/floating_to_chars.cc
> 
> for VxWorks 6.9 with a batch of errors such as:
> 
>   error: 'FP_NAN' was not declared in this scope
> 
> The missing definitions are provided by the
> system headers with guards on _HAS_C9X, which gets
> internally defined when _C99 is.
> 
> Ok to commit?

Yes, we've observed that error as well. We have just patched our vxworks
5.5 headers to provide FP_NAN et al. There's no test of definedness or
other uses of a _C99 macro anywhere, so ok by me, as it shouldn't change
anything on our end.

Rasmus