Avoid specializing std::formatter for std::tm (not a great idea after all)

5 files changed, 73 insertions, 95 deletions

diff --git a/include/spdlog/details/fmt_helper.h b/include/spdlog/details/fmt_helper.h
index 25976f50..0a8de309 100644
--- a/include/spdlog/details/fmt_helper.h
+++ b/include/spdlog/details/fmt_helper.h
@@ -164,6 +164,23 @@ inline ToDuration time_fraction(log_clock::time_point tp)
     return duration_cast<ToDuration>(duration) - duration_cast<ToDuration>(secs);
 }
 
+inline size_t strftime(char *str, size_t count, const char *format, const std::tm *time)
+{
+    // Assign to a pointer to suppress GCCs -Wformat-nonliteral
+    // First assign the nullptr to suppress -Wsuggest-attribute=format
+    std::size_t (*strftime)(char*, std::size_t, const char*, const std::tm*) = nullptr;
+    strftime = std::strftime;
+    return strftime(str, count, format, time);
+}
+
+inline size_t strftime(wchar_t *str, size_t count, const wchar_t *format, const std::tm *time)
+{
+    // See above
+    std::size_t (*wcsftime)(wchar_t*, std::size_t, const wchar_t*, const std::tm*) = nullptr;
+    wcsftime = std::wcsftime;
+    return wcsftime(str, count, format, time);
+}
+
 } // namespace fmt_helper
 } // namespace details
 } // namespace spdlog
diff --git a/include/spdlog/fmt/chrono.h b/include/spdlog/fmt/chrono.h
index 17013bc1..83fad2ff 100644
--- a/include/spdlog/fmt/chrono.h
+++ b/include/spdlog/fmt/chrono.h
@@ -4,93 +4,11 @@
 //
 
 #pragma once
-
-#if defined(SPDLOG_USE_STD_FORMAT)
-// Add a formatter for std::tm, since std::format only supports std::chrono
-// taken from fmtlib
-#include <cassert>
-#include <ctime>
-#include <format>
-#include <string>
-#include <string_view>
-#include <type_traits>
-#include <spdlog/common.h>
-
-namespace spdlog::details
-{
-    inline size_t strftime(char *str, size_t count, const char *format, const std::tm *time)
-    {
-        // Assign to a pointer to suppress GCCs -Wformat-nonliteral
-        // First assign the nullptr to suppress -Wsuggest-attribute=format
-        std::size_t (*strftime)(char*, std::size_t, const char*, const std::tm*) = nullptr;
-        strftime = std::strftime;
-        return strftime(str, count, format, time);
-    }
-
-    inline size_t strftime(wchar_t *str, size_t count, const wchar_t *format, const std::tm *time)
-    {
-        // See above
-        std::size_t (*wcsftime)(wchar_t*, std::size_t, const wchar_t*, const std::tm*) = nullptr;
-        wcsftime = std::wcsftime;
-        return wcsftime(str, count, format, time);
-    }
-
-    // Casts a nonnegative integer to unsigned.
-    template <typename Int>
-    SPDLOG_CONSTEXPR auto to_unsigned(Int value) -> typename std::make_unsigned<Int>::type
-    {
-        assert(value >= 0, "negative value");
-        return static_cast<typename std::make_unsigned<Int>::type>(value);
-    }
-}
-
-template <typename Char>
-struct std::formatter<std::tm, Char>
-{
-    template <typename ParseContext>
-    SPDLOG_CONSTEXPR_FUNC auto parse(ParseContext &ctx) -> decltype(ctx.begin())
-    {
-        auto it = ctx.begin();
-        if (it != ctx.end() && *it == ':') ++it;
-        auto end = it;
-        while (end != ctx.end() && *end != '}') ++end;
-        specs = {it, spdlog::details::to_unsigned(end - it)};
-        return end;
-    }
-
-    template <typename FormatContext>
-    auto format(const std::tm &tm, FormatContext &ctx) const -> decltype(ctx.out()) {
-        basic_string<Char> tm_format;
-        tm_format.append(specs);
-        // By appending an extra space we can distinguish an empty result that
-        // indicates insufficient buffer size from a guaranteed non-empty result
-        // https://github.com/fmtlib/fmt/issues/2238
-        tm_format.push_back(' ');
-
-        const size_t MIN_SIZE = 10;
-        basic_string<Char> buf;
-        buf.resize(MIN_SIZE);
-        for (;;)
-        {
-            size_t count = spdlog::details::strftime(buf.data(), buf.size(), tm_format.c_str(), &tm);
-            if (count != 0)
-            {
-                buf.resize(count);
-                break;
-            }
-            buf.resize(buf.size() * 2);
-        }
-        // Remove the extra space.
-        return std::copy(buf.begin(), buf.end() - 1, ctx.out());
-    }
-
-    basic_string_view<Char> specs;
-};
-
-#else
 //
 // include bundled or external copy of fmtlib's chrono support
 //
+
+#if !defined(SPDLOG_USE_STD_FORMAT)
 #    if !defined(SPDLOG_FMT_EXTERNAL)
 #        ifdef SPDLOG_HEADER_ONLY
 #            ifndef FMT_HEADER_ONLY
