From 99d36f56a2f59e6affdc52304badc18ce2a768b8 Mon Sep 17 00:00:00 2001 From: Stefan <29021710+Saalvage@users.noreply.github.com> Date: Thu, 15 Jun 2023 10:33:18 +0200 Subject: [PATCH] Fix `hh_mm_ss` formatting for values >= 1 day (#3727) Co-authored-by: Stephan T. Lavavej --- stl/inc/__msvc_chrono.hpp | 2 +- stl/inc/chrono | 32 +++++------- tests/std/test.lst | 1 + .../env.lst | 4 ++ .../test.cpp | 50 +++++++++++++++++++ .../test.cpp | 18 ++++--- 6 files changed, 78 insertions(+), 29 deletions(-) create mode 100644 tests/std/tests/GH_003676_format_large_hh_mm_ss_values/env.lst create mode 100644 tests/std/tests/GH_003676_format_large_hh_mm_ss_values/test.cpp diff --git a/stl/inc/__msvc_chrono.hpp b/stl/inc/__msvc_chrono.hpp index 8c1661798e..45b9f75f78 100644 --- 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 index aa4db5e1c7..620706b959 100644 --- 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) { // 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 - _NODISCARD constexpr auto _Hh_mm_ss_part_underflow_to_zero(const _Duration& _Val) { - return hh_mm_ss>{_Remove_duration_part(_Val)}; - } - template void _Write_fractional_seconds( basic_ostream<_CharT, _Traits>& _Os, const seconds& _Seconds, const _Precision& _Subseconds) { @@ -4993,7 +4988,7 @@ namespace chrono { template 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 @@ -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 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 index d0707da3e0..9831d460bc 100644 --- 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 new file mode 100644 index 0000000000..d6d824b587 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e25e45a402 --- /dev/null +++ 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 +#include +#include +#include +#include +#include + +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 +void assert_duration_format_equal(const duration& dur, const string& str) { + ostringstream oss; + oss << hh_mm_ss{dur}; + assert_equal(str, oss.str()); + assert_equal(str, format("{:%T}", dur)); +} + +template +void assert_duration_format_equal_positive(const duration& 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{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 index b15d9cc5cb..29067af605 100644 --- 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{1.55f}) == STR("13:11:59 1")); - assert(format(STR("{:%T %j}"), duration{-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{1.55f}) == STR("37:11:59 1")); + assert(format(STR("{:%T %j}"), duration{-1.55f}) == STR("-37:11:59 1")); } template @@ -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")); }