Skip to content

Commit a0a17c1

Browse files
dimztimzdimztimz
committed
[libc++] Fix unnecessary flushes in std::print() on POSIX
The check whether the stream is associated with a terminal or not is needed only on Windows. The flush is needed only on Windows and when the stream is terminal. The flushing is done only once before using the native Unicode API on Windows. Additionally, the correct flush is now called. In the case of C stream overloads of print(), std::fflush() should be used, and in the ostream overloads, only ostream::flush() member function should be used. Before this fix, the ostream overloads called ostream::flush() and then std::fflush(). See also https://wg21.link/LWG4044. Fixes #70142
1 parent 3aae916 commit a0a17c1

File tree

5 files changed

+43
-127
lines changed

5 files changed

+43
-127
lines changed

‎libcxx/include/__ostream/print.h

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ _LIBCPP_EXPORTED_FROM_ABI FILE* __get_ostream_file(ostream& __os);
9595
# ifndef _LIBCPP_HAS_NO_UNICODE
9696
template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
9797
_LIBCPP_HIDE_FROM_ABI void __vprint_unicode(ostream& __os, string_view __fmt, format_args __args, bool __write_nl) {
98-
# if _LIBCPP_AVAILABILITY_HAS_PRINT == 0
98+
# if _LIBCPP_AVAILABILITY_HAS_PRINT == 0 || !defined(_LIBCPP_WIN32API)
9999
return std::__vprint_nonunicode(__os, __fmt, __args, __write_nl);
100100
# else
101101
FILE* __file = std::__get_ostream_file(__os);
@@ -118,10 +118,8 @@ _LIBCPP_HIDE_FROM_ABI void __vprint_unicode(ostream& __os, string_view __fmt, fo
118118
# endif // _LIBCPP_HAS_NO_EXCEPTIONS
119119
ostream::sentry __s(__os);
120120
if (__s) {
121-
# ifndef _LIBCPP_WIN32API
122-
__print::__vprint_unicode_posix(__file, __fmt, __args, __write_nl, true);
123-
# elif !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS)
124-
__print::__vprint_unicode_windows(__file, __fmt, __args, __write_nl, true);
121+
# ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
122+
__print::__vprint_unicode_windows(__file, __fmt, __args, __write_nl);
125123
# else
126124
# error "Windows builds with wchar_t disabled are not supported."
127125
# endif

‎libcxx/include/print

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -234,26 +234,11 @@ __vprint_nonunicode(FILE* __stream, string_view __fmt, format_args __args, bool
234234
// terminal when the output is redirected. Typically during testing the
235235
// output is redirected to be able to capture it. This makes it hard to
236236
// test this code path.
237-
template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
238-
_LIBCPP_HIDE_FROM_ABI inline void
239-
__vprint_unicode_posix(FILE* __stream, string_view __fmt, format_args __args, bool __write_nl, bool __is_terminal) {
240-
// TODO PRINT Should flush errors throw too?
241-
if (__is_terminal)
242-
std::fflush(__stream);
243-
244-
__print::__vprint_nonunicode(__stream, __fmt, __args, __write_nl);
245-
}
246237

247238
# ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
248239
template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
249240
_LIBCPP_HIDE_FROM_ABI inline void
250-
__vprint_unicode_windows(FILE* __stream, string_view __fmt, format_args __args, bool __write_nl, bool __is_terminal) {
251-
if (!__is_terminal)
252-
return __print::__vprint_nonunicode(__stream, __fmt, __args, __write_nl);
253-
254-
// TODO PRINT Should flush errors throw too?
255-
std::fflush(__stream);
256-
241+
__vprint_unicode_windows([[maybe_unused]] FILE* __stream, string_view __fmt, format_args __args, bool __write_nl) {
257242
string __str = std::vformat(__fmt, __args);
258243
// UTF-16 uses the same number or less code units than UTF-8.
259244
// However the size of the code unit is 16 bits instead of 8 bits.
@@ -314,9 +299,15 @@ __vprint_unicode([[maybe_unused]] FILE* __stream,
314299
// Windows there is a different API. This API requires transcoding.
315300

316301
# ifndef _LIBCPP_WIN32API
317-
__print::__vprint_unicode_posix(__stream, __fmt, __args, __write_nl, __print::__is_terminal(__stream));
302+
__print::__vprint_nonunicode(__stream, __fmt, __args, __write_nl);
318303
# elif !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS)
319-
__print::__vprint_unicode_windows(__stream, __fmt, __args, __write_nl, __print::__is_terminal(__stream));
304+
if (__print::__is_terminal(__stream)) {
305+
// TODO PRINT Should flush errors throw too?
306+
std::fflush(__stream);
307+
__print::__vprint_unicode_windows(__stream, __fmt, __args, __write_nl);
308+
} else {
309+
__print::__vprint_nonunicode(__stream, __fmt, __args, __write_nl);
310+
}
320311
# else
321312
# error "Windows builds with wchar_t disabled are not supported."
322313
# endif

‎libcxx/test/libcxx/input.output/iostream.format/output.streams/ostream.formatted/ostream.formatted.print/vprint_unicode.pass.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,14 +74,22 @@ static void test_is_terminal_file_stream() {
7474
assert(stream.is_open());
7575
assert(stream.good());
7676
std::print(stream, "test");
77+
#ifdef _WIN32
7778
assert(is_terminal_calls == 1);
79+
#else
80+
assert(is_terminal_calls == 0);
81+
#endif
7882
}
7983
{
8084
std::ofstream stream(filename);
8185
assert(stream.is_open());
8286
assert(stream.good());
8387
std::print(stream, "test");
88+
#ifdef _WIN32
8489
assert(is_terminal_calls == 2);
90+
#else
91+
assert(is_terminal_calls == 0);
92+
#endif
8593
}
8694
}
8795

@@ -98,7 +106,11 @@ static void test_is_terminal_rdbuf_derived_from_filebuf() {
98106

99107
std::ostream stream(&buf);
100108
std::print(stream, "test");
109+
#ifdef _WIN32
101110
assert(is_terminal_calls == 1);
111+
#else
112+
assert(is_terminal_calls == 0);
113+
#endif
102114
}
103115

104116
// When the stream is cout, clog, or cerr, its FILE* may be a terminal. Validate
@@ -108,15 +120,27 @@ static void test_is_terminal_std_cout_cerr_clog() {
108120
is_terminal_result = false;
109121
{
110122
std::print(std::cout, "test");
123+
#ifdef _WIN32
111124
assert(is_terminal_calls == 1);
125+
#else
126+
assert(is_terminal_calls == 0);
127+
#endif
112128
}
113129
{
114130
std::print(std::cerr, "test");
131+
#ifdef _WIN32
115132
assert(is_terminal_calls == 2);
133+
#else
134+
assert(is_terminal_calls == 0);
135+
#endif
116136
}
117137
{
118138
std::print(std::clog, "test");
139+
#ifdef _WIN32
119140
assert(is_terminal_calls == 3);
141+
#else
142+
assert(is_terminal_calls == 0);
143+
#endif
120144
}
121145
}
122146

@@ -149,7 +173,11 @@ static void test_is_terminal_is_flushed() {
149173
// A terminal sync is called.
150174
is_terminal_result = true;
151175
std::print(stream, "");
176+
#ifdef _WIN32
152177
assert(buf.sync_calls == 1); // only called from the destructor of the sentry
178+
#else
179+
assert(buf.sync_calls == 0);
180+
#endif
153181
}
154182

155183
int main(int, char**) {

‎libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_posix.pass.cpp

Lines changed: 0 additions & 77 deletions
This file was deleted.

‎libcxx/test/libcxx/input.output/iostream.format/print.fun/vprint_unicode_windows.pass.cpp

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
// Tests the implementation of
2121
// void __print::__vprint_unicode_windows(FILE* __stream, string_view __fmt,
2222
// format_args __args, bool __write_nl,
23-
// bool __is_terminal);
23+
// );
2424
//
2525
// In the library when the stdout is redirected to a file it is no
2626
// longer considered a terminal and the special terminal handling is no
@@ -57,43 +57,19 @@ scoped_test_env env;
5757
std::string filename = env.create_file("output.txt");
5858

5959
static void test_basics() {
60-
FILE* file = std::fopen(filename.c_str(), "wb");
61-
assert(file);
62-
63-
// Test writing to a "non-terminal" stream does not call WriteConsoleW.
64-
std::__print::__vprint_unicode_windows(file, "Hello", std::make_format_args(), false, false);
65-
assert(std::ftell(file) == 5);
66-
6760
// It's not possible to reliably test whether writing to a "terminal" stream
6861
// flushes before writing. Testing flushing a closed stream worked on some
6962
// platforms, but was unreliable.
7063
calling = true;
71-
std::__print::__vprint_unicode_windows(file, " world", std::make_format_args(), false, true);
64+
std::__print::__vprint_unicode_windows(stdout, " world", std::make_format_args(), false);
7265
}
7366

7467
// When the output is a file the data is written as-is.
7568
// When the output is a "terminal" invalid UTF-8 input is flagged.
7669
static void test(std::wstring_view output, std::string_view input) {
77-
// *** File ***
78-
FILE* file = std::fopen(filename.c_str(), "wb");
79-
assert(file);
80-
81-
std::__print::__vprint_unicode_windows(file, input, std::make_format_args(), false, false);
82-
assert(std::ftell(file) == static_cast<long>(input.size()));
83-
std::fclose(file);
84-
85-
file = std::fopen(filename.c_str(), "rb");
86-
assert(file);
87-
88-
std::vector<char> buffer(input.size());
89-
size_t read = fread(buffer.data(), 1, buffer.size(), file);
90-
assert(read == input.size());
91-
assert(input == std::string_view(buffer.begin(), buffer.end()));
92-
std::fclose(file);
93-
9470
// *** Terminal ***
9571
expected = output;
96-
std::__print::__vprint_unicode_windows(file, input, std::make_format_args(), false, true);
72+
std::__print::__vprint_unicode_windows(stdout, input, std::make_format_args(), false);
9773
}
9874

9975
static void test() {

0 commit comments

Comments
 (0)