From c379573725a0ee27ad08eb84da8b12f7b401572c Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sun, 28 Nov 2021 22:00:07 +0100
Subject: [PATCH 01/14] Add support for subsecond printing for
 std::chrono::duration according to the c++20 standard

---
 include/fmt/chrono.h | 81 +++++++++++++++++++++++++++++++++-----------
 test/chrono-test.cc  | 44 ++++++++++++++++++++++--
 2 files changed, 103 insertions(+), 22 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index d54c8ae79ec0..60e5a4599be6 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1320,19 +1320,19 @@ inline bool isfinite(T) {
 
 // Converts value to int and checks that it's in the range [0, upper).
 template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value)>
-inline int to_nonnegative_int(T value, int upper) {
+inline std::intmax_t to_nonnegative_int(T value, std::intmax_t upper) {
   FMT_ASSERT(value >= 0 && to_unsigned(value) <= to_unsigned(upper),
              "invalid value");
   (void)upper;
-  return static_cast<int>(value);
+  return static_cast<std::intmax_t>(value);
 }
 template <typename T, FMT_ENABLE_IF(!std::is_integral<T>::value)>
-inline int to_nonnegative_int(T value, int upper) {
+inline std::intmax_t to_nonnegative_int(T value, std::intmax_t upper) {
   FMT_ASSERT(
       std::isnan(value) || (value >= 0 && value <= static_cast<T>(upper)),
       "invalid value");
   (void)upper;
-  return static_cast<int>(value);
+  return static_cast<std::intmax_t>(value);
 }
 
 template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value)>
@@ -1389,16 +1389,55 @@ inline std::chrono::duration<Rep, std::milli> get_milliseconds(
 #endif
 }
 
-template <typename Rep, typename Period,
-          FMT_ENABLE_IF(std::is_floating_point<Rep>::value)>
-inline std::chrono::duration<Rep, std::milli> get_milliseconds(
-    std::chrono::duration<Rep, Period> d) {
-  using common_type = typename std::common_type<Rep, std::intmax_t>::type;
-  auto ms = mod(d.count() * static_cast<common_type>(Period::num) /
-                    static_cast<common_type>(Period::den) * 1000,
-                1000);
-  return std::chrono::duration<Rep, std::milli>(static_cast<Rep>(ms));
-}
+template <class Duration> class subsecond_helper {
+  /// Returns the amount of digits according to the c++ 20 spec
+  /// In the range [0, 18], if more than 18 fractional digits are required,
+  /// then we return 6 for microseconds precision
+  static constexpr int num_digits(std::intmax_t num,
+                                  std::intmax_t den,
+                                  std::uint32_t n = 0) {
+    return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
+  }
+
+  static constexpr std::intmax_t pow10(std::uint32_t n) {
+    return n == 0 ? 1 : 10 * pow10(n - 1);
+  }
+
+  template <class Rep, class Period,
+            FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)>
+  static constexpr std::chrono::duration<Rep, Period> abs(
+      std::chrono::duration<Rep, Period> d) {
+    return d >= d.zero() ? d : -d;
+  }
+
+  template <class Rep, class Period,
+            FMT_ENABLE_IF(!std::numeric_limits<Rep>::is_signed)>
+  static constexpr std::chrono::duration<Rep, Period> abs(
+      std::chrono::duration<Rep, Period> d) {
+    return d;
+  }
+
+ public:
+  static constexpr auto fractional_width =
+      num_digits(Duration::period::num, Duration::period::den);
+
+  using precision = std::chrono::duration<
+      typename std::common_type<typename Duration::rep,
+                                std::chrono::seconds::rep>::type,
+      std::ratio<1, pow10(fractional_width)>>;
+
+  template <class Rep, class Period>
+  static constexpr typename precision::rep get_subseconds(
+      std::chrono::duration<Rep, Period> d) {
+    return std::chrono::treat_as_floating_point<typename precision::rep>::value
+               ? (abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d))
+                     .count()
+               : std::chrono::duration_cast<precision>(
+                     abs(d) -
+                     std::chrono::duration_cast<std::chrono::seconds>(d))
+                     .count();
+  }
+};
 
 template <typename Char, typename Rep, typename OutputIt,
           FMT_ENABLE_IF(std::is_integral<Rep>::value)>
