Implement LWG-4274: The chrono::hh_mm_ss constructor is ill-formed for unsigned durations (#5893)

Co-authored-by: Stephan T. Lavavej <[email protected]>

Author: vmichal

stl/inc/chrono

@@ -1634,18 +1634,18 @@ namespace chrono {
         constexpr hh_mm_ss() noexcept : hh_mm_ss{_Duration::zero()} {}
         constexpr explicit hh_mm_ss(_Duration _Dur)
             : _Is_neg{_Dur < _Duration::zero()},
-              _Hours{_CHRONO _Duration_cast_underflow_to_zero<_CHRONO hours>(_CHRONO abs(_Dur))},
+              _Hours{_CHRONO _Duration_cast_underflow_to_zero<_CHRONO hours>(_Abs_if_needed(_Dur, _Is_neg))},
               _Mins{_CHRONO _Duration_cast_underflow_to_zero<_CHRONO minutes>(
-                  _CHRONO _Remove_duration_part<_CHRONO hours>(_CHRONO abs(_Dur)))},
+                  _CHRONO _Remove_duration_part<_CHRONO hours>(_Abs_if_needed(_Dur, _Is_neg)))},
               _Secs{_CHRONO _Duration_cast_underflow_to_zero<_CHRONO seconds>(
                   _CHRONO _Remove_duration_part<_CHRONO minutes>(
-                      _CHRONO _Remove_duration_part<_CHRONO hours>(_CHRONO abs(_Dur))))} {
+                      _CHRONO _Remove_duration_part<_CHRONO hours>(_Abs_if_needed(_Dur, _Is_neg))))} {
             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();
+                _Sub_secs = _Abs_if_needed(_Dur, _Is_neg) - hours() - minutes() - seconds();
             } else {
-                _Sub_secs =
-                    _CHRONO duration_cast<precision>(_CHRONO _Remove_duration_part<_CHRONO seconds>(_CHRONO abs(_Dur)));
+                _Sub_secs = _CHRONO duration_cast<precision>(
+                    _CHRONO _Remove_duration_part<_CHRONO seconds>(_Abs_if_needed(_Dur, _Is_neg)));
             }
         }
 
@@ -1674,6 +1674,14 @@ namespace chrono {
         }
 
     private:
+        _NODISCARD static constexpr _Duration _Abs_if_needed(_Duration _Dur, bool _Is_neg) {
+            if constexpr (is_unsigned_v<typename _Duration::rep>) {
+                return _Dur;
+            } else {
+                return _Is_neg ? -_Dur : _Dur;
+            }
+        }
+
         bool _Is_neg;
         _CHRONO hours _Hours;
         _CHRONO minutes _Mins;

tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp

@@ -1061,7 +1061,15 @@ void test_locale() {
     assert(stream(year_month_weekday_last{2021y / May / Tuesday[last]}) == STR("2021/Mai/Di[last]"));
 }
 
+void test_unsigned_sys_time_format_after_LWG_4274() {
+    const sys_time<duration<unsigned int>> tp{};
+    const string s = format("{:%Y-%m-%d %H:%M:%S}", tp);
+    assert(s == "1970-01-01 00:00:00");
+}
+
 void test() {
+    test_unsigned_sys_time_format_after_LWG_4274();
+
     test_parse_conversion_spec<char>();
     test_parse_conversion_spec<wchar_t>();
 

tests/std/tests/P0355R7_calendars_and_time_zones_hms/test.cpp

@@ -7,6 +7,8 @@
 #include <ratio>
 #include <type_traits>
 
+// Extended to test LWG-4274 "The chrono::hh_mm_ss constructor is ill-formed for unsigned durations"
+
 using namespace std;
 using namespace std::chrono;
 
@@ -105,6 +107,27 @@ constexpr void constructor() {
     assert(f_hms_hours{}.subseconds() == f_hms_hours{hours::zero()}.subseconds());
 }
 
+// Test LWG-4274 "The chrono::hh_mm_ss constructor is ill-formed for unsigned durations"
+constexpr void constructor_unsigned_durations() {
+    {
+        duration<unsigned long long, milli> unsigned_duration{37 + 1000 * (7 + 4 * 60 + 3 * 3600)};
+        hh_mm_ss a{unsigned_duration};
+        assert(!a.is_negative());
+        assert(a.hours() == 3h);
+        assert(a.minutes() == 4min);
+        assert(a.seconds() == 7s);
+        assert(a.subseconds() == 37ms);
+    }
+
+    {
+        // Reproducing example from GH-5569 "<chrono>: Cannot construct an hh_mm_ss object from an unsigned duration"
+        duration<uint32_t> dur{1};
+        hh_mm_ss hms{dur};
+        assert(!hms.is_negative());
+        assert(hms.seconds() == 1s);
+    }
+}
+
 constexpr void is_negative() {
     static_assert(noexcept(hms_hours{}.is_negative()));
     static_assert(noexcept(f_hms_hours{}.is_negative()));
@@ -210,6 +233,7 @@ constexpr bool test() {
     make12_24();
     fractional_width();
     constructor();
+    constructor_unsigned_durations();
     is_negative();
     hour();
     mins();