Fix handling of chrono durations with minimal signed rep

fmtlib · Jun 1, 2019 · 78daa50 · 78daa50
1 parent 87e4ea2
commit 78daa50
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 9 deletions.
diff --git a/include/fmt/chrono.h b/include/fmt/chrono.h
@@ -401,6 +401,17 @@ inline T mod(T x, int y) {
   return std::fmod(x, y);
 }
 
+// If T is an integral type, maps T to its unsigned counterpart, otherwise
+// leaves it unchanged (unlike std::make_unsigned).
+template <typename T, bool INTEGRAL = std::is_integral<T>::value>
+struct make_unsigned_or_unchanged {
+  using type = T;
+};
+
+template <typename T> struct make_unsigned_or_unchanged<T, true> {
+  using type = typename std::make_unsigned<T>::type;
+};
+
 template <typename Rep, typename Period,
           typename std::enable_if<std::is_integral<Rep>::value, int>::type = 0>
 inline std::chrono::duration<Rep, std::milli> get_milliseconds(
@@ -438,21 +449,27 @@ struct chrono_formatter {
   FormatContext& context;
   OutputIt out;
   int precision;
-  typedef typename std::conditional<std::is_integral<Rep>::value &&
-                                        sizeof(Rep) < sizeof(int),
-                                    int, Rep>::type rep;
+  // rep is unsigned to avoid overflow.
+  using rep = typename std::conditional<
+      std::is_integral<Rep>::value && sizeof(Rep) < sizeof(int), unsigned,
+      typename make_unsigned_or_unchanged<Rep>::type>::type;
   rep val;
   typedef std::chrono::duration<rep> seconds;
   seconds s;
   typedef std::chrono::duration<rep, std::milli> milliseconds;
+  bool negative;
 
   typedef typename FormatContext::char_type char_type;
 
   explicit chrono_formatter(FormatContext& ctx, OutputIt o,
                             std::chrono::duration<Rep, Period> d)
-      : context(ctx), out(o), val(d.count()) {
-    if (d.count() < 0) d = -d;
-    s = std::chrono::duration_cast<seconds>(d);
+      : context(ctx), out(o), val(d.count()), negative(false) {
+    if (d.count() < 0) {
+      val = -val;
+      negative = true;
+    }
+    s = std::chrono::duration_cast<seconds>(
+        std::chrono::duration<rep, Period>(val));
   }
 
   Rep hour() const { return mod((s.count() / 3600), 24); }
@@ -474,9 +491,9 @@ struct chrono_formatter {
   }
 
   void write_sign() {
-    if (val < 0) {
+    if (negative) {
       *out++ = '-';
-      val = -val;
+      negative = false;
     }
   }
 

diff --git a/test/chrono-test.cc b/test/chrono-test.cc
@@ -306,7 +306,7 @@ TEST(ChronoTest, InvalidColons) {
                fmt::format_error);
 }
 
-TEST(ChronoTest, NegativeDuration) {
+TEST(ChronoTest, NegativeDurations) {
   EXPECT_EQ("-12345", fmt::format("{:%Q}", std::chrono::seconds(-12345)));
   EXPECT_EQ("-03:25:45",
             fmt::format("{:%H:%M:%S}", std::chrono::seconds(-12345)));
@@ -316,6 +316,9 @@ TEST(ChronoTest, NegativeDuration) {
   EXPECT_EQ("-00.127",
             fmt::format("{:%S}",
                         std::chrono::duration<signed char, std::milli>{-127}));
+  auto min = std::numeric_limits<int>::min();
+  EXPECT_EQ(fmt::format("{}", min),
+            fmt::format("{:%Q}", std::chrono::duration<int>(min)));
 }
 
 TEST(ChronoTest, SpecialDurations) {