@@ -1553,8 +1592,8 @@ struct chrono_formatter {
   void write(Rep value, int width) {
     write_sign();
     if (isnan(value)) return write_nan();
-    uint32_or_64_or_128_t<int> n =
-        to_unsigned(to_nonnegative_int(value, max_value<int>()));
+    uint32_or_64_or_128_t<std::intmax_t> n =
+        to_unsigned(to_nonnegative_int(value, max_value<std::intmax_t>()));
     int num_digits = detail::count_digits(n);
     if (width > num_digits) out = std::fill_n(out, width - num_digits, '0');
     out = format_decimal<char_type>(out, n, num_digits).end;
@@ -1645,10 +1684,12 @@ struct chrono_formatter {
 #else
       auto tmpval = std::chrono::duration<Rep, Period>(val);
 #endif
-      auto ms = get_milliseconds(tmpval);
-      if (ms != std::chrono::milliseconds(0)) {
+      using subsec_helper = detail::subsecond_helper<duration_Rep>;
+      // Could use c++ 17 if constexpr
+      if (std::ratio_less<typename subsec_helper::precision::period,
+                          std::chrono::seconds::period>::value) {
         *out++ = '.';
-        write(ms.count(), 3);
+        write(subsec_helper::get_subseconds(tmpval), subsec_helper::fractional_width);
       }
       return;
     }
@@ -1678,7 +1719,7 @@ struct chrono_formatter {
     on_24_hour_time();
     *out++ = ':';
     if (handle_nan_inf()) return;
-    write(second(), 2);
+    on_second(numeric_system::standard);
   }
 
   void on_am_pm() {
diff --git a/test/chrono-test.cc b/test/chrono-test.cc
index a2bc20c2eca5..856736833240 100644
--- a/test/chrono-test.cc
+++ b/test/chrono-test.cc
@@ -544,8 +544,8 @@ TEST(chrono_test, negative_durations) {
 
 TEST(chrono_test, special_durations) {
   EXPECT_EQ(
-      "40.",
-      fmt::format("{:%S}", std::chrono::duration<double>(1e20)).substr(0, 3));
+      "40",
+      fmt::format("{:%S}", std::chrono::duration<double>(1e20)).substr(0, 2));
   auto nan = std::numeric_limits<double>::quiet_NaN();
   EXPECT_EQ(
       "nan nan nan nan nan:nan nan",
@@ -585,4 +585,44 @@ TEST(chrono_test, weekday) {
   }
 }
 
+TEST(chrono_test, cpp20_duration_subsecond_support) {
+  using attoseconds = std::chrono::duration<std::intmax_t, std::atto>;
+  // Check that 18 digits of subsecond precision are supported
+  EXPECT_EQ(fmt::format("{:%S}", attoseconds{673231113420148734}),
+            "00.673231113420148734");
+  EXPECT_EQ(fmt::format("{:%S}", attoseconds{-673231113420148734}),
+            "-00.673231113420148734");
+  EXPECT_EQ(fmt::format("{:%S}", std::chrono::nanoseconds{13420148734}),
+            "13.420148734");
+  EXPECT_EQ(fmt::format("{:%S}", std::chrono::nanoseconds{-13420148734}),
+            "-13.420148734");
+  EXPECT_EQ(fmt::format("{:%S}", std::chrono::milliseconds{1234}), "01.234");
+  {
+    // Check that {:%H:%M:%S} is equivalent to {:%T}
+    auto dur = std::chrono::milliseconds{3601234};
+    auto formatted_dur = fmt::format("{:%T}", dur);
+    EXPECT_EQ(formatted_dur, "01:00:01.234");
+    EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur);
+  }
+  using nanoseconds_dbl = std::chrono::duration<double, std::nano>;
+  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{-123456789.123456789}),
+            "-00.123456789");
+  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{9123456789.123456789}),
+            "09.123456789");
+  // Only seconds part is printed
+  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123456789}), "39.123456789");
+  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123000000}), "39.123000000");
+  {
+    // Now the hour is printed, and we also test if negative doubles work
+    auto dur = nanoseconds_dbl{-99123456789};
+    auto formatted_dur = fmt::format("{:%T}", dur);
+    EXPECT_EQ(formatted_dur, "-00:01:39.123456789");
+    EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur);
+  }
+  // Check that durations with precision greater than std::chrono::seconds have
+  // fixed precision and empty zeros
+  EXPECT_EQ(fmt::format("{:%S}", std::chrono::microseconds{7000000}),
+            "07.000000");
+}
+
 #endif  // FMT_STATIC_THOUSANDS_SEPARATOR

From 57d35cc6e60f7defaf7df82dad38fba967cb4f75 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sun, 28 Nov 2021 22:25:38 +0100
Subject: [PATCH 02/14] Remove assert test that overflows intmax_t

---
 test/chrono-test.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/chrono-test.cc b/test/chrono-test.cc