diff --git a/include/spdlog/sinks/daily_file_sink.h b/include/spdlog/sinks/daily_file_sink.h
index e56ab763..0e41612a 100644
--- a/include/spdlog/sinks/daily_file_sink.h
+++ b/include/spdlog/sinks/daily_file_sink.h
@@ -5,9 +5,9 @@
 
 #include <spdlog/common.h>
 #include <spdlog/details/file_helper.h>
+#include <spdlog/details/fmt_helper.h>
 #include <spdlog/details/null_mutex.h>
 #include <spdlog/fmt/fmt.h>
-#include <spdlog/fmt/chrono.h>
 #include <spdlog/sinks/base_sink.h>
 #include <spdlog/details/os.h>
 #include <spdlog/details/circular_q.h>
@@ -48,14 +48,57 @@ struct daily_filename_format_calculator
 {
     static filename_t calc_filename(const filename_t &filename, const tm &now_tm)
     {
-        // generate fmt datetime format string, e.g. {:%Y-%m-%d}.
-        filename_t fmt_filename = fmt_lib::format(SPDLOG_FILENAME_T("{{:{}}}"), filename);
-#if defined(SPDLOG_USE_STD_FORMAT)
-        return std::vformat(fmt_filename, std::make_format_args(now_tm));
-#elif defined(_MSC_VER) && defined(SPDLOG_WCHAR_FILENAMES) // for some reason msvc doesn't allow fmt::runtime(..) with wchar here
-        return fmt::format(fmt_filename, now_tm);
+        // adapted from fmtlib
+#ifdef SPDLOG_USE_STD_FORMAT
+        filename_t tm_format;
+        tm_format.append(filename);
+        // By appending an extra space we can distinguish an empty result that
+        // indicates insufficient buffer size from a guaranteed non-empty result
+        // https://github.com/fmtlib/fmt/issues/2238
+        tm_format.push_back(' ');
+
+        const size_t MIN_SIZE = 10;
+        filename_t buf;
+        buf.resize(MIN_SIZE);
+        for (;;)
+        {
+            size_t count = details::fmt_helper::strftime(buf.data(), buf.size(), tm_format.c_str(), &now_tm);
+            if (count != 0)
+            {
+                // Remove the extra space.
+                buf.resize(count - 1);
+                break;
+            }
+            buf.resize(buf.size() * 2);
+        }
+        
+        return buf;
 #else
-        return fmt::format(SPDLOG_FMT_RUNTIME(fmt_filename), now_tm);
+    fmt::basic_memory_buffer<Char> tm_format;
+    tm_format.append(specs.begin(), specs.end());
+    // By appending an extra space we can distinguish an empty result that
+    // indicates insufficient buffer size from a guaranteed non-empty result
+    // https://github.com/fmtlib/fmt/issues/2238
+    tm_format.push_back(' ');
+    tm_format.push_back('\0');
+
+    fmt::basic_memory_buffer<Char> buf;
+    size_t start = buf.size();
+    for (;;)
+    {
+        size_t size = buf.capacity() - start;
+        size_t count = details::fmt_helper:::strftime(&buf[start], size, &tm_format[0], &tm);
+        if (count != 0)
+        {
+            // Remove the extra space.
+            buf.resize(start + count - 1);
+            break;
+        }
+        const size_t MIN_GROWTH = 10;
+        buf.reserve(buf.capacity() + (size > MIN_GROWTH ? size : MIN_GROWTH));
+    }
+
+    return fmt::to_string(buf);
 #endif
     }
 };
diff --git a/include/spdlog/stopwatch.h b/include/spdlog/stopwatch.h
index 52b4b6ad..a1542b2d 100644
--- a/include/spdlog/stopwatch.h
+++ b/include/spdlog/stopwatch.h
@@ -42,7 +42,7 @@ public:
 
     void reset()
     {
-        start_tp_ = clock ::now();
+        start_tp_ = clock::now();
     }
 };
 } // namespace spdlog
diff --git a/tests/test_file_logging.cpp b/tests/test_file_logging.cpp
index eab524dc..c808916d 100644
--- a/tests/test_file_logging.cpp
+++ b/tests/test_file_logging.cpp
@@ -20,7 +20,7 @@ TEST_CASE("simple_file_logger", "[simple_logger]]")
     logger->flush();
     require_message_count(SIMPLE_LOG, 2);
     using spdlog::details::os::default_eol;
-    REQUIRE(file_contents(SIMPLE_LOG) == fmt::format("Test message 1{}Test message 2{}", default_eol, default_eol));
+    REQUIRE(file_contents(SIMPLE_LOG) == spdlog::fmt_lib::format("Test message 1{}Test message 2{}", default_eol, default_eol));
 }
 
 TEST_CASE("flush_on", "[flush_on]]")
@@ -41,7 +41,7 @@ TEST_CASE("flush_on", "[flush_on]]")
     require_message_count(SIMPLE_LOG, 3);
     using spdlog::details::os::default_eol;
     REQUIRE(file_contents(SIMPLE_LOG) ==
-            fmt::format("Should not be flushed{}Test message 1{}Test message 2{}", default_eol, default_eol, default_eol));
+            spdlog::fmt_lib::format("Should not be flushed{}Test message 1{}Test message 2{}", default_eol, default_eol, default_eol));
 }
 
 TEST_CASE("rotating_file_logger1", "[rotating_logger]]")