From 0cbc004949692282f88db144f240153bc5f3a349 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Wed, 20 Jul 2022 17:10:10 +0200 Subject: [PATCH 1/9] Refactor detail::print() by splitting into two functions. The part with SetConsoleW is a separate function, without fwrite(). --- include/fmt/format-inl.h | 23 ++++++++++++++--------- include/fmt/format.h | 1 + 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/include/fmt/format-inl.h b/include/fmt/format-inl.h index 8391aa9b7414..61256d2ccc7b 100644 --- a/include/fmt/format-inl.h +++ b/include/fmt/format-inl.h @@ -1473,17 +1473,13 @@ FMT_FUNC std::string vformat(string_view fmt, format_args args) { return to_string(buffer); } -#ifdef _WIN32 namespace detail { +#ifdef _WIN32 using dword = conditional_t; extern "C" __declspec(dllimport) int __stdcall WriteConsoleW( // void*, const void*, dword, dword*, void*); -} // namespace detail -#endif -namespace detail { -FMT_FUNC void print(std::FILE* f, string_view text) { -#ifdef _WIN32 +FMT_FUNC bool write_console_on_windows(std::FILE* f, string_view text) { auto fd = _fileno(f); if (_isatty(fd)) { detail::utf8_to_utf16 u16(string_view(text.data(), text.size())); @@ -1491,11 +1487,20 @@ FMT_FUNC void print(std::FILE* f, string_view text) { if (detail::WriteConsoleW(reinterpret_cast(_get_osfhandle(fd)), u16.c_str(), static_cast(u16.size()), &written, nullptr)) { - return; + return true; } - // Fallback to fwrite on failure. It can happen if the output has been - // redirected to NUL. } + // We return false if the file descriptor was not TTY, or it was but + // SetConsoleW failed which can happen if the output has been redirected to + // NUL. In both cases when we return false, we should attempt to do regular + // write via fwrite or std::ostream::write. + return false; +} +#endif + +FMT_FUNC void print(std::FILE* f, string_view text) { +#ifdef _WIN32 + if (write_console_on_windows(f, text)) return; #endif detail::fwrite_fully(text.data(), 1, text.size(), f); } diff --git a/include/fmt/format.h b/include/fmt/format.h index 83b2a22266c3..40f15d155129 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -921,6 +921,7 @@ struct is_contiguous> : std::true_type { }; namespace detail { +bool write_console_on_windows(std::FILE* f, string_view text); FMT_API void print(std::FILE*, string_view); } From e17de67514f826d2902822349fcff00d441afd28 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Mon, 18 Jul 2022 15:58:17 +0200 Subject: [PATCH 2/9] Make Unicode handing when writing to std::ostream more robust. Calls to print(ostream&) in the special Unicode case on Windows fallback to writing via ostream::write instead of fwrite(). --- include/fmt/ostream.h | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index ced05f75baf2..e6c8256e8c81 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -81,13 +81,14 @@ template class filebuf_access; -inline bool write(std::filebuf& buf, fmt::string_view data) { - FILE* f = get_file(buf); - if (!f) return false; - print(f, data); - return true; +inline bool write_ostream_msvc_unicode(std::ostream& os, + fmt::string_view data) { + if (auto* buf = dynamic_cast(os.rdbuf())) + if (FILE* f = get_file(*buf)) return write_console_on_windows(f, data); + return false; } -inline bool write(std::wfilebuf&, fmt::basic_string_view) { +inline bool write_ostream_msvc_unicode(std::wostream&, + fmt::basic_string_view) { return false; } @@ -95,10 +96,8 @@ inline bool write(std::wfilebuf&, fmt::basic_string_view) { // It is a separate function rather than a part of vprint to simplify testing. template void write_buffer(std::basic_ostream& os, buffer& buf) { - if (const_check(FMT_MSC_VERSION)) { - auto filebuf = dynamic_cast*>(os.rdbuf()); - if (filebuf && write(*filebuf, {buf.data(), buf.size()})) return; - } + if (const_check(FMT_MSC_VERSION)) + if (write_ostream_msvc_unicode(os, {buf.data(), buf.size()})) return; const Char* buf_data = buf.data(); using unsigned_streamsize = std::make_unsigned::type; unsigned_streamsize size = buf.size(); From e6d14892768099b5e4cd571530667120c496bea4 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Wed, 20 Jul 2022 20:25:52 +0200 Subject: [PATCH 3/9] Fix Unicode handling when writing to an ostream on GCC on Windows --- include/fmt/ostream.h | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index e6c8256e8c81..1eb9fd746241 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -10,6 +10,10 @@ #include #include +#if _WIN32 && __GLIBCXX__ +# include +# include +#endif #include "format.h" @@ -92,12 +96,36 @@ inline bool write_ostream_msvc_unicode(std::wostream&, return false; } +#if _WIN32 && __GLIBCXX__ +inline bool write_ostream_gcc_mingw_unicode(std::ostream& os, + fmt::string_view data) { + auto* rdbuf = os.rdbuf(); + FILE* c_file; + if (auto* fbuf = dynamic_cast<__gnu_cxx::stdio_sync_filebuf*>(rdbuf)) + c_file = fbuf->file(); + else if (auto* fbuf = dynamic_cast<__gnu_cxx::stdio_filebuf*>(rdbuf)) + c_file = fbuf->file(); + else + return false; + if (c_file) return write_console_on_windows(c_file, data); + return false; +} + +inline bool write_ostream_gcc_mingw_unicode(std::wostream&, + fmt::basic_string_view) { + return false; +} +#endif + // Write the content of buf to os. // It is a separate function rather than a part of vprint to simplify testing. template void write_buffer(std::basic_ostream& os, buffer& buf) { - if (const_check(FMT_MSC_VERSION)) - if (write_ostream_msvc_unicode(os, {buf.data(), buf.size()})) return; +#if FMT_MSC_VERSION + if (write_ostream_msvc_unicode(os, {buf.data(), buf.size()})) return; +#elif _WIN32 && __GLIBCXX__ + if (write_ostream_gcc_mingw_unicode(os, {buf.data(), buf.size()})) return; +#endif const Char* buf_data = buf.data(); using unsigned_streamsize = std::make_unsigned::type; unsigned_streamsize size = buf.size(); From 382f60850d6b68778285b5726403c026aefbb052 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Wed, 20 Jul 2022 17:57:08 +0200 Subject: [PATCH 4/9] Add TODO note about detail::is_utf8() --- include/fmt/ostream.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index 1eb9fd746241..4a0df9873a02 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -121,6 +121,7 @@ inline bool write_ostream_gcc_mingw_unicode(std::wostream&, // It is a separate function rather than a part of vprint to simplify testing. template void write_buffer(std::basic_ostream& os, buffer& buf) { + // TODO: check detail::is_utf8(). Here or bellow in print or in vprint? #if FMT_MSC_VERSION if (write_ostream_msvc_unicode(os, {buf.data(), buf.size()})) return; #elif _WIN32 && __GLIBCXX__ From fe2e381de570881352ebb621f5e39234f2d8efc9 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Wed, 20 Jul 2022 21:04:59 +0200 Subject: [PATCH 5/9] Fix warning -Wundef --- include/fmt/ostream.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index 4a0df9873a02..8c02b0a0e7ec 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -10,7 +10,7 @@ #include #include -#if _WIN32 && __GLIBCXX__ +#if defined(_WIN32) && defined(__GLIBCXX__) # include # include #endif @@ -96,7 +96,7 @@ inline bool write_ostream_msvc_unicode(std::wostream&, return false; } -#if _WIN32 && __GLIBCXX__ +#if defined(_WIN32) && defined(__GLIBCXX__) inline bool write_ostream_gcc_mingw_unicode(std::ostream& os, fmt::string_view data) { auto* rdbuf = os.rdbuf(); @@ -124,7 +124,7 @@ void write_buffer(std::basic_ostream& os, buffer& buf) { // TODO: check detail::is_utf8(). Here or bellow in print or in vprint? #if FMT_MSC_VERSION if (write_ostream_msvc_unicode(os, {buf.data(), buf.size()})) return; -#elif _WIN32 && __GLIBCXX__ +#elif defined(_WIN32) && defined(__GLIBCXX__) if (write_ostream_gcc_mingw_unicode(os, {buf.data(), buf.size()})) return; #endif const Char* buf_data = buf.data(); From 4230d7481ea1b7a2c211143f6cb6dd94962344fd Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Wed, 20 Jul 2022 21:24:24 +0200 Subject: [PATCH 6/9] Fix for non-Windows OSs --- include/fmt/format.h | 2 ++ include/fmt/ostream.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/include/fmt/format.h b/include/fmt/format.h index 40f15d155129..aec91fae713f 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -921,7 +921,9 @@ struct is_contiguous> : std::true_type { }; namespace detail { +#ifdef _WIN32 bool write_console_on_windows(std::FILE* f, string_view text); +#endif FMT_API void print(std::FILE*, string_view); } diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index 8c02b0a0e7ec..acbf507a34ea 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -87,8 +87,10 @@ template class filebuf_access(os.rdbuf())) if (FILE* f = get_file(*buf)) return write_console_on_windows(f, data); +#endif return false; } inline bool write_ostream_msvc_unicode(std::wostream&, From 7a94e609887b165915e7eaac3fa180944a2c9722 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Wed, 20 Jul 2022 21:37:23 +0200 Subject: [PATCH 7/9] Fix building as DLL on Windows --- include/fmt/format.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fmt/format.h b/include/fmt/format.h index aec91fae713f..ab20887b7e44 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -922,7 +922,7 @@ struct is_contiguous> : std::true_type { namespace detail { #ifdef _WIN32 -bool write_console_on_windows(std::FILE* f, string_view text); +FMT_API bool write_console_on_windows(std::FILE* f, string_view text); #endif FMT_API void print(std::FILE*, string_view); } From 5fd371ebe16bb2c39eb96d82fa0661ba82b7e0b2 Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Sat, 23 Jul 2022 04:09:22 +0200 Subject: [PATCH 8/9] Refactor --- include/fmt/format-inl.h | 4 ++-- include/fmt/format.h | 2 +- include/fmt/ostream.h | 38 ++++++++++++-------------------------- 3 files changed, 15 insertions(+), 29 deletions(-) diff --git a/include/fmt/format-inl.h b/include/fmt/format-inl.h index 61256d2ccc7b..22b1ec8df0eb 100644 --- a/include/fmt/format-inl.h +++ b/include/fmt/format-inl.h @@ -1479,7 +1479,7 @@ using dword = conditional_t; extern "C" __declspec(dllimport) int __stdcall WriteConsoleW( // void*, const void*, dword, dword*, void*); -FMT_FUNC bool write_console_on_windows(std::FILE* f, string_view text) { +FMT_FUNC bool write_console(std::FILE* f, string_view text) { auto fd = _fileno(f); if (_isatty(fd)) { detail::utf8_to_utf16 u16(string_view(text.data(), text.size())); @@ -1500,7 +1500,7 @@ FMT_FUNC bool write_console_on_windows(std::FILE* f, string_view text) { FMT_FUNC void print(std::FILE* f, string_view text) { #ifdef _WIN32 - if (write_console_on_windows(f, text)) return; + if (write_console(f, text)) return; #endif detail::fwrite_fully(text.data(), 1, text.size(), f); } diff --git a/include/fmt/format.h b/include/fmt/format.h index ab20887b7e44..0615666d1952 100644 --- a/include/fmt/format.h +++ b/include/fmt/format.h @@ -922,7 +922,7 @@ struct is_contiguous> : std::true_type { namespace detail { #ifdef _WIN32 -FMT_API bool write_console_on_windows(std::FILE* f, string_view text); +FMT_API bool write_console(std::FILE* f, string_view text); #endif FMT_API void print(std::FILE*, string_view); } diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index acbf507a34ea..fcab4dd3810c 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -85,22 +85,11 @@ template class filebuf_access; -inline bool write_ostream_msvc_unicode(std::ostream& os, - fmt::string_view data) { -#ifdef _WIN32 +inline bool write_ostream_unicode(std::ostream& os, fmt::string_view data) { +#if FMT_MSC_VERSION if (auto* buf = dynamic_cast(os.rdbuf())) - if (FILE* f = get_file(*buf)) return write_console_on_windows(f, data); -#endif - return false; -} -inline bool write_ostream_msvc_unicode(std::wostream&, - fmt::basic_string_view) { - return false; -} - -#if defined(_WIN32) && defined(__GLIBCXX__) -inline bool write_ostream_gcc_mingw_unicode(std::ostream& os, - fmt::string_view data) { + if (FILE* f = get_file(*buf)) return write_console(f, data); +#elif defined(_WIN32) && defined(__GLIBCXX__) auto* rdbuf = os.rdbuf(); FILE* c_file; if (auto* fbuf = dynamic_cast<__gnu_cxx::stdio_sync_filebuf*>(rdbuf)) @@ -109,26 +98,23 @@ inline bool write_ostream_gcc_mingw_unicode(std::ostream& os, c_file = fbuf->file(); else return false; - if (c_file) return write_console_on_windows(c_file, data); + if (c_file) return write_console(c_file, data); +#else + (void)os; // suppress warning + (void)data; +#endif return false; } - -inline bool write_ostream_gcc_mingw_unicode(std::wostream&, - fmt::basic_string_view) { +inline bool write_ostream_unicode(std::wostream&, + fmt::basic_string_view) { return false; } -#endif // Write the content of buf to os. // It is a separate function rather than a part of vprint to simplify testing. template void write_buffer(std::basic_ostream& os, buffer& buf) { - // TODO: check detail::is_utf8(). Here or bellow in print or in vprint? -#if FMT_MSC_VERSION - if (write_ostream_msvc_unicode(os, {buf.data(), buf.size()})) return; -#elif defined(_WIN32) && defined(__GLIBCXX__) - if (write_ostream_gcc_mingw_unicode(os, {buf.data(), buf.size()})) return; -#endif + if (write_ostream_unicode(os, {buf.data(), buf.size()})) return; const Char* buf_data = buf.data(); using unsigned_streamsize = std::make_unsigned::type; unsigned_streamsize size = buf.size(); From 72737d008d2e5ad9cc5b4be19d269628089513fc Mon Sep 17 00:00:00 2001 From: Dimitrij Mijoski Date: Sat, 23 Jul 2022 12:22:33 +0200 Subject: [PATCH 9/9] Suppress warning --- include/fmt/ostream.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/fmt/ostream.h b/include/fmt/ostream.h index fcab4dd3810c..641fd08bfae7 100644 --- a/include/fmt/ostream.h +++ b/include/fmt/ostream.h @@ -100,8 +100,8 @@ inline bool write_ostream_unicode(std::ostream& os, fmt::string_view data) { return false; if (c_file) return write_console(c_file, data); #else - (void)os; // suppress warning - (void)data; + ignore_unused(os); + ignore_unused(data); #endif return false; }