Skip to content

Commit 230b24d

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 llvm#70142
1 parent 658ff0b commit 230b24d

File tree

5 files changed

+43
-129
lines changed

5 files changed

+43
-129
lines changed

Diff for: ‎libcxx/include/__ostream/print.h

+3-5
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ _LIBCPP_EXPORTED_FROM_ABI FILE* __get_ostream_file(ostream& __os);
9797
# ifndef _LIBCPP_HAS_NO_UNICODE
9898
template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
9999
_LIBCPP_HIDE_FROM_ABI void __vprint_unicode(ostream& __os, string_view __fmt, format_args __args, bool __write_nl) {
100-
# if _LIBCPP_AVAILABILITY_HAS_PRINT == 0
100+
# if _LIBCPP_AVAILABILITY_HAS_PRINT == 0 || !defined(_LIBCPP_WIN32API)
101101
return std::__vprint_nonunicode(__os, __fmt, __args, __write_nl);
102102
# else
103103
FILE* __file = std::__get_ostream_file(__os);
@@ -120,10 +120,8 @@ _LIBCPP_HIDE_FROM_ABI void __vprint_unicode(ostream& __os, string_view __fmt, fo
120120
# endif // _LIBCPP_HAS_EXCEPTIONS
121121
ostream::sentry __s(__os);
122122
if (__s) {
123-
# ifndef _LIBCPP_WIN32API
124-
__print::__vprint_unicode_posix(__file, __fmt, __args, __write_nl, true);
125-
# elif !defined(_LIBCPP_HAS_NO_WIDE_CHARACTERS)
126-
__print::__vprint_unicode_windows(__file, __fmt, __args, __write_nl, true);
123+
# ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS
124+
__print::__vprint_unicode_windows(__file, __fmt, __args, __write_nl);
127125
# else
128126
# error "Windows builds with wchar_t disabled are not supported."
129127
# endif

Diff for: ‎libcxx/include/print

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

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

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

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

+28
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,22 @@ static void test_is_terminal_file_stream() {
8181
assert(stream.is_open());
8282
assert(stream.good());
8383
std::print(stream, "test");
84+
#ifdef _WIN32
8485
assert(is_terminal_calls == 1);
86+
#else
87+
assert(is_terminal_calls == 0);
88+
#endif
8589
}
8690
{
8791
std::ofstream stream(filename);
8892
assert(stream.is_open());
8993
assert(stream.good());
9094
std::print(stream, "test");
95+
#ifdef _WIN32
9196
assert(is_terminal_calls == 2);
97+
#else
98+
assert(is_terminal_calls == 0);
99+
#endif
92100
}
93101
}
94102

@@ -105,7 +113,11 @@ static void test_is_terminal_rdbuf_derived_from_filebuf() {
105113

106114
std::ostream stream(&buf);
107115
std::print(stream, "test");
116+
#ifdef _WIN32
108117
assert(is_terminal_calls == 1);
118+
#else
119+
assert(is_terminal_calls == 0);
120+
#endif
109121
}
110122

111123
// When the stream is cout, clog, or cerr, its FILE* may be a terminal. Validate
@@ -115,15 +127,27 @@ static void test_is_terminal_std_cout_cerr_clog() {
115127
is_terminal_result = false;
116128
{
117129
std::print(std::cout, "test");
130+
#ifdef _WIN32
118131
assert(is_terminal_calls == 1);
132+
#else
133+
assert(is_terminal_calls == 0);
134+
#endif
119135
}
120136
{
121137
std::print(std::cerr, "test");
138+
#ifdef _WIN32
122139
assert(is_terminal_calls == 2);
140+
#else
141+
assert(is_terminal_calls == 0);
142+
#endif
123143
}
124144
{
125145
std::print(std::clog, "test");
146+
#ifdef _WIN32
126147
assert(is_terminal_calls == 3);
148+
#else
149+
assert(is_terminal_calls == 0);
150+
#endif
127151
}
128152
}
129153

@@ -156,7 +180,11 @@ static void test_is_terminal_is_flushed() {
156180
// A terminal sync is called.
157181
is_terminal_result = true;
158182
std::print(stream, "");
183+
#ifdef _WIN32
159184
assert(buf.sync_calls == 1); // only called from the destructor of the sentry
185+
#else
186+
assert(buf.sync_calls == 0);
187+
#endif
160188
}
161189

162190
int main(int, char**) {

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

-79
This file was deleted.

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

+3-27
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
// Tests the implementation of
2323
// void __print::__vprint_unicode_windows(FILE* __stream, string_view __fmt,
2424
// format_args __args, bool __write_nl,
25-
// bool __is_terminal);
25+
// );
2626
//
2727
// In the library when the stdout is redirected to a file it is no
2828
// longer considered a terminal and the special terminal handling is no
@@ -59,43 +59,19 @@ scoped_test_env env;
5959
std::string filename = env.create_file("output.txt");
6060

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

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

10177
static void test() {

0 commit comments

Comments
 (0)