From 5d28f2cf6bac6db6da0593e42c23afae185a6829 Mon Sep 17 00:00:00 2001
From: Shawn Zhong <github@shawnzhong.com>
Date: Sat, 17 Dec 2022 03:03:32 -0600
Subject: [PATCH] Fix duration with precision

---
 include/fmt/chrono.h | 11 +++--------
 test/chrono-test.cc  | 29 +++++++++++++++--------------
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
index 53f861fa..362f1324 100644
--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -1551,8 +1551,6 @@ class tm_writer {
 };
 
 struct chrono_format_checker : null_chrono_spec_handler<chrono_format_checker> {
-  bool has_precision_integral = false;
-
   FMT_NORETURN void unsupported() { FMT_THROW(format_error("no date")); }
 
   template <typename Char>
@@ -1565,11 +1563,7 @@ struct chrono_format_checker : null_chrono_spec_handler<chrono_format_checker> {
   FMT_CONSTEXPR void on_24_hour_time() {}
   FMT_CONSTEXPR void on_iso_time() {}
   FMT_CONSTEXPR void on_am_pm() {}
-  FMT_CONSTEXPR void on_duration_value() const {
-    if (has_precision_integral) {
-      FMT_THROW(format_error("precision not allowed for this argument type"));
-    }
-  }
+  FMT_CONSTEXPR void on_duration_value() {}
   FMT_CONSTEXPR void on_duration_unit() {}
 };
 
@@ -2041,7 +2035,8 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
     if (begin == end) return {begin, begin};
     auto checker = detail::chrono_format_checker();
     if (*begin == '.') {
-      checker.has_precision_integral = !std::is_floating_point<Rep>::value;
+      if (!std::is_floating_point<Rep>::value)
+        handler.on_error("precision not allowed for this argument type");
       begin = detail::parse_precision(begin, end, handler);
     }
     if (begin != end && *begin == 'L') {
diff --git a/test/chrono-test.cc b/test/chrono-test.cc
index 2321c64c..50f04c93 100644
--- a/test/chrono-test.cc
+++ b/test/chrono-test.cc
@@ -481,6 +481,9 @@ TEST(chrono_test, format_default_fp) {
 }
 
 TEST(chrono_test, format_precision) {
+  EXPECT_THROW_MSG(
+      (void)fmt::format(runtime("{:.2}"), std::chrono::seconds(42)),
+      fmt::format_error, "precision not allowed for this argument type");
   EXPECT_THROW_MSG(
       (void)fmt::format(runtime("{:.2%Q}"), std::chrono::seconds(42)),
       fmt::format_error, "precision not allowed for this argument type");
@@ -627,24 +630,22 @@ TEST(chrono_test, cpp20_duration_subsecond_support) {
   EXPECT_EQ(fmt::format("{:%S}", std::chrono::nanoseconds{-13420148734}),
             "-13.420148734");
   EXPECT_EQ(fmt::format("{:%S}", std::chrono::milliseconds{1234}), "01.234");
-  // Check subsecond presision modifier.
-  EXPECT_EQ(fmt::format("{:.6%S}", std::chrono::nanoseconds{1234}),
-            "00.000001");
-  EXPECT_EQ(fmt::format("{:.18%S}", std::chrono::nanoseconds{1234}),
-            "00.000001234000000000");
-  EXPECT_EQ(fmt::format("{:.{}%S}", std::chrono::nanoseconds{1234}, 6),
-            "00.000001");
-  EXPECT_EQ(fmt::format("{:.6%S}", std::chrono::milliseconds{1234}),
-            "01.234000");
-  EXPECT_EQ(fmt::format("{:.6%S}", std::chrono::milliseconds{-1234}),
-            "-01.234000");
-  EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::seconds{1234}), "34.000");
-  EXPECT_EQ(fmt::format("{:.3%S}", std::chrono::hours{1234}), "00.000");
+  // Check subsecond precision modifier.
+  using dns = std::chrono::duration<double, std::nano>;
+  using ds = std::chrono::duration<double>;
+  using dh = std::chrono::duration<double, std::ratio<3600>>;
+  EXPECT_EQ(fmt::format("{:.6%S}", dns{1234}), "00.000001");
+  EXPECT_EQ(fmt::format("{:.18%S}", dns{1234}), "00.000001234000000000");
+  EXPECT_EQ(fmt::format("{:.{}%S}", dns{1234}, 6), "00.000001");
+  EXPECT_EQ(fmt::format("{:.6%S}", dms{1234}), "01.234000");
+  EXPECT_EQ(fmt::format("{:.6%S}", dms{-1234}), "-01.234000");
+  EXPECT_EQ(fmt::format("{:.3%S}", ds{1234}), "34.000");
+  EXPECT_EQ(fmt::format("{:.3%S}", dh{1234}), "00.000");
   EXPECT_EQ(fmt::format("{:.5%S}", dms(1.234)), "00.00123");
   EXPECT_EQ(fmt::format("{:.8%S}", dms(1.234)), "00.00123400");
   {
     // Check that {:%H:%M:%S} is equivalent to {:%T}.
-    auto dur = std::chrono::milliseconds{3601234};
+    auto dur = dms{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);