Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Sat, Dec 16, 2023 at 7:04 PM Jonathan Wakely wrote: > On Sun, 17 Dec 2023 at 00:02, Jonathan Wakely wrote: > > > > On Sat, 16 Dec 2023 at 23:06, David Edelsohn wrote: > > > > > > On Sat, Dec 16, 2023 at 4:44 PM Jakub Jelinek > wrote: > > >> > > >> On Sat, Dec 16, 2023 at 04:24:13PM -0500, David Edelsohn wrote: > > >> > AIX stdio.h defines fileno as a macro although there is a symbol in > libc. > > >> > > > >> > I think that print.cc at least needs to > > >> > > > >> > > > >> > #undef fileno > > >> > > > >> > > > >> > before the usage. > > >> > > >> Or (::fileno)(f) ? > > > > > > > > > Yes, that also avoids the error. > > > > Yup, I've just tested it. I'll push that change in the morning. > > Actually I just pushed it now. The functions in that file are only > actually used on Windows, so if they build on linux and AIX, that's > good enough. > Thanks. I had tested and was just about to push it myself. Thanks, David
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Sun, 17 Dec 2023 at 00:02, Jonathan Wakely wrote: > > On Sat, 16 Dec 2023 at 23:06, David Edelsohn wrote: > > > > On Sat, Dec 16, 2023 at 4:44 PM Jakub Jelinek wrote: > >> > >> On Sat, Dec 16, 2023 at 04:24:13PM -0500, David Edelsohn wrote: > >> > AIX stdio.h defines fileno as a macro although there is a symbol in libc. > >> > > >> > I think that print.cc at least needs to > >> > > >> > > >> > #undef fileno > >> > > >> > > >> > before the usage. > >> > >> Or (::fileno)(f) ? > > > > > > Yes, that also avoids the error. > > Yup, I've just tested it. I'll push that change in the morning. Actually I just pushed it now. The functions in that file are only actually used on Windows, so if they build on linux and AIX, that's good enough.
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Sat, 16 Dec 2023 at 23:06, David Edelsohn wrote: > > On Sat, Dec 16, 2023 at 4:44 PM Jakub Jelinek wrote: >> >> On Sat, Dec 16, 2023 at 04:24:13PM -0500, David Edelsohn wrote: >> > AIX stdio.h defines fileno as a macro although there is a symbol in libc. >> > >> > I think that print.cc at least needs to >> > >> > >> > #undef fileno >> > >> > >> > before the usage. >> >> Or (::fileno)(f) ? > > > Yes, that also avoids the error. Yup, I've just tested it. I'll push that change in the morning. > > Thanks, David > >> >> >> Jakub >>
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Sat, Dec 16, 2023 at 4:44 PM Jakub Jelinek wrote: > On Sat, Dec 16, 2023 at 04:24:13PM -0500, David Edelsohn wrote: > > AIX stdio.h defines fileno as a macro although there is a symbol in libc. > > > > I think that print.cc at least needs to > > > > > > #undef fileno > > > > > > before the usage. > > Or (::fileno)(f) ? > Yes, that also avoids the error. Thanks, David > > Jakub > >
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Sat, Dec 16, 2023 at 04:24:13PM -0500, David Edelsohn wrote: > AIX stdio.h defines fileno as a macro although there is a symbol in libc. > > I think that print.cc at least needs to > > > #undef fileno > > > before the usage. Or (::fileno)(f) ? Jakub
Re: [committed] libstdc++: Implement C++23 header [PR107760]
Hi, Jonathan
Unfortunately this patch broke bootstrap on AIX.
In file included from /tmp/GCC/gcc/include-fixed/wchar.h:64,
from
/tmp/GCC/powerpc-ibm-aix7.2.5.0/libstdc++-v3/include/cwchar:44,
from
/tmp/GCC/powerpc-ibm-aix7.2.5.0/libstdc++-v3/include/bits/postypes.h:40,
from
/tmp/GCC/powerpc-ibm-aix7.2.5.0/libstdc++-v3/include/bits/char_traits.h:42,
from
/tmp/GCC/powerpc-ibm-aix7.2.5.0/libstdc++-v3/include/string:42,
from
/nasfarm/edelsohn/src/src/libstdc++-v3/src/c++23/print.cc:26:
/nasfarm/edelsohn/src/src/libstdc++-v3/src/c++23/print.cc: In function
'void* std::__open_terminal(FILE*)':
/nasfarm/edelsohn/src/src/libstdc++-v3/src/c++23/print.cc:78:24: error:
expected id-expression before '(' token
78 | if (int fd = ::fileno(f); fd >= 0 && ::isatty(fd))
|^~
make[6]: *** [Makefile:747: print.lo] Error 1
AIX stdio.h defines fileno as a macro although there is a symbol in libc.
I think that print.cc at least needs to
#undef fileno
before the usage.
Thanks, David
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Fri, 15 Dec 2023 at 14:49, Tim Song wrote:
>
>
>
> On Fri, Dec 15, 2023 at 4:43 AM Jonathan Wakely wrote:
>>
>> On Fri, 15 Dec 2023 at 01:17, Tim Song wrote:
>> >
>> > On Thu, Dec 14, 2023 at 6:05 PM Jonathan Wakely wrote:
>> >> + inline void
>> >> + vprint_unicode(ostream& __os, string_view __fmt, format_args __args)
>> >> + {
>> >> +ostream::sentry __cerb(__os);
>> >> +if (__cerb)
>> >> + {
>> >> +
>> >> + const streamsize __w = __os.width();
>> >> + const bool __left
>> >> + = (__os.flags() & ios_base::adjustfield) == ios_base::left;
>> >
>> >
>> > I'm pretty sure - when I wrote this wording anyway - that the intent was
>> > that it was just an unformatted write at the end. The wording in
>> > [ostream.formatted.print] doesn't use the "determines padding" words of
>> > power that would invoke [ostream.formatted.reqmts]/3.
>>
>> Ah, OK. I misunderstood "formatted output function" as implying
>> padding, failing to notice that we need those words of power to be
>> present. My thinking was that if the stream has padding set in its
>> format flags, it could be surprising if they're ignored by a formatted
>> output function. And padding in the format string applies to
>> individual replacement fields, not the whole string, and it's hard to
>> use the stream's fill character and alignment.
>
>
> But we would get none of the Unicode-aware padding logic we
> do in format, which puts it in a very weird place.
>
> And for cases where Unicode is not a problem, it's easy to get padding
> with just os << std::format(...);
Yes, good point.
>>
>> You can do this to use the ostream's width:
>>
>> std::print("{0:{1}}", std::format(...), os.width());
>>
>> But to reuse its fill char and adjustfield you need to do something
>> awful like I did in the committed code:
>>
>> std::string_view align;
>> if (os.flags() & ios::adjustfield) == ios::right)
>> align = ">"
>> auto fs = std::format("{{:{}{}{}}}", os.fill(), align, os.width());
>> std::vprint_nonunicode(os, fs, std::make_args(std::format(...)));
>>
>>
>> And now you have to hardcode a choice between vprint_unicode and
>> vprint_nonunicode, instead of letting std::print decide it. Let's hope
>> nobody ever needs to do any of that ;-)
>
>
> At least the upcoming runtime_format alleviates that :)
Right, that's on my list to implement "soon"!
>>
>>
>> I'll remove the code for padding the padding, thanks for checking the patch.
>>
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Fri, Dec 15, 2023 at 4:43 AM Jonathan Wakely wrote:
> On Fri, 15 Dec 2023 at 01:17, Tim Song wrote:
> >
> > On Thu, Dec 14, 2023 at 6:05 PM Jonathan Wakely
> wrote:
> >> + inline void
> >> + vprint_unicode(ostream& __os, string_view __fmt, format_args __args)
> >> + {
> >> +ostream::sentry __cerb(__os);
> >> +if (__cerb)
> >> + {
> >> +
> >> + const streamsize __w = __os.width();
> >> + const bool __left
> >> + = (__os.flags() & ios_base::adjustfield) == ios_base::left;
> >
> >
> > I'm pretty sure - when I wrote this wording anyway - that the intent was
> that it was just an unformatted write at the end. The wording in
> [ostream.formatted.print] doesn't use the "determines padding" words of
> power that would invoke [ostream.formatted.reqmts]/3.
>
> Ah, OK. I misunderstood "formatted output function" as implying
> padding, failing to notice that we need those words of power to be
> present. My thinking was that if the stream has padding set in its
> format flags, it could be surprising if they're ignored by a formatted
> output function. And padding in the format string applies to
> individual replacement fields, not the whole string, and it's hard to
> use the stream's fill character and alignment.
>
But we would get none of the Unicode-aware padding logic we
do in format, which puts it in a very weird place.
And for cases where Unicode is not a problem, it's easy to get padding
with just os << std::format(...);
> You can do this to use the ostream's width:
>
> std::print("{0:{1}}", std::format(...), os.width());
>
> But to reuse its fill char and adjustfield you need to do something
> awful like I did in the committed code:
>
> std::string_view align;
> if (os.flags() & ios::adjustfield) == ios::right)
> align = ">"
> auto fs = std::format("{{:{}{}{}}}", os.fill(), align, os.width());
> std::vprint_nonunicode(os, fs, std::make_args(std::format(...)));
> And now you have to hardcode a choice between vprint_unicode and
> vprint_nonunicode, instead of letting std::print decide it. Let's hope
> nobody ever needs to do any of that ;-)
>
At least the upcoming runtime_format alleviates that :)
>
> I'll remove the code for padding the padding, thanks for checking the
> patch.
>
>
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Fri, 15 Dec 2023 at 01:17, Tim Song wrote:
>
> On Thu, Dec 14, 2023 at 6:05 PM Jonathan Wakely wrote:
>> + inline void
>> + vprint_unicode(ostream& __os, string_view __fmt, format_args __args)
>> + {
>> +ostream::sentry __cerb(__os);
>> +if (__cerb)
>> + {
>> +
>> + const streamsize __w = __os.width();
>> + const bool __left
>> + = (__os.flags() & ios_base::adjustfield) == ios_base::left;
>
>
> I'm pretty sure - when I wrote this wording anyway - that the intent was that
> it was just an unformatted write at the end. The wording in
> [ostream.formatted.print] doesn't use the "determines padding" words of power
> that would invoke [ostream.formatted.reqmts]/3.
Ah, OK. I misunderstood "formatted output function" as implying
padding, failing to notice that we need those words of power to be
present. My thinking was that if the stream has padding set in its
format flags, it could be surprising if they're ignored by a formatted
output function. And padding in the format string applies to
individual replacement fields, not the whole string, and it's hard to
use the stream's fill character and alignment.
You can do this to use the ostream's width:
std::print("{0:{1}}", std::format(...), os.width());
But to reuse its fill char and adjustfield you need to do something
awful like I did in the committed code:
std::string_view align;
if (os.flags() & ios::adjustfield) == ios::right)
align = ">"
auto fs = std::format("{{:{}{}{}}}", os.fill(), align, os.width());
std::vprint_nonunicode(os, fs, std::make_args(std::format(...)));
And now you have to hardcode a choice between vprint_unicode and
vprint_nonunicode, instead of letting std::print decide it. Let's hope
nobody ever needs to do any of that ;-)
I'll remove the code for padding the padding, thanks for checking the patch.
Re: [committed] libstdc++: Implement C++23 header [PR107760]
On Thu, Dec 14, 2023 at 6:05 PM Jonathan Wakely wrote:
> Tested x86_64-linux. Pushed to trunk.
>
> -- >8 --
>
> This adds the C++23 std::print functions, which use std::format to write
> to a FILE stream or std::ostream (defaulting to stdout).
>
> The new extern symbols are in the libstdc++exp.a archive, so we aren't
> committing to stable symbols in the DSO yet. There's a UTF-8 validating
> and transcoding function added by this change. That can certainly be
> optimized, but it's internal to libstdc++exp.a so can be tweaked later
> at leisure.
>
> Currently the external symbols work for all targets, but are only
> actually used for Windows, where it's necessary to transcode to UTF-16
> to write to the console. The standard seems to encourage us to also
> diagnose invalid UTF-8 for non-Windows targets when writing to a
> terminal (and only when writing to a terminal), but I'm reliably
> informed that that wasn't the intent of the wording. Checking for
> invalid UTF-8 sequences only needs to happen for Windows, which is good
> as checking for a terminal requires a call to isatty, and on Linux that
> uses an ioctl syscall, which would make std::print ten times slower!
>
> Testing the std::print behaviour is difficult if it depends on whether
> the output stream is connected to a Windows console or not, as we can't
> (as far as I know) do that non-interactively in DejaGNU. One of the new
> tests uses the internal __write_to_terminal function directly. That
> allows us to verify its UTF-8 error handling on POSIX targets, even
> though that's not actually used by std::print. For Windows, that
> __write_to_terminal function transcodes to UTF-16 but then uses
> WriteConsoleW which fails unless it really is writing to the console.
> That means the 27_io/print/2.cc test FAILs on Windows. The UTF-16
> transcoding has been manually tested using mingw-w64 and Wine, and
> appears to work.
>
> libstdc++-v3/ChangeLog:
>
> PR libstdc++/107760
> * include/Makefile.am: Add new header.
> * include/Makefile.in: Regenerate.
> * include/bits/version.def (__cpp_lib_print): Define.
> * include/bits/version.h: Regenerate.
> * include/std/format (__literal_encoding_is_utf8): New function.
> (_Seq_sink::view()): New member function.
> * include/std/ostream (vprintf_nonunicode, vprintf_unicode)
> (print, println): New functions.
> * include/std/print: New file.
> * src/c++23/Makefile.am: Add new source file.
> * src/c++23/Makefile.in: Regenerate.
> * src/c++23/print.cc: New file.
> * testsuite/27_io/basic_ostream/print/1.cc: New test.
> * testsuite/27_io/print/1.cc: New test.
> * testsuite/27_io/print/2.cc: New test.
> ---
> libstdc++-v3/include/Makefile.am | 1 +
> libstdc++-v3/include/Makefile.in | 1 +
> libstdc++-v3/include/bits/version.def | 9 +
> libstdc++-v3/include/bits/version.h | 29 +-
> libstdc++-v3/include/std/format | 53 +++
> libstdc++-v3/include/std/ostream | 152
> libstdc++-v3/include/std/print| 138 +++
> libstdc++-v3/src/c++23/Makefile.am| 8 +-
> libstdc++-v3/src/c++23/Makefile.in| 10 +-
> libstdc++-v3/src/c++23/print.cc | 348 ++
> .../testsuite/27_io/basic_ostream/print/1.cc | 112 ++
> libstdc++-v3/testsuite/27_io/print/1.cc | 85 +
> libstdc++-v3/testsuite/27_io/print/2.cc | 151
> 13 files changed, 1085 insertions(+), 12 deletions(-)
> create mode 100644 libstdc++-v3/include/std/print
> create mode 100644 libstdc++-v3/src/c++23/print.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/print/1.cc
> create mode 100644 libstdc++-v3/testsuite/27_io/print/2.cc
>
> diff --git a/libstdc++-v3/include/Makefile.am
> b/libstdc++-v3/include/Makefile.am
> index 17d9d9cec31..368b92eafbc 100644
> --- a/libstdc++-v3/include/Makefile.am
> +++ b/libstdc++-v3/include/Makefile.am
> @@ -85,6 +85,7 @@ std_headers = \
> ${std_srcdir}/memory_resource \
> ${std_srcdir}/mutex \
> ${std_srcdir}/ostream \
> + ${std_srcdir}/print \
> ${std_srcdir}/queue \
> ${std_srcdir}/random \
> ${std_srcdir}/regex \
> diff --git a/libstdc++-v3/include/Makefile.in
> b/libstdc++-v3/include/Makefile.in
> index f038af709cc..a31588c0100 100644
> --- a/libstdc++-v3/include/Makefile.in
> +++ b/libstdc++-v3/include/Makefile.in
> @@ -441,6 +441,7 @@ std_freestanding = \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/memory_resource \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/mutex \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/ostream \
> +@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/print \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/queue \
> @GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/random \
> @GLIBCXX_HOSTED_TRUE@
[committed] libstdc++: Implement C++23 header [PR107760]
Tested x86_64-linux. Pushed to trunk.
-- >8 --
This adds the C++23 std::print functions, which use std::format to write
to a FILE stream or std::ostream (defaulting to stdout).
The new extern symbols are in the libstdc++exp.a archive, so we aren't
committing to stable symbols in the DSO yet. There's a UTF-8 validating
and transcoding function added by this change. That can certainly be
optimized, but it's internal to libstdc++exp.a so can be tweaked later
at leisure.
Currently the external symbols work for all targets, but are only
actually used for Windows, where it's necessary to transcode to UTF-16
to write to the console. The standard seems to encourage us to also
diagnose invalid UTF-8 for non-Windows targets when writing to a
terminal (and only when writing to a terminal), but I'm reliably
informed that that wasn't the intent of the wording. Checking for
invalid UTF-8 sequences only needs to happen for Windows, which is good
as checking for a terminal requires a call to isatty, and on Linux that
uses an ioctl syscall, which would make std::print ten times slower!
Testing the std::print behaviour is difficult if it depends on whether
the output stream is connected to a Windows console or not, as we can't
(as far as I know) do that non-interactively in DejaGNU. One of the new
tests uses the internal __write_to_terminal function directly. That
allows us to verify its UTF-8 error handling on POSIX targets, even
though that's not actually used by std::print. For Windows, that
__write_to_terminal function transcodes to UTF-16 but then uses
WriteConsoleW which fails unless it really is writing to the console.
That means the 27_io/print/2.cc test FAILs on Windows. The UTF-16
transcoding has been manually tested using mingw-w64 and Wine, and
appears to work.
libstdc++-v3/ChangeLog:
PR libstdc++/107760
* include/Makefile.am: Add new header.
* include/Makefile.in: Regenerate.
* include/bits/version.def (__cpp_lib_print): Define.
* include/bits/version.h: Regenerate.
* include/std/format (__literal_encoding_is_utf8): New function.
(_Seq_sink::view()): New member function.
* include/std/ostream (vprintf_nonunicode, vprintf_unicode)
(print, println): New functions.
* include/std/print: New file.
* src/c++23/Makefile.am: Add new source file.
* src/c++23/Makefile.in: Regenerate.
* src/c++23/print.cc: New file.
* testsuite/27_io/basic_ostream/print/1.cc: New test.
* testsuite/27_io/print/1.cc: New test.
* testsuite/27_io/print/2.cc: New test.
---
libstdc++-v3/include/Makefile.am | 1 +
libstdc++-v3/include/Makefile.in | 1 +
libstdc++-v3/include/bits/version.def | 9 +
libstdc++-v3/include/bits/version.h | 29 +-
libstdc++-v3/include/std/format | 53 +++
libstdc++-v3/include/std/ostream | 152
libstdc++-v3/include/std/print| 138 +++
libstdc++-v3/src/c++23/Makefile.am| 8 +-
libstdc++-v3/src/c++23/Makefile.in| 10 +-
libstdc++-v3/src/c++23/print.cc | 348 ++
.../testsuite/27_io/basic_ostream/print/1.cc | 112 ++
libstdc++-v3/testsuite/27_io/print/1.cc | 85 +
libstdc++-v3/testsuite/27_io/print/2.cc | 151
13 files changed, 1085 insertions(+), 12 deletions(-)
create mode 100644 libstdc++-v3/include/std/print
create mode 100644 libstdc++-v3/src/c++23/print.cc
create mode 100644 libstdc++-v3/testsuite/27_io/basic_ostream/print/1.cc
create mode 100644 libstdc++-v3/testsuite/27_io/print/1.cc
create mode 100644 libstdc++-v3/testsuite/27_io/print/2.cc
diff --git a/libstdc++-v3/include/Makefile.am b/libstdc++-v3/include/Makefile.am
index 17d9d9cec31..368b92eafbc 100644
--- a/libstdc++-v3/include/Makefile.am
+++ b/libstdc++-v3/include/Makefile.am
@@ -85,6 +85,7 @@ std_headers = \
${std_srcdir}/memory_resource \
${std_srcdir}/mutex \
${std_srcdir}/ostream \
+ ${std_srcdir}/print \
${std_srcdir}/queue \
${std_srcdir}/random \
${std_srcdir}/regex \
diff --git a/libstdc++-v3/include/Makefile.in b/libstdc++-v3/include/Makefile.in
index f038af709cc..a31588c0100 100644
--- a/libstdc++-v3/include/Makefile.in
+++ b/libstdc++-v3/include/Makefile.in
@@ -441,6 +441,7 @@ std_freestanding = \
@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/memory_resource \
@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/mutex \
@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/ostream \
+@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/print \
@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/queue \
@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/random \
@GLIBCXX_HOSTED_TRUE@ ${std_srcdir}/regex \
diff --git a/libstdc++-v3/include/bits/version.def
b/libstdc++-v3/include/bits/version.def
index 38b73ec9b5d..0134a71b3ab 100644
--- a/libstdc++-v3/include/bits/version.def
+++ b/libstdc++-v3/include/bits/version.def
@@ -1645,6
