Fix hh_mm_ss formatting for values >= 1 day (#3727)

Co-authored-by: Stephan T. Lavavej <[email protected]>
microsoft · Jun 15, 2023 · 99d36f5 · 99d36f5
1 parent 47679bb
commit 99d36f5
Show file tree

Hide file tree

Showing 6 changed files with 78 additions and 29 deletions.
diff --git a/stl/inc/__msvc_chrono.hpp b/stl/inc/__msvc_chrono.hpp
@@ -501,7 +501,7 @@ namespace chrono {
         is_arithmetic_v<_Rep>) /* strengthened */ {
         // create a duration whose count() is the absolute value of _Dur.count()
         if (_Dur < duration<_Rep, _Period>::zero()) {
-            return duration<_Rep, _Period>::zero() - _Dur;
+            return -_Dur;
         } else {
             return _Dur;
         }

diff --git a/stl/inc/chrono b/stl/inc/chrono
@@ -1601,12 +1601,12 @@ namespace chrono {
 
         constexpr hh_mm_ss() noexcept : hh_mm_ss{_Duration::zero()} {}
         constexpr explicit hh_mm_ss(_Duration _Dur)
-            : _Is_neg{_Dur < _Duration::zero()}, //
+            : _Is_neg{_Dur < _Duration::zero()},
               _Hours{_Duration_cast_underflow_to_zero<_CHRONO hours>(_CHRONO abs(_Dur))},
               _Mins{_Duration_cast_underflow_to_zero<_CHRONO minutes>(
                   _Remove_duration_part<_CHRONO hours>(_CHRONO abs(_Dur)))},
               _Secs{_Duration_cast_underflow_to_zero<_CHRONO seconds>(
-                  _Remove_duration_part<_CHRONO minutes>(_CHRONO abs(_Dur)))} {
+                  _Remove_duration_part<_CHRONO minutes>(_Remove_duration_part<_CHRONO hours>(_CHRONO abs(_Dur))))} {
             if constexpr (treat_as_floating_point_v<typename precision::rep>) {
                 // no need to deal with underflow here, because floating durations allow it
                 _Sub_secs = _CHRONO abs(_Dur) - hours() - minutes() - seconds();
@@ -4942,11 +4942,6 @@ namespace chrono {
         _STL_INTERNAL_CHECK(false);
     }
 
-    template <class _Duration>
-    _NODISCARD constexpr auto _Hh_mm_ss_part_underflow_to_zero(const _Duration& _Val) {
-        return hh_mm_ss<common_type_t<days, _Duration>>{_Remove_duration_part<days>(_Val)};
-    }
-
     template <unsigned int _Fractional_width, class _CharT, class _Traits, class _Precision>
     void _Write_fractional_seconds(
         basic_ostream<_CharT, _Traits>& _Os, const seconds& _Seconds, const _Precision& _Subseconds) {
@@ -4993,7 +4988,7 @@ namespace chrono {
 
     template <class _CharT, class _Traits, class _Rep, class _Period>
     void _Write_seconds(basic_ostream<_CharT, _Traits>& _Os, const duration<_Rep, _Period>& _Val) {
-        _Write_seconds(_Os, _Hh_mm_ss_part_underflow_to_zero(_Val));
+        _Write_seconds(_Os, hh_mm_ss{_Val});
     }
 
     template <class _Ty>
@@ -5008,7 +5003,7 @@ namespace chrono {
         int _Seconds        = 0;
 
         if constexpr (_Is_specialization_v<_Ty, duration>) {
-            return _Fill_tm(_Hh_mm_ss_part_underflow_to_zero(_Val));
+            return _Fill_tm(hh_mm_ss{_Val});
         } else if constexpr (_Is_specialization_v<_Ty, _Local_time_format_t>) {
             return _Fill_tm(_Val._Time);
         } else if constexpr (is_same_v<_Ty, day>) {
@@ -5616,10 +5611,13 @@ namespace chrono {
                 _Os << _CharT{'/'};
                 _Custom_write(_Os, {._Type = 'y'}, _Time, _Val);
                 return true;
+            case 'H':
+                // Print hour even if >= 24.
+                _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), _Time.tm_hour);
+                return true;
             case 'T':
-                // Alias for %H:%M:%S but we need to rewrite %S to display fractions of a second.
-                _Validate_specifiers({._Type = 'H'}, _Val);
-                _Os << _STD put_time(&_Time, _STATICALLY_WIDEN(_CharT, "%H:%M:"));
+                // Alias for %H:%M:%S but we allow hours > 23 and rewrite %S to display fractions of a second.
+                _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}:{:02}:"), _Time.tm_hour, _Time.tm_min);
                 [[fallthrough]];
             case 'S':
                 if (_Has_modifier) {
@@ -5674,14 +5672,8 @@ namespace chrono {
         template <class _Ty>
         static void _Validate_specifiers(const _Chrono_spec<_CharT>& _Spec, const _Ty& _Val) {
             if constexpr (_Is_specialization_v<_Ty, duration> || is_same_v<_Ty, sys_info>
-                          || _Is_specialization_v<_Ty, time_point> || _Is_specialization_v<_Ty, _Local_time_format_t>) {
-                return;
-            }
-
-            if constexpr (_Is_specialization_v<_Ty, hh_mm_ss>) {
-                if (_Spec._Type == 'H' && _Val.hours() >= hours{24}) {
-                    _Throw_format_error("Cannot localize hh_mm_ss with an absolute value of 24 hours or more.");
-                }
+                          || _Is_specialization_v<_Ty, time_point> || _Is_specialization_v<_Ty, _Local_time_format_t>
+                          || _Is_specialization_v<_Ty, hh_mm_ss>) {
                 return;
             }
 

diff --git a/tests/std/test.lst b/tests/std/test.lst
@@ -223,6 +223,7 @@ tests\GH_003105_piecewise_densities
 tests\GH_003119_error_category_ctor
 tests\GH_003246_cmath_narrowing
 tests\GH_003617_vectorized_meow_element
+tests\GH_003676_format_large_hh_mm_ss_values
 tests\LWG2381_num_get_floating_point
 tests\LWG2597_complex_branch_cut
 tests\LWG3018_shared_ptr_function

diff --git a/tests/std/tests/GH_003676_format_large_hh_mm_ss_values/env.lst b/tests/std/tests/GH_003676_format_large_hh_mm_ss_values/env.lst
@@ -0,0 +1,4 @@
+# Copyright (c) Microsoft Corporation.
+# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+RUNALL_INCLUDE ..\concepts_20_matrix.lst
diff --git a/tests/std/tests/GH_003676_format_large_hh_mm_ss_values/test.cpp b/tests/std/tests/GH_003676_format_large_hh_mm_ss_values/test.cpp
@@ -0,0 +1,50 @@
+// Copyright (c) Microsoft Corporation.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#include <cassert>
+#include <chrono>
+#include <format>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+using namespace std;
+using namespace chrono;
+
+void assert_equal(const auto& expected, const auto& value) {
+    if (expected == value) {
+        return;
+    }
+
+    cout << "Expected: " << expected << endl;
+    cout << "Got:      " << value << endl;
+    assert(false);
+}
+
+template <typename R, typename P>
+void assert_duration_format_equal(const duration<R, P>& dur, const string& str) {
+    ostringstream oss;
+    oss << hh_mm_ss{dur};
+    assert_equal(str, oss.str());
+    assert_equal(str, format("{:%T}", dur));
+}
+
+template <typename R, typename P>
+void assert_duration_format_equal_positive(const duration<R, P>& dur, const string& str) {
+    assert_duration_format_equal(dur, str);
+    assert_duration_format_equal(-dur, '-' + str);
+}
+
+int main() {
+    assert_duration_format_equal(0ns, "00:00:00.000000000");
+    assert_duration_format_equal(-0h, "00:00:00");
+    assert_duration_format_equal(years{0}, "00:00:00");
+
+    assert_duration_format_equal_positive(3h, "03:00:00");
+    assert_duration_format_equal_positive(10h + 20min + 30s, "10:20:30");
+    assert_duration_format_equal_positive(days{3}, "72:00:00");
+    assert_duration_format_equal_positive(years{1}, "8765:49:12");
+    assert_duration_format_equal_positive(duration<float, days::period>{1.55f}, "37:11:59");
+    assert_duration_format_equal_positive(2ms, "00:00:00.002");
+    assert_duration_format_equal_positive(60min, "01:00:00");
+}
diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp
@@ -276,12 +276,12 @@ void test_duration_formatter() {
     assert(format(STR("{:%T}"), 4083007ms) == STR("01:08:03.007"));
     assert(format(STR("{:%T}"), -4083007ms) == STR("-01:08:03.007"));
 
-    assert(format(STR("{:%T %j %q %Q}"), days{4} + 30min) == STR("00:30:00 4 min 5790"));
-    assert(format(STR("{:%T %j %q %Q}"), -days{4} - 30min) == STR("-00:30:00 4 min 5790"));
-    assert(format(STR("{:%T %j}"), days{4} + 23h + 30min) == STR("23:30:00 4"));
-    assert(format(STR("{:%T %j}"), -days{4} - 23h - 30min) == STR("-23:30:00 4"));
-    assert(format(STR("{:%T %j}"), duration<float, days::period>{1.55f}) == STR("13:11:59 1"));
-    assert(format(STR("{:%T %j}"), duration<float, days::period>{-1.55f}) == STR("-13:11:59 1"));
+    assert(format(STR("{:%T %j %q %Q}"), days{4} + 30min) == STR("96:30:00 4 min 5790"));
+    assert(format(STR("{:%T %j %q %Q}"), -days{4} - 30min) == STR("-96:30:00 4 min 5790"));
+    assert(format(STR("{:%T %j}"), days{4} + 23h + 30min) == STR("119:30:00 4"));
+    assert(format(STR("{:%T %j}"), -days{4} - 23h - 30min) == STR("-119:30:00 4"));
+    assert(format(STR("{:%T %j}"), duration<float, days::period>{1.55f}) == STR("37:11:59 1"));
+    assert(format(STR("{:%T %j}"), duration<float, days::period>{-1.55f}) == STR("-37:11:59 1"));
 }
 
 template <typename CharT>
@@ -775,8 +775,10 @@ void test_hh_mm_ss_formatter() {
     assert(format(STR("{:%H %I %M %S %r %R %T %p}"), hh_mm_ss{-13h - 14min - 15351ms})
            == STR("-13 01 14 15.351 01:14:15 PM 13:14 13:14:15.351 PM"));
 
-    throw_helper(STR("{}"), hh_mm_ss{24h});
-    throw_helper(STR("{}"), hh_mm_ss{-24h});
+    assert(format(STR("{}"), hh_mm_ss{24h}) == STR("24:00:00"));
+    assert(format(STR("{}"), hh_mm_ss{-24h}) == STR("-24:00:00"));
+    assert(format(STR("{:%H}"), hh_mm_ss{24h}) == STR("24"));
+    assert(format(STR("{:%H}"), hh_mm_ss{-24h}) == STR("-24"));
     assert(format(STR("{:%M %S}"), hh_mm_ss{27h + 12min + 30s}) == STR("12 30"));
 }