From a9e41a6df2abd19cd088bfbb9c294fceb6c92cdc Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Sat, 7 Nov 2026 15:42:47 -0800 Subject: [PATCH] Rename rep to abs_rep in duration_formatter and simplify the tests The old name differed from Rep only in case, which made the narrowing fixed in #5003 easy to miss. --- include/fmt/chrono.h | 39 +++++++++++++++++++++------------------ test/chrono-test.cc | 14 ++++---------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 62d7a7c6..eb6b1d0d 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1665,30 +1665,33 @@ template struct duration_formatter { using iterator = basic_appender; iterator out; - // rep is unsigned to avoid overflow. - using rep = + // Integral reps are made unsigned so that negating the minimum value can't + // overflow, and at least as wide as int to leave headroom for the conversion + // to seconds. + using abs_rep = conditional_t::value && sizeof(Rep) < sizeof(int), unsigned, typename make_unsigned_or_unchanged::type>; - rep val; + abs_rep val; int precision; locale_ref locale; bool localized = false; - using seconds = std::chrono::duration; + using seconds = std::chrono::duration; seconds s; - using milliseconds = std::chrono::duration; - bool negative; + using milliseconds = std::chrono::duration; + bool negative = false; using tm_writer_type = tm_writer; duration_formatter(iterator o, std::chrono::duration d, locale_ref loc) - : out(o), val(static_cast(d.count())), locale(loc), negative(false) { + : out(o), val(static_cast(d.count())), locale(loc) { if (d.count() < 0) { val = 0 - val; negative = true; } - s = detail::duration_cast(std::chrono::duration(val)); + s = detail::duration_cast( + std::chrono::duration(val)); } // If val is NaN or infinite, writes it to out and returns true. @@ -1703,16 +1706,16 @@ struct duration_formatter { return true; } - auto days() const -> rep { return s.count() / 86400; } - auto hour() const -> rep { return mod((s.count() / 3600), 24); } + auto days() const -> abs_rep { return s.count() / 86400; } + auto hour() const -> abs_rep { return mod(s.count() / 3600, 24); } - auto hour12() const -> rep { - auto hour = mod((s.count() / 3600), 12); + auto hour12() const -> abs_rep { + auto hour = mod(s.count() / 3600, 12); return hour < 1 ? 12 : hour; } - auto minute() const -> rep { return mod((s.count() / 60), 60); } - auto second() const -> rep { return mod(s.count(), 60); } + auto minute() const -> abs_rep { return mod(s.count() / 60, 60); } + auto second() const -> abs_rep { return mod(s.count(), 60); } auto time() const -> std::tm { auto time = std::tm(); @@ -1728,7 +1731,7 @@ struct duration_formatter { negative = false; } - void write(rep value, int width, pad_type pad = pad_type::zero) { + void write(abs_rep value, int width, pad_type pad = pad_type::zero) { write_sign(); if (isnan(value)) return write_nan(); uint32_or_64_or_128_t n = @@ -1816,9 +1819,9 @@ struct duration_formatter { if (write_if_nonfinite()) return; if (ns == numeric_system::standard) { - if (std::is_floating_point::value) { + if (std::is_floating_point::value) { auto buf = memory_buffer(); - write_floating_seconds(buf, std::chrono::duration(val), + write_floating_seconds(buf, std::chrono::duration(val), precision); write_sign(); if (buf.size() < 2 || buf[1] == '.') @@ -1827,7 +1830,7 @@ struct duration_formatter { } else { write(second(), 2, pad); write_fractional_seconds( - out, std::chrono::duration(val), precision); + out, std::chrono::duration(val), precision); } return; } diff --git a/test/chrono-test.cc b/test/chrono-test.cc index 2b04a3a3..d7c35c1b 100644 --- a/test/chrono-test.cc +++ b/test/chrono-test.cc @@ -538,6 +538,10 @@ TEST(chrono_test, format_specs) { EXPECT_EQ(fmt::format("{:%j}", days(12)), "12"); EXPECT_EQ(fmt::format("{:%j}", days(12345)), "12345"); EXPECT_EQ(fmt::format("{:%j}", std::chrono::hours(12345 * 24 + 12)), "12345"); + using signed_days = std::chrono::duration>; + EXPECT_EQ(fmt::format("{:%j}", signed_days(-128)), "-128"); + using two_days = std::chrono::duration>; + EXPECT_EQ(fmt::format("{:%j}", two_days(200)), "400"); EXPECT_EQ(fmt::format("{:%H:%M:%S}", std::chrono::seconds(12345)), "03:25:45"); EXPECT_EQ(fmt::format("{:%R}", std::chrono::seconds(12345)), "03:25"); @@ -546,16 +550,6 @@ TEST(chrono_test, format_specs) { EXPECT_EQ(fmt::format("{:%q}", std::chrono::seconds(12345)), "s"); } -TEST(chrono_test, format_days_with_signed_char_rep) { - using signed_days = std::chrono::duration>; - EXPECT_EQ(fmt::format("{:%j}", signed_days(-128)), "-128"); -} - -TEST(chrono_test, format_days_with_unsigned_char_rep) { - using two_days = std::chrono::duration>; - EXPECT_EQ(fmt::format("{:%j}", two_days(200)), "400"); -} - TEST(chrono_test, invalid_specs) { auto sec = std::chrono::seconds(0); EXPECT_THROW_MSG((void)fmt::format(runtime("{:%a}"), sec), fmt::format_error,