Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatting of system clocks ought to be to UTC, not to local time. #3230

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions include/fmt/chrono.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -494,10 +504,12 @@ inline std::tm localtime(std::time_t time) {
return lt.tm_;
}

template<class Duration>
inline std::tm localtime(
std::chrono::time_point<std::chrono::system_clock> time_point) {
return localtime(std::chrono::system_clock::to_time_t(time_point));
std::chrono::local_time<Duration> 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,
Expand Down Expand Up @@ -2103,6 +2115,37 @@ struct formatter<std::chrono::time_point<std::chrono::system_clock, Duration>,
auto format(std::chrono::time_point<std::chrono::system_clock, Duration> val,
FormatContext& ctx) const -> decltype(ctx.out()) {
using period = typename Duration::period;
if (period::num != 1 || period::den != 1 ||
std::is_floating_point<typename Duration::rep>::value) {
const auto epoch = val.time_since_epoch();
const auto subsecs = std::chrono::duration_cast<Duration>(
epoch - std::chrono::duration_cast<std::chrono::seconds>(epoch));

return formatter<std::tm, Char>::do_format(
gmtime(std::chrono::time_point_cast<std::chrono::seconds>(val)),
ctx, &subsecs);
}

return formatter<std::tm, Char>::format(
gmtime(std::chrono::time_point_cast<std::chrono::seconds>(val)),
ctx);
}
};

#if FMT_USE_LOCAL_TIME
template <typename Char, typename Duration>
struct formatter<std::chrono::local_time<Duration>,
Char> : formatter<std::tm, Char> {
FMT_CONSTEXPR formatter() {
basic_string_view<Char> default_specs =
detail::string_literal<Char, '%', 'F', ' ', '%', 'T'>{};
this->do_parse(default_specs.begin(), default_specs.end());
}

template <typename FormatContext>
auto format(std::chrono::local_time<Duration> val,
FormatContext& ctx) const -> decltype(ctx.out()) {
using period = typename Duration::period;
if (period::num != 1 || period::den != 1 ||
std::is_floating_point<typename Duration::rep>::value) {
const auto epoch = val.time_since_epoch();
Expand All @@ -2119,6 +2162,7 @@ struct formatter<std::chrono::time_point<std::chrono::system_clock, Duration>,
ctx);
}
};
#endif

#if FMT_USE_UTC_TIME
template <typename Char, typename Duration>
Expand Down
79 changes: 71 additions & 8 deletions test/chrono-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -247,21 +247,21 @@ TEST(chrono_test, gmtime) {
EXPECT_TRUE(equal(tm, fmt::gmtime(t)));
}

template <typename TimePoint> auto strftime_full(TimePoint tp) -> std::string {
template <typename TimePoint> 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::seconds>(
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<std::chrono::system_clock, std::chrono::seconds>;
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<std::string> spec_list = {
"%%", "%n", "%t", "%Y", "%EY", "%y", "%Oy", "%Ey", "%C",
Expand All @@ -283,7 +283,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);

Expand All @@ -295,6 +295,68 @@ 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
template <typename Duration> auto strftime_full_local(std::chrono::local_time<Duration> 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) {
auto t1 = std::chrono::time_point_cast<std::chrono::seconds>(
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<std::chrono::seconds>;
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<std::string> 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);
Expand All @@ -307,6 +369,7 @@ TEST(chrono_test, time_point) {
EXPECT_EQ(sys_output, fmt::format("{:%Oz}", tm));
}
}
#endif

#ifndef FMT_STATIC_THOUSANDS_SEPARATOR

Expand Down Expand Up @@ -757,7 +820,7 @@ TEST(chrono_test, timestamps_sub_seconds) {
const auto t9_sec = std::chrono::time_point_cast<std::chrono::seconds>(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<std::chrono::system_clock,
Expand Down