Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hh_mm_ss formatting for values >= 1 day #3727

Merged
merged 7 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stl/inc/__msvc_chrono.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
34 changes: 12 additions & 22 deletions stl/inc/chrono
Original file line number Diff line number Diff line change
Expand Up @@ -1603,10 +1603,8 @@ namespace chrono {
constexpr explicit hh_mm_ss(_Duration _Dur)
: _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)))} {
_Mins{_Duration_cast_underflow_to_zero<_CHRONO minutes>(_CHRONO abs(_Dur)) - _Hours},
_Secs{_Duration_cast_underflow_to_zero<_CHRONO seconds>(_CHRONO abs(_Dur)) - _Hours - _Mins} {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
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();
Expand Down Expand Up @@ -4942,11 +4940,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) {
Expand Down Expand Up @@ -4993,7 +4986,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>
Expand All @@ -5008,7 +5001,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>) {
Expand Down Expand Up @@ -5616,10 +5609,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) {
Expand Down Expand Up @@ -5674,14 +5670,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;
}

Expand Down
1 change: 1 addition & 0 deletions tests/std/test.lst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright (c) Microsoft Corporation.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

RUNALL_INCLUDE ..\concepts_20_matrix.lst
50 changes: 50 additions & 0 deletions tests/std/tests/GH_003676_format_large_hh_mm_ss_values/test.cpp
Original file line number Diff line number Diff line change
@@ -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>

using namespace std;
using namespace chrono;
using namespace chrono_literals;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved

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>& duration, const string& str) {
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
stringstream ss;
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
ss << hh_mm_ss{duration};
assert_equal(str, ss.str());
assert_equal(str, format("{:%T}", duration));
}

template <typename R, typename P>
void assert_duration_format_equal_positive(const duration<R, P>& duration, const string& str) {
assert_duration_format_equal(duration, str);
assert_duration_format_equal(-duration, '-' + 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:12:00");
assert_duration_format_equal_positive(2ms, "00:00:00.002");
assert_duration_format_equal_positive(60min, "01:00:00");
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:12:00 1"));
assert(format(STR("{:%T %j}"), duration<float, days::period>{-1.55f}) == STR("-37:12:00 1"));
}

template <typename CharT>
Expand Down Expand Up @@ -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"));
}

Expand Down