From 8227a2f38260b38f120ddfd257cda4f5a1fd737f Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 16 Jan 2024 12:57:50 -0500 Subject: [PATCH 1/7] fix chrono duration rounding with leading zeroes --- CMakeLists.txt | 2 +- include/fmt/chrono.h | 35 +++++++++++++++++++++++++++++------ test/chrono-test.cc | 12 ++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ec8154edcb4b..20a26b908033 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -140,7 +140,7 @@ set_verbose(FMT_INC_DIR ${CMAKE_INSTALL_INCLUDEDIR} CACHE STRING "Installation directory for include files, a relative path that " "will be joined with ${CMAKE_INSTALL_PREFIX} or an absolute path.") -option(FMT_PEDANTIC "Enable extra warnings and expensive tests." OFF) +option(FMT_PEDANTIC "Enable extra warnings and expensive tests." ON) option(FMT_WERROR "Halt the compilation with an error on compiler warnings." OFF) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 09cce20feaa0..992ab4c7638b 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1154,15 +1154,38 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) { } else { *out++ = '.'; leading_zeroes = (std::min)(leading_zeroes, precision); - out = std::fill_n(out, leading_zeroes, '0'); int remaining = precision - leading_zeroes; - if (remaining != 0 && remaining < num_digits) { - n /= to_unsigned(detail::pow10(to_unsigned(num_digits - remaining))); - out = format_decimal(out, n, remaining).end; + if (remaining < num_digits) { + int num_truncated_digits = num_digits - remaining; + n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits)-1)); //002999 + const int old_num_digits = detail::count_digits(n); + int roundingDigit = n % 10; + n /= 10; + if (roundingDigit > 5 || (roundingDigit == 5 && n % 10 % 2 != 0)) { + n += 1; + } + if (old_num_digits == detail::count_digits(n)) { + if (leading_zeroes) { + out = std::fill_n(out, leading_zeroes-1, '0'); + *out++ = '1'; + out = std::fill_n(out, remaining, '0'); + } + else { + n -= 1; + out = format_decimal(out, n, remaining).end; + } + } + else { + out = std::fill_n(out, leading_zeroes, '0'); + out = format_decimal(out, n, remaining).end; + } return; } - out = format_decimal(out, n, num_digits).end; - remaining -= num_digits; + out = std::fill_n(out, leading_zeroes, '0'); + if (n) { + out = format_decimal(out, n, num_digits).end; + remaining -= num_digits; + } out = std::fill_n(out, remaining, '0'); } } diff --git a/test/chrono-test.cc b/test/chrono-test.cc index b2d03f979578..ad45933853ea 100644 --- a/test/chrono-test.cc +++ b/test/chrono-test.cc @@ -791,6 +791,18 @@ TEST(chrono_test, cpp20_duration_subsecond_support) { "01.234000"); EXPECT_EQ(fmt::format("{:.6%S}", std::chrono::milliseconds{-1234}), "-01.234000"); + EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{12345}), + "12.34"); + EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{12375}), + "12.38"); + EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{-12375}), + "-12.38"); + EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{12054}), + "12.05"); + EXPECT_EQ(fmt::format("{:.1%S}", std::chrono::milliseconds{12054}), + "12.1"); + EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{99999}), + "39.99"); EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::seconds{1234}), "34.000"); EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::hours{1234}), "00.000"); EXPECT_EQ(fmt::format("{:.5%S}", dms(1.234)), "00.00123"); From 7e96e39d149c5cc53a6290a0d76e30d7f3d64957 Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 16 Jan 2024 13:04:41 -0500 Subject: [PATCH 2/7] fix type --- include/fmt/chrono.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 992ab4c7638b..8c5990171083 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1159,7 +1159,7 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) { int num_truncated_digits = num_digits - remaining; n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits)-1)); //002999 const int old_num_digits = detail::count_digits(n); - int roundingDigit = n % 10; + auto roundingDigit = n % 10; n /= 10; if (roundingDigit > 5 || (roundingDigit == 5 && n % 10 % 2 != 0)) { n += 1; From e81ef54b30528a4416014bbf840dc2b70ce6cf09 Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 16 Jan 2024 21:15:17 -0500 Subject: [PATCH 3/7] add tests and single digit case --- include/fmt/chrono.h | 38 ++++++++++++++++++++++++++------------ test/chrono-test.cc | 4 ++++ 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 8c5990171083..424ba08c3420 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1161,23 +1161,37 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) { const int old_num_digits = detail::count_digits(n); auto roundingDigit = n % 10; n /= 10; - if (roundingDigit > 5 || (roundingDigit == 5 && n % 10 % 2 != 0)) { - n += 1; - } - if (old_num_digits == detail::count_digits(n)) { - if (leading_zeroes) { - out = std::fill_n(out, leading_zeroes-1, '0'); - *out++ = '1'; - out = std::fill_n(out, remaining, '0'); + if (n) { + if (roundingDigit > 5 || (roundingDigit == 5 && n % 10 % 2 != 0)) { + n += 1; + } + if (old_num_digits == detail::count_digits(n)) { + if (leading_zeroes) { + out = std::fill_n(out, leading_zeroes-1, '0'); + *out++ = '1'; + out = std::fill_n(out, remaining, '0'); + } + else { + n -= 1; + out = format_decimal(out, n, remaining).end; + } } else { - n -= 1; + out = std::fill_n(out, leading_zeroes, '0'); out = format_decimal(out, n, remaining).end; } } - else { - out = std::fill_n(out, leading_zeroes, '0'); - out = format_decimal(out, n, remaining).end; + else { + if (roundingDigit >= 5) { + if (leading_zeroes) { + out = std::fill_n(out, leading_zeroes-1, '0'); + *out++ = '1'; + out = std::fill_n(out, remaining, '0'); + } + } + else { + out = std::fill_n(out, leading_zeroes, '0'); + } } return; } diff --git a/test/chrono-test.cc b/test/chrono-test.cc index ad45933853ea..189ba49779ee 100644 --- a/test/chrono-test.cc +++ b/test/chrono-test.cc @@ -803,6 +803,10 @@ TEST(chrono_test, cpp20_duration_subsecond_support) { "12.1"); EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{99999}), "39.99"); + EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{1000}), + "01.00"); + EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::milliseconds{1}), + "00.001"); EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::seconds{1234}), "34.000"); EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::hours{1234}), "00.000"); EXPECT_EQ(fmt::format("{:.5%S}", dms(1.234)), "00.00123"); From 2ce9f9a28f78386724a909be2f291dc6516032ae Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 16 Jan 2024 21:51:55 -0500 Subject: [PATCH 4/7] cmake change --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 20a26b908033..ec8154edcb4b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -140,7 +140,7 @@ set_verbose(FMT_INC_DIR ${CMAKE_INSTALL_INCLUDEDIR} CACHE STRING "Installation directory for include files, a relative path that " "will be joined with ${CMAKE_INSTALL_PREFIX} or an absolute path.") -option(FMT_PEDANTIC "Enable extra warnings and expensive tests." ON) +option(FMT_PEDANTIC "Enable extra warnings and expensive tests." OFF) option(FMT_WERROR "Halt the compilation with an error on compiler warnings." OFF) From 2ccc7aa4553aee086680d87fd6c4fd0bdd8e96f3 Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 16 Jan 2024 22:04:51 -0500 Subject: [PATCH 5/7] remove comment --- include/fmt/chrono.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 424ba08c3420..bc21e4da572a 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1157,7 +1157,7 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) { int remaining = precision - leading_zeroes; if (remaining < num_digits) { int num_truncated_digits = num_digits - remaining; - n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits)-1)); //002999 + n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits)-1)); const int old_num_digits = detail::count_digits(n); auto roundingDigit = n % 10; n /= 10; From 4d38a66f96139e5bed661961f8538aebb79df786 Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 16 Jan 2024 22:33:42 -0500 Subject: [PATCH 6/7] formatting --- include/fmt/chrono.h | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index bc21e4da572a..41e2f5e8316c 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1155,41 +1155,37 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) { *out++ = '.'; leading_zeroes = (std::min)(leading_zeroes, precision); int remaining = precision - leading_zeroes; - if (remaining < num_digits) { + if (remaining < num_digits) { int num_truncated_digits = num_digits - remaining; - n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits)-1)); + n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits) - 1)); const int old_num_digits = detail::count_digits(n); auto roundingDigit = n % 10; n /= 10; if (n) { if (roundingDigit > 5 || (roundingDigit == 5 && n % 10 % 2 != 0)) { n += 1; - } + } if (old_num_digits == detail::count_digits(n)) { if (leading_zeroes) { - out = std::fill_n(out, leading_zeroes-1, '0'); + out = std::fill_n(out, leading_zeroes - 1, '0'); *out++ = '1'; out = std::fill_n(out, remaining, '0'); - } - else { + } else { n -= 1; out = format_decimal(out, n, remaining).end; } - } - else { + } else { out = std::fill_n(out, leading_zeroes, '0'); out = format_decimal(out, n, remaining).end; } - } - else { - if (roundingDigit >= 5) { + } else { + if (roundingDigit >= 5) { if (leading_zeroes) { - out = std::fill_n(out, leading_zeroes-1, '0'); + out = std::fill_n(out, leading_zeroes - 1, '0'); *out++ = '1'; out = std::fill_n(out, remaining, '0'); } - } - else { + } else { out = std::fill_n(out, leading_zeroes, '0'); } } From f8b9e7950476282a14225403fd6efe4dfb6d7102 Mon Sep 17 00:00:00 2001 From: Jin S Date: Tue, 6 Feb 2024 15:15:39 -0500 Subject: [PATCH 7/7] truncation instead of rounding --- include/fmt/chrono.h | 36 ++++-------------------------------- test/chrono-test.cc | 10 ++++------ 2 files changed, 8 insertions(+), 38 deletions(-) diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h index 41e2f5e8316c..a6ea4b295a01 100644 --- a/include/fmt/chrono.h +++ b/include/fmt/chrono.h @@ -1151,47 +1151,19 @@ void write_fractional_seconds(OutputIt& out, Duration d, int precision = -1) { out = std::fill_n(out, leading_zeroes, '0'); out = format_decimal(out, n, num_digits).end; } - } else { + } else if (precision > 0) { *out++ = '.'; leading_zeroes = (std::min)(leading_zeroes, precision); int remaining = precision - leading_zeroes; + out = std::fill_n(out, leading_zeroes, '0'); if (remaining < num_digits) { int num_truncated_digits = num_digits - remaining; - n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits) - 1)); - const int old_num_digits = detail::count_digits(n); - auto roundingDigit = n % 10; - n /= 10; + n /= to_unsigned(detail::pow10(to_unsigned(num_truncated_digits))); if (n) { - if (roundingDigit > 5 || (roundingDigit == 5 && n % 10 % 2 != 0)) { - n += 1; - } - if (old_num_digits == detail::count_digits(n)) { - if (leading_zeroes) { - out = std::fill_n(out, leading_zeroes - 1, '0'); - *out++ = '1'; - out = std::fill_n(out, remaining, '0'); - } else { - n -= 1; - out = format_decimal(out, n, remaining).end; - } - } else { - out = std::fill_n(out, leading_zeroes, '0'); - out = format_decimal(out, n, remaining).end; - } - } else { - if (roundingDigit >= 5) { - if (leading_zeroes) { - out = std::fill_n(out, leading_zeroes - 1, '0'); - *out++ = '1'; - out = std::fill_n(out, remaining, '0'); - } - } else { - out = std::fill_n(out, leading_zeroes, '0'); - } + out = format_decimal(out, n, remaining).end; } return; } - out = std::fill_n(out, leading_zeroes, '0'); if (n) { out = format_decimal(out, n, num_digits).end; remaining -= num_digits; diff --git a/test/chrono-test.cc b/test/chrono-test.cc index 189ba49779ee..2afe864e7b25 100644 --- a/test/chrono-test.cc +++ b/test/chrono-test.cc @@ -794,13 +794,11 @@ TEST(chrono_test, cpp20_duration_subsecond_support) { EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{12345}), "12.34"); EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{12375}), - "12.38"); + "12.37"); EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{-12375}), - "-12.38"); - EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{12054}), - "12.05"); - EXPECT_EQ(fmt::format("{:.1%S}", std::chrono::milliseconds{12054}), - "12.1"); + "-12.37"); + EXPECT_EQ(fmt::format("{:.0%S}", std::chrono::milliseconds{12054}), + "12"); EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{99999}), "39.99"); EXPECT_EQ(fmt::format("{:.2%S}", std::chrono::milliseconds{1000}),