index 856736833240..23e45da9bf10 100644
--- a/test/chrono-test.cc
+++ b/test/chrono-test.cc
@@ -550,8 +550,6 @@ TEST(chrono_test, special_durations) {
   EXPECT_EQ(
       "nan nan nan nan nan:nan nan",
       fmt::format("{:%I %H %M %S %R %r}", std::chrono::duration<double>(nan)));
-  (void)fmt::format("{:%S}",
-                    std::chrono::duration<float, std::atto>(1.79400457e+31f));
   EXPECT_EQ(fmt::format("{}", std::chrono::duration<float, std::exa>(1)),
             "1Es");
   EXPECT_EQ(fmt::format("{}", std::chrono::duration<float, std::atto>(1)),
@@ -588,6 +586,8 @@ TEST(chrono_test, weekday) {
 TEST(chrono_test, cpp20_duration_subsecond_support) {
   using attoseconds = std::chrono::duration<std::intmax_t, std::atto>;
   // Check that 18 digits of subsecond precision are supported
+  EXPECT_EQ(fmt::format("{:%S}", attoseconds{999999999999999999}),
+            "00.999999999999999999");
   EXPECT_EQ(fmt::format("{:%S}", attoseconds{673231113420148734}),
             "00.673231113420148734");
   EXPECT_EQ(fmt::format("{:%S}", attoseconds{-673231113420148734}),

From 863d2ffc577212008822b1fa5d46ed0b987f1725 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Mon, 29 Nov 2021 01:51:59 +0100
Subject: [PATCH 03/14] * Hopefully fix int64_t to int32_t conversion errors. *
 Allow proper Duration::rep type to propagate via template argument deduction

---
 include/fmt/chrono.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 60e5a4599be6..6915c5ca4ddd 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1319,20 +1319,20 @@ inline bool isfinite(T) {
 }
 
 // Converts value to int and checks that it's in the range [0, upper).
-template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value)>
-inline std::intmax_t to_nonnegative_int(T value, std::intmax_t upper) {
+template <typename T, typename Int, FMT_ENABLE_IF(std::is_integral<T>::value)>
+inline Int to_nonnegative_int(T value, Int upper) {
   FMT_ASSERT(value >= 0 && to_unsigned(value) <= to_unsigned(upper),
              "invalid value");
   (void)upper;
-  return static_cast<std::intmax_t>(value);
+  return static_cast<Int>(value);
 }
-template <typename T, FMT_ENABLE_IF(!std::is_integral<T>::value)>
-inline std::intmax_t to_nonnegative_int(T value, std::intmax_t upper) {
+template <typename T, typename Int, FMT_ENABLE_IF(!std::is_integral<T>::value)>
+inline Int to_nonnegative_int(T value, Int upper) {
   FMT_ASSERT(
       std::isnan(value) || (value >= 0 && value <= static_cast<T>(upper)),
       "invalid value");
   (void)upper;
-  return static_cast<std::intmax_t>(value);
+  return static_cast<Int>(value);
 }
 
 template <typename T, FMT_ENABLE_IF(std::is_integral<T>::value)>

From beacfd3289dfe1973a42ec7aed1379269710d085 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Mon, 29 Nov 2021 01:52:38 +0100
Subject: [PATCH 04/14] * Hopefully fix int64_t to int32_t conversion errors. *
 Allow proper Duration::rep type to propagate via template argument deduction

---
 include/fmt/chrono.h | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 6915c5ca4ddd..6453d5263181 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1589,7 +1589,8 @@ struct chrono_formatter {
     }
   }
 
-  void write(Rep value, int width) {
+  template <typename RepType>
+  void write(RepType value, int width) {
     write_sign();
     if (isnan(value)) return write_nan();
     uint32_or_64_or_128_t<std::intmax_t> n =
@@ -1676,20 +1677,13 @@ struct chrono_formatter {
 
     if (ns == numeric_system::standard) {
       write(second(), 2);
-#if FMT_SAFE_DURATION_CAST
-      // convert rep->Rep
       using duration_rep = std::chrono::duration<rep, Period>;
-      using duration_Rep = std::chrono::duration<Rep, Period>;
-      auto tmpval = fmt_safe_duration_cast<duration_Rep>(duration_rep{val});
-#else
-      auto tmpval = std::chrono::duration<Rep, Period>(val);
-#endif
-      using subsec_helper = detail::subsecond_helper<duration_Rep>;
+      using subsec_helper = detail::subsecond_helper<duration_rep>;
       // Could use c++ 17 if constexpr
       if (std::ratio_less<typename subsec_helper::precision::period,
                           std::chrono::seconds::period>::value) {
         *out++ = '.';
-        write(subsec_helper::get_subseconds(tmpval), subsec_helper::fractional_width);
+        write(subsec_helper::get_subseconds(duration_rep{val}), subsec_helper::fractional_width);
       }
       return;
     }

From b8e3e3388c151e79995b0d751be903c93c1c9799 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sat, 4 Dec 2021 17:46:36 +0100
Subject: [PATCH 05/14] Fix sign conversion (-Wsign-conversion) warning treated
 as error in num_digits()

---
 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 6453d5263181..21b940d90ecf 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1395,7 +1395,7 @@ template <class Duration> class subsecond_helper {
   /// then we return 6 for microseconds precision
   static constexpr int num_digits(std::intmax_t num,
                                   std::intmax_t den,
-                                  std::uint32_t n = 0) {
+                                  int n = 0) {
     return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
   }
 

From 5619e8757c24fb2d6759efb4c7d7b056bc67138f Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sat, 4 Dec 2021 17:50:13 +0100
Subject: [PATCH 06/14] Format chrono.h with clang-format

---
 include/fmt/chrono.h | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 21b940d90ecf..3b1c0fc56f39 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1393,8 +1393,7 @@ template <class Duration> class subsecond_helper {
   /// Returns the amount of digits according to the c++ 20 spec
   /// In the range [0, 18], if more than 18 fractional digits are required,
   /// then we return 6 for microseconds precision
-  static constexpr int num_digits(std::intmax_t num,
-                                  std::intmax_t den,
+  static constexpr int num_digits(std::intmax_t num, std::intmax_t den,
                                   int n = 0) {
     return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
   }
@@ -1589,8 +1588,7 @@ struct chrono_formatter {
     }
   }
 
-  template <typename RepType>
-  void write(RepType value, int width) {
+  template <typename RepType> void write(RepType value, int width) {
     write_sign();
     if (isnan(value)) return write_nan();
     uint32_or_64_or_128_t<std::intmax_t> n =
@@ -1683,7 +1681,8 @@ struct chrono_formatter {
       if (std::ratio_less<typename subsec_helper::precision::period,
                           std::chrono::seconds::period>::value) {
         *out++ = '.';
-        write(subsec_helper::get_subseconds(duration_rep{val}), subsec_helper::fractional_width);
+        write(subsec_helper::get_subseconds(duration_rep{val}),
+              subsec_helper::fractional_width);
       }
       return;
     }

From 75444670c1d630064b2c01f791934aea48b67190 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sat, 4 Dec 2021 19:41:39 +0100
Subject: [PATCH 07/14] Remove extra forward slash in doxygen style comment

Co-authored-by: Victor Zverovich <victor.zverovich@gmail.com>
---
 include/fmt/chrono.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 3b1c0fc56f39..b5ee0c9a0788 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1390,9 +1390,9 @@ inline std::chrono::duration<Rep, std::milli> get_milliseconds(
 }
 
 template <class Duration> class subsecond_helper {
-  /// Returns the amount of digits according to the c++ 20 spec
-  /// In the range [0, 18], if more than 18 fractional digits are required,
-  /// then we return 6 for microseconds precision
+  // Returns the amount of digits according to the c++ 20 spec
+  // In the range [0, 18], if more than 18 fractional digits are required,
+  // then we return 6 for microseconds precision
   static constexpr int num_digits(std::intmax_t num, std::intmax_t den,
                                   int n = 0) {
     return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));

From 3d860b7cf508b63f72eb2a9d6cb515561c38af09 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sun, 5 Dec 2021 16:03:07 +0100
Subject: [PATCH 08/14] Apply all suggestions from GitHub, except for replacing
 the utility subsecond_helper class with a function

---
 include/fmt/chrono.h | 17 +++++++++++------
 test/chrono-test.cc  | 25 ++++++++++++-------------
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index b5ee0c9a0788..8ae57ae80f48 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1393,12 +1393,11 @@ template <class Duration> class subsecond_helper {
   // Returns the amount of digits according to the c++ 20 spec
   // In the range [0, 18], if more than 18 fractional digits are required,
   // then we return 6 for microseconds precision
-  static constexpr int num_digits(std::intmax_t num, std::intmax_t den,
-                                  int n = 0) {
+  static constexpr int num_digits(long long num, long long den, int n = 0) {
     return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
   }
 
-  static constexpr std::intmax_t pow10(std::uint32_t n) {
+  static constexpr long long pow10(std::uint32_t n) {
     return n == 0 ? 1 : 10 * pow10(n - 1);
   }
 
@@ -1406,7 +1405,13 @@ template <class Duration> class subsecond_helper {
             FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)>
   static constexpr std::chrono::duration<Rep, Period> abs(
       std::chrono::duration<Rep, Period> d) {
-    return d >= d.zero() ? d : -d;
+    // We need to compare the duration using the count() method directly
+    // due to a compiler bug in clang-11 regarding the spaceship operator,
+    // when -Wzero-as-null-pointer-constant is enabled.
+    // In clang-12 the bug has been fixed. See
+    // https://bugs.llvm.org/show_bug.cgi?id=46235 and the reproducible example:
+    // https://www.godbolt.org/z/Knbb5joYx
+    return d.count() >= d.zero().count() ? d : -d;
   }
 
   template <class Rep, class Period,
@@ -1591,8 +1596,8 @@ struct chrono_formatter {
   template <typename RepType> void write(RepType value, int width) {
     write_sign();
     if (isnan(value)) return write_nan();
-    uint32_or_64_or_128_t<std::intmax_t> n =
-        to_unsigned(to_nonnegative_int(value, max_value<std::intmax_t>()));
+    uint32_or_64_or_128_t<long long> n =
+        to_unsigned(to_nonnegative_int(value, max_value<long long>()));
     int num_digits = detail::count_digits(n);
     if (width > num_digits) out = std::fill_n(out, width - num_digits, '0');
     out = format_decimal<char_type>(out, n, num_digits).end;
diff --git a/test/chrono-test.cc b/test/chrono-test.cc
index 23e45da9bf10..f98daac39156 100644
--- a/test/chrono-test.cc
+++ b/test/chrono-test.cc
@@ -543,9 +543,10 @@ TEST(chrono_test, negative_durations) {
 }
 
 TEST(chrono_test, special_durations) {
-  EXPECT_EQ(
-      "40",
-      fmt::format("{:%S}", std::chrono::duration<double>(1e20)).substr(0, 2));
+  auto value = fmt::format("{:%S}", std::chrono::duration<double>(1e20));
+  // No decimal point is printed so size() is 2.
+  EXPECT_EQ(value.size(), 2);
+  EXPECT_EQ("40", value.substr(0, 2));
   auto nan = std::numeric_limits<double>::quiet_NaN();
   EXPECT_EQ(
       "nan nan nan nan nan:nan nan",
@@ -584,8 +585,8 @@ TEST(chrono_test, weekday) {
 }
 
 TEST(chrono_test, cpp20_duration_subsecond_support) {
-  using attoseconds = std::chrono::duration<std::intmax_t, std::atto>;
-  // Check that 18 digits of subsecond precision are supported
+  using attoseconds = std::chrono::duration<long long, std::atto>;
+  // Check that 18 digits of subsecond precision are supported.
   EXPECT_EQ(fmt::format("{:%S}", attoseconds{999999999999999999}),
             "00.999999999999999999");
   EXPECT_EQ(fmt::format("{:%S}", attoseconds{673231113420148734}),
@@ -598,29 +599,27 @@ TEST(chrono_test, cpp20_duration_subsecond_support) {
             "-13.420148734");
   EXPECT_EQ(fmt::format("{:%S}", std::chrono::milliseconds{1234}), "01.234");
   {
-    // Check that {:%H:%M:%S} is equivalent to {:%T}
+    // Check that {:%H:%M:%S} is equivalent to {:%T}.
     auto dur = std::chrono::milliseconds{3601234};
     auto formatted_dur = fmt::format("{:%T}", dur);
     EXPECT_EQ(formatted_dur, "01:00:01.234");
     EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur);
   }
   using nanoseconds_dbl = std::chrono::duration<double, std::nano>;
-  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{-123456789.123456789}),
-            "-00.123456789");
-  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{9123456789.123456789}),
-            "09.123456789");
-  // Only seconds part is printed
+  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{-123456789}), "-00.123456789");
+  EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{9123456789}), "09.123456789");
+  // Verify that only the seconds part is extracted and printed.
   EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123456789}), "39.123456789");
   EXPECT_EQ(fmt::format("{:%S}", nanoseconds_dbl{99123000000}), "39.123000000");
   {
-    // Now the hour is printed, and we also test if negative doubles work
+    // Now the hour is printed, and we also test if negative doubles work.
     auto dur = nanoseconds_dbl{-99123456789};
     auto formatted_dur = fmt::format("{:%T}", dur);
     EXPECT_EQ(formatted_dur, "-00:01:39.123456789");
     EXPECT_EQ(fmt::format("{:%H:%M:%S}", dur), formatted_dur);
   }
   // Check that durations with precision greater than std::chrono::seconds have
-  // fixed precision and empty zeros
+  // fixed precision, and print zeros even if there is no fractional part.
   EXPECT_EQ(fmt::format("{:%S}", std::chrono::microseconds{7000000}),
             "07.000000");
 }

From 0fcdb017abfe984f3a66819e08639bb84b5081ea Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sun, 5 Dec 2021 16:44:41 +0100
Subject: [PATCH 09/14] * Move logic of handling subseconds from utility class
 to function with name write_fractional_seconds() * Revert write(Rep value,
 int width) function to previous state

---
 include/fmt/chrono.h | 133 +++++++++++++++++++++++--------------------
 1 file changed, 72 insertions(+), 61 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 8ae57ae80f48..15cfa4122bd6 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1389,59 +1389,47 @@ inline std::chrono::duration<Rep, std::milli> get_milliseconds(
 #endif
 }
 
-template <class Duration> class subsecond_helper {
-  // Returns the amount of digits according to the c++ 20 spec
-  // In the range [0, 18], if more than 18 fractional digits are required,
-  // then we return 6 for microseconds precision
-  static constexpr int num_digits(long long num, long long den, int n = 0) {
-    return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
-  }
+// Returns the amount of digits according to the c++ 20 spec
+// In the range [0, 18], if more than 18 fractional digits are required,
+// then we return 6 for microseconds precision.
+static constexpr int num_digits(long long num, long long den, int n = 0) {
+  return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
+}
 
-  static constexpr long long pow10(std::uint32_t n) {
-    return n == 0 ? 1 : 10 * pow10(n - 1);
-  }
+static constexpr long long pow10(std::uint32_t n) {
+  return n == 0 ? 1 : 10 * pow10(n - 1);
+}
 
-  template <class Rep, class Period,
-            FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)>
-  static constexpr std::chrono::duration<Rep, Period> abs(
-      std::chrono::duration<Rep, Period> d) {
-    // We need to compare the duration using the count() method directly
-    // due to a compiler bug in clang-11 regarding the spaceship operator,
-    // when -Wzero-as-null-pointer-constant is enabled.
-    // In clang-12 the bug has been fixed. See
-    // https://bugs.llvm.org/show_bug.cgi?id=46235 and the reproducible example:
-    // https://www.godbolt.org/z/Knbb5joYx
-    return d.count() >= d.zero().count() ? d : -d;
-  }
+template <class Rep, class Period,
+          FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)>
+static constexpr std::chrono::duration<Rep, Period> abs(
+    std::chrono::duration<Rep, Period> d) {
+  // We need to compare the duration using the count() method directly
+  // due to a compiler bug in clang-11 regarding the spaceship operator,
+  // when -Wzero-as-null-pointer-constant is enabled.
+  // In clang-12 the bug has been fixed. See
+  // https://bugs.llvm.org/show_bug.cgi?id=46235 and the reproducible example:
+  // https://www.godbolt.org/z/Knbb5joYx
+  return d.count() >= d.zero().count() ? d : -d;
+}
 
-  template <class Rep, class Period,
-            FMT_ENABLE_IF(!std::numeric_limits<Rep>::is_signed)>
-  static constexpr std::chrono::duration<Rep, Period> abs(
-      std::chrono::duration<Rep, Period> d) {
-    return d;
-  }
+template <class Rep, class Period,
+          FMT_ENABLE_IF(!std::numeric_limits<Rep>::is_signed)>
+static constexpr std::chrono::duration<Rep, Period> abs(
+    std::chrono::duration<Rep, Period> d) {
+  return d;
+}
 
- public:
-  static constexpr auto fractional_width =
-      num_digits(Duration::period::num, Duration::period::den);
-
-  using precision = std::chrono::duration<
-      typename std::common_type<typename Duration::rep,
-                                std::chrono::seconds::rep>::type,
-      std::ratio<1, pow10(fractional_width)>>;
-
-  template <class Rep, class Period>
-  static constexpr typename precision::rep get_subseconds(
-      std::chrono::duration<Rep, Period> d) {
-    return std::chrono::treat_as_floating_point<typename precision::rep>::value
-               ? (abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d))
-                     .count()
-               : std::chrono::duration_cast<precision>(
-                     abs(d) -
-                     std::chrono::duration_cast<std::chrono::seconds>(d))
-                     .count();
-  }
-};
+template <class Rep, class Period, class ToDuration>
+static constexpr typename ToDuration::rep get_subseconds(
+    std::chrono::duration<Rep, Period> d) {
+  return std::chrono::treat_as_floating_point<typename ToDuration::rep>::value
+             ? (abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d))
+                   .count()
+             : std::chrono::duration_cast<ToDuration>(
+                   abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d))
+                   .count();
+}
 
 template <typename Char, typename Rep, typename OutputIt,
           FMT_ENABLE_IF(std::is_integral<Rep>::value)>
@@ -1593,16 +1581,47 @@ struct chrono_formatter {
     }
   }
 
-  template <typename RepType> void write(RepType value, int width) {
+  void write(Rep value, int width) {
     write_sign();
     if (isnan(value)) return write_nan();
-    uint32_or_64_or_128_t<long long> n =
-        to_unsigned(to_nonnegative_int(value, max_value<long long>()));
+    uint32_or_64_or_128_t<int> n =
+        to_unsigned(to_nonnegative_int(value, max_value<int>()));
     int num_digits = detail::count_digits(n);
     if (width > num_digits) out = std::fill_n(out, width - num_digits, '0');
     out = format_decimal<char_type>(out, n, num_digits).end;
   }
 
+  template <class Duration> void write_fractional_seconds(Duration d) {
+    static constexpr auto fractional_width =
+        detail::num_digits(Duration::period::num, Duration::period::den);
+
+    using precision = std::chrono::duration<
+        typename std::common_type<typename Duration::rep,
+                                  std::chrono::seconds::rep>::type,
+        std::ratio<1, detail::pow10(fractional_width)>>;
+    // We could use c++ 17 if constexpr here.
+    if (std::ratio_less<typename precision::period,
+                        std::chrono::seconds::period>::value) {
+      *out++ = '.';
+      const auto subseconds =
+          std::chrono::treat_as_floating_point<typename precision::rep>::value
+              ? (detail::abs(d) -
+                 std::chrono::duration_cast<std::chrono::seconds>(d))
+                    .count()
+              : std::chrono::duration_cast<precision>(
+                    detail::abs(d) -
+                    std::chrono::duration_cast<std::chrono::seconds>(d))
+                    .count();
+      uint32_or_64_or_128_t<long long> n =
+          to_unsigned(to_nonnegative_int(subseconds, max_value<long long>()));
+      int num_digits = detail::count_digits(n);
+      if (fractional_width > num_digits) {
+        out = std::fill_n(out, fractional_width - num_digits, '0');
+      }
+      out = format_decimal<char_type>(out, n, num_digits).end;
+    }
+  }
+
   void write_nan() { std::copy_n("nan", 3, out); }
   void write_pinf() { std::copy_n("inf", 3, out); }
   void write_ninf() { std::copy_n("-inf", 4, out); }
@@ -1680,15 +1699,7 @@ struct chrono_formatter {
 
     if (ns == numeric_system::standard) {
       write(second(), 2);
-      using duration_rep = std::chrono::duration<rep, Period>;
-      using subsec_helper = detail::subsecond_helper<duration_rep>;
-      // Could use c++ 17 if constexpr
-      if (std::ratio_less<typename subsec_helper::precision::period,
-                          std::chrono::seconds::period>::value) {
-        *out++ = '.';
-        write(subsec_helper::get_subseconds(duration_rep{val}),
-              subsec_helper::fractional_width);
-      }
+      write_fractional_seconds(std::chrono::duration<rep, Period>{val});
       return;
     }
     auto time = tm();

From 55f62cd8ea9fb66ac741b99f9c11679d6f59b0db Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sun, 5 Dec 2021 16:54:56 +0100
Subject: [PATCH 10/14] Fix -Wshadow warning

---
 include/fmt/chrono.h | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 15cfa4122bd6..cae242a48543 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1595,20 +1595,21 @@ struct chrono_formatter {
     static constexpr auto fractional_width =
         detail::num_digits(Duration::period::num, Duration::period::den);
 
-    using precision = std::chrono::duration<
+    using subsecond_precision = std::chrono::duration<
         typename std::common_type<typename Duration::rep,
                                   std::chrono::seconds::rep>::type,
         std::ratio<1, detail::pow10(fractional_width)>>;
     // We could use c++ 17 if constexpr here.
-    if (std::ratio_less<typename precision::period,
+    if (std::ratio_less<typename subsecond_precision::period,
                         std::chrono::seconds::period>::value) {
       *out++ = '.';
       const auto subseconds =
-          std::chrono::treat_as_floating_point<typename precision::rep>::value
+          std::chrono::treat_as_floating_point<
+              typename subsecond_precision::rep>::value
               ? (detail::abs(d) -
                  std::chrono::duration_cast<std::chrono::seconds>(d))
                     .count()
-              : std::chrono::duration_cast<precision>(
+              : std::chrono::duration_cast<subsecond_precision>(
                     detail::abs(d) -
                     std::chrono::duration_cast<std::chrono::seconds>(d))
                     .count();

From a0abbed5187e1f821c2ede6f2eb2dcbb43565ca6 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Sun, 5 Dec 2021 17:02:14 +0100
Subject: [PATCH 11/14] Remove unsued get_subseconds() function, its logic has
 been moved to write_fractional_seconds()

---
 include/fmt/chrono.h | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index cae242a48543..ca138438f3b8 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1420,17 +1420,6 @@ static constexpr std::chrono::duration<Rep, Period> abs(
   return d;
 }
 
-template <class Rep, class Period, class ToDuration>
-static constexpr typename ToDuration::rep get_subseconds(
-    std::chrono::duration<Rep, Period> d) {
-  return std::chrono::treat_as_floating_point<typename ToDuration::rep>::value
-             ? (abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d))
-                   .count()
-             : std::chrono::duration_cast<ToDuration>(
-                   abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d))
-                   .count();
-}
-
 template <typename Char, typename Rep, typename OutputIt,
           FMT_ENABLE_IF(std::is_integral<Rep>::value)>
 OutputIt format_duration_value(OutputIt out, Rep val, int) {

From 38fb7a6691066c5e2f14e4700a8cf6bdd6f427b5 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Mon, 6 Dec 2021 17:58:44 +0100
Subject: [PATCH 12/14] Change comment from lowercase int to uppercase Int

---
 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 ca138438f3b8..e4db47353e8e 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1318,7 +1318,7 @@ inline bool isfinite(T) {
   return true;
 }
 
-// Converts value to int and checks that it's in the range [0, upper).
+// Converts value to Int and checks that it's in the range [0, upper).
 template <typename T, typename Int, FMT_ENABLE_IF(std::is_integral<T>::value)>
 inline Int to_nonnegative_int(T value, Int upper) {
   FMT_ASSERT(value >= 0 && to_unsigned(value) <= to_unsigned(upper),

From 89c8198282b25d8edaf8f76f7ebb51fc54a53726 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Mon, 6 Dec 2021 18:06:01 +0100
Subject: [PATCH 13/14] Simplify test check

---
 test/chrono-test.cc | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/test/chrono-test.cc b/test/chrono-test.cc
index f98daac39156..42360e5f12f1 100644
--- a/test/chrono-test.cc
+++ b/test/chrono-test.cc
@@ -544,9 +544,7 @@ TEST(chrono_test, negative_durations) {
 
 TEST(chrono_test, special_durations) {
   auto value = fmt::format("{:%S}", std::chrono::duration<double>(1e20));
-  // No decimal point is printed so size() is 2.
-  EXPECT_EQ(value.size(), 2);
-  EXPECT_EQ("40", value.substr(0, 2));
+  EXPECT_EQ(value, "40");
   auto nan = std::numeric_limits<double>::quiet_NaN();
   EXPECT_EQ(
       "nan nan nan nan nan:nan nan",

From 9344c470eed4f71cf2969fbd563e47331b8b04a5 Mon Sep 17 00:00:00 2001
From: matrackif <filip.matracki@gmail.com>
Date: Thu, 9 Dec 2021 11:14:05 +0100
Subject: [PATCH 14/14] Integrate suggested changes

* Remove static from detail functions, they are no longer member functions of a class and static is unnecessary.
* Change comment from "amount" to "number"
---
 include/fmt/chrono.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index e4db47353e8e..585eb6b60bbe 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1389,20 +1389,20 @@ inline std::chrono::duration<Rep, std::milli> get_milliseconds(
 #endif
 }
 
-// Returns the amount of digits according to the c++ 20 spec
+// Returns the number of digits according to the c++ 20 spec
 // In the range [0, 18], if more than 18 fractional digits are required,
 // then we return 6 for microseconds precision.
-static constexpr int num_digits(long long num, long long den, int n = 0) {
+constexpr int num_digits(long long num, long long den, int n = 0) {
   return num % den == 0 ? n : (n > 18 ? 6 : num_digits(num * 10, den, n + 1));
 }
 
-static constexpr long long pow10(std::uint32_t n) {
+constexpr long long pow10(std::uint32_t n) {
   return n == 0 ? 1 : 10 * pow10(n - 1);
 }
 
 template <class Rep, class Period,
           FMT_ENABLE_IF(std::numeric_limits<Rep>::is_signed)>
-static constexpr std::chrono::duration<Rep, Period> abs(
+constexpr std::chrono::duration<Rep, Period> abs(
     std::chrono::duration<Rep, Period> d) {
   // We need to compare the duration using the count() method directly
   // due to a compiler bug in clang-11 regarding the spaceship operator,
@@ -1581,7 +1581,7 @@ struct chrono_formatter {
   }
 
   template <class Duration> void write_fractional_seconds(Duration d) {
-    static constexpr auto fractional_width =
+    constexpr auto fractional_width =
         detail::num_digits(Duration::period::num, Duration::period::den);
 
     using subsecond_precision = std::chrono::duration<