From ebeb3707b157ff386d8d78947cc2253858efb9bc Mon Sep 17 00:00:00 2001 From: Charles Milette Date: Thu, 21 Apr 2022 21:46:58 -0400 Subject: [PATCH] Switch to vformat_to Drive-by: reduce the amount of occurences of #ifdef SPDLOG_USE_STD_FORMAT --- include/spdlog/common.h | 14 +++--- include/spdlog/details/fmt_helper.h | 14 ++++++ include/spdlog/details/os-inl.h | 6 +-- include/spdlog/logger.h | 17 ++----- include/spdlog/sinks/msvc_sink.h | 7 +-- include/spdlog/sinks/ringbuffer_sink.h | 6 +-- tests/test_daily_logger.cpp | 55 ++++++-------------- tests/test_errors.cpp | 18 ++----- tests/test_fmt_helper.cpp | 25 ++------- tests/test_pattern_formatter.cpp | 70 +++++--------------------- 10 files changed, 66 insertions(+), 166 deletions(-) diff --git a/include/spdlog/common.h b/include/spdlog/common.h index 8259c733..f0b2a996 100644 --- a/include/spdlog/common.h +++ b/include/spdlog/common.h @@ -44,15 +44,13 @@ #include -#ifndef SPDLOG_USE_STD_FORMAT -# if FMT_VERSION >= 80000 // backward compatibility with fmt versions older than 8 -# define SPDLOG_FMT_RUNTIME(format_string) fmt::runtime(format_string) -# if defined(SPDLOG_WCHAR_FILENAMES) || defined(SPDLOG_WCHAR_TO_UTF8_SUPPORT) -# include -# endif -# else -# define SPDLOG_FMT_RUNTIME(format_string) format_string +#if !defined(SPDLOG_USE_STD_FORMAT) && FMT_VERSION >= 80000 // backward compatibility with fmt versions older than 8 +# define SPDLOG_FMT_RUNTIME(format_string) fmt::runtime(format_string) +# if defined(SPDLOG_WCHAR_FILENAMES) || defined(SPDLOG_WCHAR_TO_UTF8_SUPPORT) +# include # endif +#else +# define SPDLOG_FMT_RUNTIME(format_string) format_string #endif // visual studio up to 2013 does not support noexcept nor constexpr diff --git a/include/spdlog/details/fmt_helper.h b/include/spdlog/details/fmt_helper.h index 1a60bc0d..9a1b69d3 100644 --- a/include/spdlog/details/fmt_helper.h +++ b/include/spdlog/details/fmt_helper.h @@ -23,6 +23,20 @@ inline spdlog::string_view_t to_string_view(const memory_buf_t &buf) SPDLOG_NOEX return spdlog::string_view_t{buf.data(), buf.size()}; } +#ifdef SPDLOG_USE_STD_FORMAT +template +std::basic_string to_string(const std::basic_string &buf) +{ + return buf; +} +#else +template +std::basic_string to_string(const fmt::basic_memory_buffer &buf) +{ + return fmt::to_string(buf); +} +#endif + inline void append_string_view(spdlog::string_view_t view, memory_buf_t &dest) { auto *buf_ptr = view.data(); diff --git a/include/spdlog/details/os-inl.h b/include/spdlog/details/os-inl.h index c3bf691c..aa4ff709 100644 --- a/include/spdlog/details/os-inl.h +++ b/include/spdlog/details/os-inl.h @@ -388,11 +388,7 @@ SPDLOG_INLINE std::string filename_to_str(const filename_t &filename) { memory_buf_t buf; wstr_to_utf8buf(filename, buf); -# ifdef SPDLOG_USE_STD_FORMAT - return buf; -# else - return fmt::to_string(buf); -# endif + return fmt_helper::to_string(buf); } #else SPDLOG_INLINE std::string filename_to_str(const filename_t &filename) diff --git a/include/spdlog/logger.h b/include/spdlog/logger.h index a8abae41..9f4bc1a6 100644 --- a/include/spdlog/logger.h +++ b/include/spdlog/logger.h @@ -363,13 +363,10 @@ protected: } SPDLOG_TRY { -#ifdef SPDLOG_USE_STD_FORMAT - memory_buf_t buf = std::vformat(fmt, std::make_format_args(std::forward(args)...)); -#else memory_buf_t buf; - fmt::detail::vformat_to(buf, fmt, fmt::make_format_args(std::forward(args)...)); -#endif - details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size())); + fmt_lib::vformat_to(std::back_inserter(buf), fmt, fmt_lib::make_format_args(std::forward(args)...)); + + details::log_msg log_msg(loc, name_, lvl, details::fmt_helper::to_string_view(buf)); log_it_(log_msg, log_enabled, traceback_enabled); } SPDLOG_LOGGER_CATCH(loc) @@ -388,13 +385,9 @@ protected: SPDLOG_TRY { // format to wmemory_buffer and convert to utf8 - ; -# ifdef SPDLOG_USE_STD_FORMAT - wmemory_buf_t wbuf = std::vformat(fmt, std::make_wformat_args(std::forward(args)...)); -# else wmemory_buf_t wbuf; - fmt::detail::vformat_to(wbuf, fmt, fmt::make_format_args(std::forward(args)...)); -# endif + fmt_lib::vformat_to(std::back_inserter(wbuf), fmt, fmt_lib::make_format_args(std::forward(args)...)); + memory_buf_t buf; details::os::wstr_to_utf8buf(wstring_view_t(wbuf.data(), wbuf.size()), buf); details::log_msg log_msg(loc, name_, lvl, string_view_t(buf.data(), buf.size())); diff --git a/include/spdlog/sinks/msvc_sink.h b/include/spdlog/sinks/msvc_sink.h index cf2516d7..b1dfd305 100644 --- a/include/spdlog/sinks/msvc_sink.h +++ b/include/spdlog/sinks/msvc_sink.h @@ -30,11 +30,8 @@ protected: { memory_buf_t formatted; base_sink::formatter_->format(msg, formatted); -# ifdef SPDLOG_USE_STD_FORMAT - OutputDebugStringA(formatted.c_str()); -# else - OutputDebugStringA(fmt::to_string(formatted).c_str()); -# endif + formatted.push_back('\0'); // add a null terminator for OutputDebugStringA + OutputDebugStringA(formatted.data()); } void flush_() override {} diff --git a/include/spdlog/sinks/ringbuffer_sink.h b/include/spdlog/sinks/ringbuffer_sink.h index 5836d98b..de31c99c 100644 --- a/include/spdlog/sinks/ringbuffer_sink.h +++ b/include/spdlog/sinks/ringbuffer_sink.h @@ -50,11 +50,7 @@ public: { memory_buf_t formatted; base_sink::formatter_->format(q_.at(i), formatted); -#ifdef SPDLOG_USE_STD_FORMAT - ret.push_back(std::move(formatted)); -#else - ret.push_back(fmt::to_string(formatted)); -#endif + ret.push_back(fmt_helper::to_string(formatted)); } return ret; } diff --git a/tests/test_daily_logger.cpp b/tests/test_daily_logger.cpp index cf93c46e..8cd9d206 100644 --- a/tests/test_daily_logger.cpp +++ b/tests/test_daily_logger.cpp @@ -5,8 +5,20 @@ #ifdef SPDLOG_USE_STD_FORMAT using filename_memory_buf_t = std::basic_string; + +std::string filename_buf_to_utf8string(const filename_memory_buf_t &w) +{ + spdlog::memory_buf_t buf; + spdlog::details::os::wstr_to_utf8buf(w, buf); + return spdlog::details::fmt_helper::to_string(buf); +} #else using filename_memory_buf_t = fmt::basic_memory_buffer; + +std::string filename_buf_to_utf8string(const filename_memory_buf_t &w) +{ + return spdlog::details::fmt_helper::to_string(w); +} #endif TEST_CASE("daily_logger with dateonly calculator", "[daily_logger]") @@ -30,23 +42,7 @@ TEST_CASE("daily_logger with dateonly calculator", "[daily_logger]") } logger->flush(); -#ifdef SPDLOG_WCHAR_FILENAMES - spdlog::memory_buf_t buf; -# ifdef SPDLOG_USE_STD_FORMAT - spdlog::details::os::wstr_to_utf8buf(w, buf); - auto &filename = buf; -# else - spdlog::details::os::wstr_to_utf8buf(fmt::to_string(w), buf); - auto filename = fmt::to_string(buf); -# endif -#else -# ifdef SPDLOG_USE_STD_FORMAT - auto &filename = w; -# else - auto filename = fmt::to_string(w); -# endif -#endif - require_message_count(filename, 10); + require_message_count(filename_buf_to_utf8string(w), 10); } struct custom_daily_file_name_calculator @@ -56,11 +52,8 @@ struct custom_daily_file_name_calculator filename_memory_buf_t w; spdlog::fmt_lib::format_to(std::back_inserter(w), SPDLOG_FILENAME_T("{}{:04d}{:02d}{:02d}"), basename, now_tm.tm_year + 1900, now_tm.tm_mon + 1, now_tm.tm_mday); -#ifdef SPDLOG_USE_STD_FORMAT - return w; -#else - return fmt::to_string(w); -#endif + + return spdlog::details::fmt_helper::to_string(w); } }; @@ -85,23 +78,7 @@ TEST_CASE("daily_logger with custom calculator", "[daily_logger]") logger->flush(); -#ifdef SPDLOG_WCHAR_FILENAMES - spdlog::memory_buf_t buf; -# ifdef SPDLOG_USE_STD_FORMAT - spdlog::details::os::wstr_to_utf8buf(w, buf); - auto &filename = buf; -# else - spdlog::details::os::wstr_to_utf8buf(fmt::to_string(w), buf); - auto filename = fmt::to_string(buf); -# endif -#else -# ifdef SPDLOG_USE_STD_FORMAT - auto &filename = w; -# else - auto filename = fmt::to_string(w); -# endif -#endif - require_message_count(filename, 10); + require_message_count(filename_buf_to_utf8string(w), 10); } /* diff --git a/tests/test_errors.cpp b/tests/test_errors.cpp index c7aef812..896e5b0a 100644 --- a/tests/test_errors.cpp +++ b/tests/test_errors.cpp @@ -29,11 +29,7 @@ TEST_CASE("default_error_handler", "[errors]]") auto logger = spdlog::create("test-error", filename, true); logger->set_pattern("%v"); -#ifdef SPDLOG_USE_STD_FORMAT - logger->info("Test message {} {}", 1); -#else - logger->info(fmt::runtime("Test message {} {}"), 1); -#endif + logger->info(SPDLOG_FMT_RUNTIME("Test message {} {}"), 1); logger->info("Test message {}", 2); logger->flush(); @@ -53,11 +49,7 @@ TEST_CASE("custom_error_handler", "[errors]]") logger->set_error_handler([=](const std::string &) { throw custom_ex(); }); logger->info("Good message #1"); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE_THROWS_AS(logger->info("Bad format msg {} {}", "xxx"), custom_ex); -#else - REQUIRE_THROWS_AS(logger->info(fmt::runtime("Bad format msg {} {}"), "xxx"), custom_ex); -#endif + REQUIRE_THROWS_AS(logger->info(SPDLOG_FMT_RUNTIME("Bad format msg {} {}"), "xxx"), custom_ex); logger->info("Good message #2"); require_message_count(SIMPLE_LOG, 2); } @@ -96,11 +88,7 @@ TEST_CASE("async_error_handler", "[errors]]") ofs << err_msg; }); logger->info("Good message #1"); -#ifdef SPDLOG_USE_STD_FORMAT - logger->info("Bad format msg {} {}", "xxx"); -#else - logger->info(fmt::runtime("Bad format msg {} {}"), "xxx"); -#endif + logger->info(SPDLOG_FMT_RUNTIME("Bad format msg {} {}"), "xxx"); logger->info("Good message #2"); spdlog::drop("logger"); // force logger to drain the queue and shutdown } diff --git a/tests/test_fmt_helper.cpp b/tests/test_fmt_helper.cpp index dde5d482..a7edccf3 100644 --- a/tests/test_fmt_helper.cpp +++ b/tests/test_fmt_helper.cpp @@ -3,17 +3,14 @@ #include "spdlog/details/fmt_helper.h" using spdlog::memory_buf_t; +using spdlog::details::fmt_helper::to_string_view; void test_pad2(int n, const char *expected) { memory_buf_t buf; spdlog::details::fmt_helper::pad2(n, buf); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(buf == expected); -#else - REQUIRE(fmt::to_string(buf) == expected); -#endif + REQUIRE(to_string_view(buf) == expected); } void test_pad3(uint32_t n, const char *expected) @@ -21,11 +18,7 @@ void test_pad3(uint32_t n, const char *expected) memory_buf_t buf; spdlog::details::fmt_helper::pad3(n, buf); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(buf == expected); -#else - REQUIRE(fmt::to_string(buf) == expected); -#endif + REQUIRE(to_string_view(buf) == expected); } void test_pad6(std::size_t n, const char *expected) @@ -33,11 +26,7 @@ void test_pad6(std::size_t n, const char *expected) memory_buf_t buf; spdlog::details::fmt_helper::pad6(n, buf); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(buf == expected); -#else - REQUIRE(fmt::to_string(buf) == expected); -#endif + REQUIRE(to_string_view(buf) == expected); } void test_pad9(std::size_t n, const char *expected) @@ -45,11 +34,7 @@ void test_pad9(std::size_t n, const char *expected) memory_buf_t buf; spdlog::details::fmt_helper::pad9(n, buf); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(buf == expected); -#else - REQUIRE(fmt::to_string(buf) == expected); -#endif + REQUIRE(to_string_view(buf) == expected); } TEST_CASE("pad2", "[fmt_helper]") diff --git a/tests/test_pattern_formatter.cpp b/tests/test_pattern_formatter.cpp index 02a1bc2b..e4ea6cc3 100644 --- a/tests/test_pattern_formatter.cpp +++ b/tests/test_pattern_formatter.cpp @@ -2,6 +2,7 @@ #include "test_sink.h" using spdlog::memory_buf_t; +using spdlog::details::fmt_helper::to_string_view; // log to str and return it template @@ -273,11 +274,7 @@ TEST_CASE("clone-default-formatter", "[pattern_formatter]") formatter_1->format(msg, formatted_1); formatter_2->format(msg, formatted_2); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted_1 == formatted_2); -#else - REQUIRE(fmt::to_string(formatted_1) == fmt::to_string(formatted_2)); -#endif + REQUIRE(to_string_view(formatted_1) == to_string_view(formatted_2)); } TEST_CASE("clone-default-formatter2", "[pattern_formatter]") @@ -292,11 +289,7 @@ TEST_CASE("clone-default-formatter2", "[pattern_formatter]") formatter_1->format(msg, formatted_1); formatter_2->format(msg, formatted_2); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted_1 == formatted_2); -#else - REQUIRE(fmt::to_string(formatted_1) == fmt::to_string(formatted_2)); -#endif + REQUIRE(to_string_view(formatted_1) == to_string_view(formatted_2)); } TEST_CASE("clone-formatter", "[pattern_formatter]") @@ -311,11 +304,7 @@ TEST_CASE("clone-formatter", "[pattern_formatter]") formatter_1->format(msg, formatted_1); formatter_2->format(msg, formatted_2); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted_1 == formatted_2); -#else - REQUIRE(fmt::to_string(formatted_1) == fmt::to_string(formatted_2)); -#endif + REQUIRE(to_string_view(formatted_1) == to_string_view(formatted_2)); } TEST_CASE("clone-formatter-2", "[pattern_formatter]") @@ -331,11 +320,7 @@ TEST_CASE("clone-formatter-2", "[pattern_formatter]") formatter_1->format(msg, formatted_1); formatter_2->format(msg, formatted_2); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted_1 == formatted_2); -#else - REQUIRE(fmt::to_string(formatted_1) == fmt::to_string(formatted_2)); -#endif + REQUIRE(to_string_view(formatted_1) == to_string_view(formatted_2)); } class custom_test_flag : public spdlog::custom_flag_formatter @@ -382,13 +367,8 @@ TEST_CASE("clone-custom_formatter", "[pattern_formatter]") auto expected = spdlog::fmt_lib::format("[logger-name] [custom_output] some message{}", spdlog::details::os::default_eol); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted_1 == expected); - REQUIRE(formatted_2 == expected); -#else - REQUIRE(fmt::to_string(formatted_1) == expected); - REQUIRE(fmt::to_string(formatted_2) == expected); -#endif + REQUIRE(to_string_view(formatted_1) == expected); + REQUIRE(to_string_view(formatted_2) == expected); } // @@ -410,11 +390,7 @@ TEST_CASE("short filename formatter-1", "[pattern_formatter]") spdlog::details::log_msg msg(source_loc, "logger-name", spdlog::level::info, "Hello"); formatter.format(msg, formatted); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted == "myfile.cpp"); -#else - REQUIRE(fmt::to_string(formatted) == "myfile.cpp"); -#endif + REQUIRE(to_string_view(formatted) == "myfile.cpp"); } TEST_CASE("short filename formatter-2", "[pattern_formatter]") @@ -426,11 +402,7 @@ TEST_CASE("short filename formatter-2", "[pattern_formatter]") spdlog::details::log_msg msg(source_loc, "logger-name", spdlog::level::info, "Hello"); formatter.format(msg, formatted); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted == "myfile.cpp:123"); -#else - REQUIRE(fmt::to_string(formatted) == "myfile.cpp:123"); -#endif + REQUIRE(to_string_view(formatted) == "myfile.cpp:123"); } TEST_CASE("short filename formatter-3", "[pattern_formatter]") @@ -442,11 +414,7 @@ TEST_CASE("short filename formatter-3", "[pattern_formatter]") spdlog::details::log_msg msg(source_loc, "logger-name", spdlog::level::info, "Hello"); formatter.format(msg, formatted); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted == " Hello"); -#else - REQUIRE(fmt::to_string(formatted) == " Hello"); -#endif + REQUIRE(to_string_view(formatted) == " Hello"); } TEST_CASE("full filename formatter", "[pattern_formatter]") @@ -458,11 +426,7 @@ TEST_CASE("full filename formatter", "[pattern_formatter]") spdlog::details::log_msg msg(source_loc, "logger-name", spdlog::level::info, "Hello"); formatter.format(msg, formatted); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted == test_path); -#else - REQUIRE(fmt::to_string(formatted) == test_path); -#endif + REQUIRE(to_string_view(formatted) == test_path); } TEST_CASE("custom flags", "[pattern_formatter]") @@ -476,11 +440,7 @@ TEST_CASE("custom flags", "[pattern_formatter]") formatter->format(msg, formatted); auto expected = spdlog::fmt_lib::format("[logger-name] [custom1] [custom2] some message{}", spdlog::details::os::default_eol); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted == expected); -#else - REQUIRE(fmt::to_string(formatted) == expected); -#endif + REQUIRE(to_string_view(formatted) == expected); } TEST_CASE("custom flags-padding", "[pattern_formatter]") @@ -494,11 +454,7 @@ TEST_CASE("custom flags-padding", "[pattern_formatter]") formatter->format(msg, formatted); auto expected = spdlog::fmt_lib::format("[logger-name] [custom1] [ custom2] some message{}", spdlog::details::os::default_eol); -#ifdef SPDLOG_USE_STD_FORMAT - REQUIRE(formatted == expected); -#else - REQUIRE(fmt::to_string(formatted) == expected); -#endif + REQUIRE(to_string_view(formatted) == expected); } TEST_CASE("custom flags-exception", "[pattern_formatter]")