From 115001a3b165483563822645bb93fd9558908c50 Mon Sep 17 00:00:00 2001 From: "Niall Douglas (s [underscore] sourceforge {at} nedprod [dot] com)" Date: Fri, 16 Dec 2022 16:23:52 +0000 Subject: [PATCH] Formatting of system clocks ought to be to UTC, not to local time. This improves standards conformance of fmt. --- include/fmt/chrono.h | 48 ++++++++++++++++++++- test/chrono-test.cc | 100 +++++++++++++++++++++++++++++++++++++------ test/xchar-test.cc | 2 +- 3 files changed, 133 insertions(+), 17 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 53f861fa8ce1..9557a377e0da 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -22,6 +22,15 @@ FMT_BEGIN_NAMESPACE +// Check if std::chrono::local_t is available. +#ifndef FMT_USE_LOCAL_TIME +# ifdef __cpp_lib_chrono +# define FMT_USE_LOCAL_TIME (__cpp_lib_chrono >= 201907L) +# else +# define FMT_USE_LOCAL_TIME 0 +# endif +#endif + // Check if std::chrono::utc_timestamp is available. #ifndef FMT_USE_UTC_TIME # ifdef __cpp_lib_chrono @@ -453,6 +462,7 @@ auto write(OutputIt out, const std::tm& time, const std::locale& loc, FMT_MODULE_EXPORT_BEGIN +#if FMT_USE_LOCAL_TIME /** Converts given time since epoch as ``std::time_t`` value into calendar time, expressed in local time. Unlike ``std::localtime``, this function is @@ -494,10 +504,12 @@ inline std::tm localtime(std::time_t time) { return lt.tm_; } +template inline std::tm localtime( - std::chrono::time_point time_point) { - return localtime(std::chrono::system_clock::to_time_t(time_point)); + std::chrono::local_time time_point) { + return localtime(std::chrono::system_clock::to_time_t(std::chrono::current_zone()->to_sys(time_point))); } +#endif /** Converts given time since epoch as ``std::time_t`` value into calendar time, @@ -2103,6 +2115,37 @@ struct formatter, auto format(std::chrono::time_point val, FormatContext& ctx) const -> decltype(ctx.out()) { using period = typename Duration::period; + if (period::num != 1 || period::den != 1 || + std::is_floating_point::value) { + const auto epoch = val.time_since_epoch(); + const auto subsecs = std::chrono::duration_cast( + epoch - std::chrono::duration_cast(epoch)); + + return formatter::do_format( + gmtime(std::chrono::time_point_cast(val)), + ctx, &subsecs); + } + + return formatter::format( + gmtime(std::chrono::time_point_cast(val)), + ctx); + } +}; + +#if FMT_USE_LOCAL_TIME +template +struct formatter, + Char> : formatter { + FMT_CONSTEXPR formatter() { + basic_string_view default_specs = + detail::string_literal{}; + this->do_parse(default_specs.begin(), default_specs.end()); + } + + template + auto format(std::chrono::local_time val, + FormatContext& ctx) const -> decltype(ctx.out()) { + using period = typename Duration::period; if (period::num != 1 || period::den != 1 || std::is_floating_point::value) { const auto epoch = val.time_since_epoch(); @@ -2119,6 +2162,7 @@ struct formatter, ctx); } }; +#endif #if FMT_USE_UTC_TIME template diff --git a/test/chrono-test.cc b/test/chrono-test.cc index 2321c64c17a1..44eccac88d0e 100644 --- a/test/chrono-test.cc +++ b/test/chrono-test.cc @@ -235,33 +235,28 @@ auto equal(const std::tm& lhs, const std::tm& rhs) -> bool { lhs.tm_isdst == rhs.tm_isdst; } -TEST(chrono_test, localtime) { - auto t = std::time(nullptr); - auto tm = *std::localtime(&t); - EXPECT_TRUE(equal(tm, fmt::localtime(t))); -} - TEST(chrono_test, gmtime) { auto t = std::time(nullptr); auto tm = *std::gmtime(&t); EXPECT_TRUE(equal(tm, fmt::gmtime(t))); } -template auto strftime_full(TimePoint tp) -> std::string { +template +auto strftime_full_utc(TimePoint tp) -> std::string { auto t = std::chrono::system_clock::to_time_t(tp); - auto tm = *std::localtime(&t); + auto tm = *std::gmtime(&t); return system_strftime("%Y-%m-%d %H:%M:%S", &tm); } -TEST(chrono_test, time_point) { +TEST(chrono_test, system_clock_time_point) { auto t1 = std::chrono::time_point_cast( std::chrono::system_clock::now()); - EXPECT_EQ(strftime_full(t1), fmt::format("{:%Y-%m-%d %H:%M:%S}", t1)); - EXPECT_EQ(strftime_full(t1), fmt::format("{}", t1)); + EXPECT_EQ(strftime_full_utc(t1), fmt::format("{:%Y-%m-%d %H:%M:%S}", t1)); + EXPECT_EQ(strftime_full_utc(t1), fmt::format("{}", t1)); using time_point = std::chrono::time_point; auto t2 = time_point(std::chrono::seconds(42)); - EXPECT_EQ(strftime_full(t2), fmt::format("{:%Y-%m-%d %H:%M:%S}", t2)); + EXPECT_EQ(strftime_full_utc(t2), fmt::format("{:%Y-%m-%d %H:%M:%S}", t2)); std::vector spec_list = { "%%", "%n", "%t", "%Y", "%EY", "%y", "%Oy", "%Ey", "%C", @@ -283,7 +278,7 @@ TEST(chrono_test, time_point) { for (const auto& spec : spec_list) { auto t = std::chrono::system_clock::to_time_t(t1); - auto tm = *std::localtime(&t); + auto tm = *std::gmtime(&t); auto sys_output = system_strftime(spec, &tm); @@ -295,6 +290,81 @@ TEST(chrono_test, time_point) { if (std::find(spec_list.cbegin(), spec_list.cend(), "%z") != spec_list.cend()) { auto t = std::chrono::system_clock::to_time_t(t1); + auto tm = *std::gmtime(&t); + + auto sys_output = system_strftime("%z", &tm); + sys_output.insert(sys_output.end() - 2, 1, ':'); + + EXPECT_EQ(sys_output, fmt::format("{:%Ez}", t1)); + EXPECT_EQ(sys_output, fmt::format("{:%Ez}", tm)); + + EXPECT_EQ(sys_output, fmt::format("{:%Oz}", t1)); + EXPECT_EQ(sys_output, fmt::format("{:%Oz}", tm)); + } +} + +#if FMT_USE_LOCAL_TIME + +TEST(chrono_test, localtime) { + auto t = std::time(nullptr); + auto tm = *std::localtime(&t); + EXPECT_TRUE(equal(tm, fmt::localtime(t))); +} + +template +auto strftime_full_local(std::chrono::local_time tp) -> std::string { + auto t = std::chrono::system_clock::to_time_t( + std::chrono::current_zone()->to_sys(tp)); + auto tm = *std::localtime(&t); + return system_strftime("%Y-%m-%d %H:%M:%S", &tm); +} + +TEST(chrono_test, local_system_clock_time_point) { +# ifdef _WIN32 + return; // Not supported on Windows. +# endif + auto t1 = std::chrono::time_point_cast( + std::chrono::current_zone()->to_local(std::chrono::system_clock::now())); + EXPECT_EQ(strftime_full_local(t1), fmt::format("{:%Y-%m-%d %H:%M:%S}", t1)); + EXPECT_EQ(strftime_full_local(t1), fmt::format("{}", t1)); + using time_point = std::chrono::local_time; + auto t2 = time_point(std::chrono::seconds(86400 + 42)); + EXPECT_EQ(strftime_full_local(t2), fmt::format("{:%Y-%m-%d %H:%M:%S}", t2)); + + std::vector spec_list = { + "%%", "%n", "%t", "%Y", "%EY", "%y", "%Oy", "%Ey", "%C", + "%EC", "%G", "%g", "%b", "%h", "%B", "%m", "%Om", "%U", + "%OU", "%W", "%OW", "%V", "%OV", "%j", "%d", "%Od", "%e", + "%Oe", "%a", "%A", "%w", "%Ow", "%u", "%Ou", "%H", "%OH", + "%I", "%OI", "%M", "%OM", "%S", "%OS", "%x", "%Ex", "%X", + "%EX", "%D", "%F", "%R", "%T", "%p", "%z", "%Z"}; +# ifndef _WIN32 + // Disabled on Windows because these formats are not consistent among + // platforms. + spec_list.insert(spec_list.end(), {"%c", "%Ec", "%r"}); +# elif defined(__MINGW32__) && !defined(_UCRT) + // Only C89 conversion specifiers when using MSVCRT instead of UCRT + spec_list = {"%%", "%Y", "%y", "%b", "%B", "%m", "%U", "%W", "%j", "%d", "%a", + "%A", "%w", "%H", "%I", "%M", "%S", "%x", "%X", "%p", "%Z"}; +# endif + spec_list.push_back("%Y-%m-%d %H:%M:%S"); + + for (const auto& spec : spec_list) { + auto t = std::chrono::system_clock::to_time_t( + std::chrono::current_zone()->to_sys(t1)); + auto tm = *std::localtime(&t); + + auto sys_output = system_strftime(spec, &tm); + + auto fmt_spec = fmt::format("{{:{}}}", spec); + EXPECT_EQ(sys_output, fmt::format(fmt::runtime(fmt_spec), t1)); + EXPECT_EQ(sys_output, fmt::format(fmt::runtime(fmt_spec), tm)); + } + + if (std::find(spec_list.cbegin(), spec_list.cend(), "%z") != + spec_list.cend()) { + auto t = std::chrono::system_clock::to_time_t( + std::chrono::current_zone()->to_sys(t1)); auto tm = *std::localtime(&t); auto sys_output = system_strftime("%z", &tm); @@ -308,6 +378,8 @@ TEST(chrono_test, time_point) { } } +#endif // FMT_USE_LOCAL_TIME + #ifndef FMT_STATIC_THOUSANDS_SEPARATOR TEST(chrono_test, format_default) { @@ -757,7 +829,7 @@ TEST(chrono_test, timestamps_sub_seconds) { const auto t9_sec = std::chrono::time_point_cast(t9); auto t9_sub_sec_part = fmt::format("{0:09}", (t9 - t9_sec).count()); - EXPECT_EQ(fmt::format("{}.{}", strftime_full(t9_sec), t9_sub_sec_part), + EXPECT_EQ(fmt::format("{}.{}", strftime_full_utc(t9_sec), t9_sub_sec_part), fmt::format("{:%Y-%m-%d %H:%M:%S}", t9)); const std::chrono::time_point