From e6708c5cc4e5e3160c79da0c8536176661ac8988 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Thu, 15 Apr 2021 09:52:20 -0700 Subject: [PATCH 1/7] : Add year and year_month_day formatting This one is a biggie. The main things changed are the way that specifiers are handled and delegated. The general idea behind formatting time is that you can take segments of a type and format them individually. For example, you can take the year out of year_month_day and do the exact same operations you can do with a normal year. The `_Is_type_valid` function recursively checks if a parent type can be formatted by its children. The other functions didn't really need much more finessing, the `tm` structure already has all the fields we need to hold all the time info (simultaniously) and the formatters work off that. The big change here is in moving some "basic" formatters into our own function and not relying on `get_time` to format them. The main reason is that `get_time` does not play with invalid ranges, at all. A day of `40` is always illegal, but we need to be able to format it, especially in the face of `operator <<`. We could have kept what we had before, but then it becomes a clear problem that we cannot use `%F` for a `year_month_day` that has an invalid day, so I am seperating all the integral formatters out into that function. Again, because of the nested nature of times, we recurse in this function. Note that function currently uses `format_to` in probably a very inneficient way. I am all ears on how to improve that. --- stl/inc/chrono | 189 ++++++++++++++++-- .../test.cpp | 37 +++- 2 files changed, 205 insertions(+), 21 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 008c5526ec..2fa92b2d3c 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5408,6 +5408,64 @@ _NODISCARD constexpr const _CharT* _Parse_chrono_format_specs( } namespace chrono { + // This functionality already exists in put_time, so why is it duplicated here? + // Unfortunately, put_time does not like to work with illogical dates, but the standard mandates that these are + // printed properly and treated specially. For example, consider operator<< for day. If day is 40, put_time asserts + // but the standard requires that it prints "40 is not a valid day". + template + bool _Try_simple_write(basic_ostream<_CharT>& _Os, char _Type, const tm& _Time) { + if (_Type == 'd' || _Type == 'e') { + if (_Time.tm_mday < 10) { + _Os << (_Type == 'd' ? _CharT{'0'} : _CharT{' '}); + } + _Os << _Time.tm_mday; + return true; + } + + if (_Type == 'm') { + if (_Time.tm_mon + 1 < 10) { + _Os << _CharT{'0'}; + } + _Os << _Time.tm_mon + 1; + return true; + } + + if (_Type == 'Y') { + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:04}"), _Time.tm_year + 1900); + return true; + } + + if (_Type == 'y') { + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), (_Time.tm_year + 1900) % 100); + return true; + } + + if (_Type == 'C') { + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), (_Time.tm_year + 1900) / 100); + return true; + } + + if (_Type == 'F') { + _Try_simple_write(_Os, 'Y', _Time); + _Os << _CharT{'-'}; + _Try_simple_write(_Os, 'm', _Time); + _Os << _CharT{'-'}; + _Try_simple_write(_Os, 'd', _Time); + return true; + } + + if (_Type == 'D') { + _Try_simple_write(_Os, 'm', _Time); + _Os << _CharT{'/'}; + _Try_simple_write(_Os, 'd', _Time); + _Os << _CharT{'/'}; + _Try_simple_write(_Os, 'y', _Time); + return true; + } + + return false; + } + /* template // clang-format off @@ -5431,10 +5489,8 @@ namespace chrono { template basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const day& _Val) { - if (static_cast(_Val) < 10) { - _Os << _CharT{'0'}; - } - _Os << static_cast(_Val); + const tm _Time = {.tm_mday = static_cast(static_cast(_Val))}; + _Try_simple_write(_Os, 'd', _Time); if (!_Val.ok()) { _Os << _STATICALLY_WIDEN(_CharT, " is not a valid day"); } @@ -5451,6 +5507,28 @@ namespace chrono { } return _Os << static_cast(_Val) << _STATICALLY_WIDEN(_CharT, " is not a valid month"); } + + template + basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const year& _Val) { + const tm _Time = {.tm_year = static_cast(_Val)}; + _Try_simple_write(_Os, 'Y', _Time); + if (!_Val.ok()) { + _Os << _STATICALLY_WIDEN(_CharT, " is not a valid year"); + } + return _Os; + } + + template + basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const year_month_day& _Val) { + const tm _Time = {.tm_mday = static_cast(static_cast(_Val.day())), + .tm_mon = static_cast(static_cast(_Val.month())) - 1, + .tm_year = static_cast(_Val.year())}; + _Try_simple_write(_Os, 'F', _Time); + if (!_Val.ok()) { + _Os << _STATICALLY_WIDEN(_CharT, " is not a valid date"); + } + return _Os; + } } // namespace chrono /* @@ -5472,7 +5550,8 @@ struct formatter<_CHRONO sys_time<_Duration>, _CharT> { template struct _Chrono_formatter { - auto _Parse(basic_format_parse_context<_CharT>& _Parse_ctx, string_view _Valid_types) { + template + _NODISCARD auto _Parse(basic_format_parse_context<_CharT>& _Parse_ctx) { _Chrono_specs_setter<_CharT, basic_format_parse_context<_CharT>> _Callback{_Specs, _Parse_ctx}; const auto _It = _Parse_chrono_format_specs(_Parse_ctx._Unchecked_begin(), _Parse_ctx._Unchecked_end(), _Callback); @@ -5495,7 +5574,7 @@ struct _Chrono_formatter { } for (const auto& _Spec : _List) { - if (_Spec._Type != '\0' && _RANGES find(_Valid_types, _Spec._Type) == _Valid_types.end()) { + if (_Spec._Type != '\0' && !_Is_type_valid<_Type>(_Spec._Type)) { _THROW(format_error("Invalid type.")); } _Check_modifier(_Spec._Type, _Spec._Modifier); @@ -5516,7 +5595,8 @@ struct _Chrono_formatter { _Allowed_bit _Allowed; }; - static const _Table_entry _Table[] = {{'d', _O_mod}, {'e', _O_mod}, {'m', _O_mod}}; + static const _Table_entry _Table[] = { + {'d', _O_mod}, {'e', _O_mod}, {'m', _O_mod}, {'Y', _E_mod}, {'y', _EO_mod}, {'C', _E_mod}}; const _Allowed_bit _Mod = _Modifier == 'E' ? _E_mod : _O_mod; @@ -5529,8 +5609,25 @@ struct _Chrono_formatter { _THROW(format_error("Incompatible modifier for type")); } + template + _NODISCARD constexpr bool _Is_type_valid(char _Type) { + if constexpr (is_same_v<_Ty, _CHRONO day>) { + return _Type == 'd' || _Type == 'e'; + } else if constexpr (is_same_v<_Ty, _CHRONO month>) { + return _Type == 'b' || _Type == 'B' || _Type == 'h' || _Type == 'm'; + } else if constexpr (is_same_v<_Ty, _CHRONO year>) { + return _Type == 'Y' || _Type == 'y' || _Type == 'C'; + } else if constexpr (is_same_v<_Ty, _CHRONO year_month_day>) { + return _Type == 'D' || _Type == 'F' || _Is_type_valid<_CHRONO year>(_Type) + || _Is_type_valid<_CHRONO month>(_Type) || _Is_type_valid<_CHRONO day>(_Type); + } else { + // TRANSITION, remove when all types are added + static_assert(_Always_false<_Ty>, "unsupported type"); + } + } + template - auto _Write(_FormatContext& _FormatCtx, const _Ty& _Val, const tm& _Time) { + _NODISCARD auto _Write(_FormatContext& _FormatCtx, const _Ty& _Val, const tm& _Time) { basic_stringstream<_CharT> _Stream; if (_No_chrono_specs) { @@ -5543,10 +5640,12 @@ struct _Chrono_formatter { continue; } - // TRANSITION, out of bounds may be legal for non-localized decimal printing, but it isn't very - // consistent. - if (!_Val.ok()) { - _THROW(format_error("Cannot print out-of-bounds time point.")); + if (_Needs_localization(_Spec)) { + if (!_Val.ok()) { + _THROW(format_error("Cannot localize out-of-bounds time point.")); + } + } else if (_CHRONO _Try_simple_write(_Stream, _Spec._Type, _Time)) { + continue; } _CharT _Fmt_str[4]; @@ -5566,18 +5665,38 @@ struct _Chrono_formatter { _Fmt_align::_Left, [&](auto _Out) { return _Fmt_write(_STD move(_Out), _Stream.view()); }); } + _NODISCARD bool _Needs_localization(const _Chrono_specs<_CharT>& _Spec) { + if (_Spec._Modifier != '\0') { + return true; + } + + switch (_Spec._Type) { + case 'a': + case 'A': + case 'b': + case 'B': + case 'h': + case 'z': + case 'Z': + return true; + + default: + return false; + } + } + _Chrono_format_specs<_CharT> _Specs{}; bool _No_chrono_specs = false; }; template struct formatter<_CHRONO day, _CharT> { - typename basic_format_parse_context<_CharT>::iterator parse(basic_format_parse_context<_CharT>& _Parse_ctx) { - return _Impl._Parse(_Parse_ctx, "de"); + auto parse(basic_format_parse_context<_CharT>& _Parse_ctx) { + return _Impl.template _Parse<_CHRONO day>(_Parse_ctx); } template - typename _FormatContext::iterator format(const _CHRONO day& _Val, _FormatContext& _FormatCtx) { + auto format(const _CHRONO day& _Val, _FormatContext& _FormatCtx) { const tm _Time = {.tm_mday = static_cast(static_cast(_Val))}; return _Impl._Write(_FormatCtx, _Val, _Time); } @@ -5588,17 +5707,51 @@ private: template struct formatter<_CHRONO month, _CharT> { - typename basic_format_parse_context<_CharT>::iterator parse(basic_format_parse_context<_CharT>& _Parse_ctx) { - return _Impl._Parse(_Parse_ctx, "bBhm"); + auto parse(basic_format_parse_context<_CharT>& _Parse_ctx) { + return _Impl.template _Parse<_CHRONO month>(_Parse_ctx); } template - typename _FormatContext::iterator format(const _CHRONO month& _Val, _FormatContext& _FormatCtx) { + auto format(const _CHRONO month& _Val, _FormatContext& _FormatCtx) { // tm_mon is [0, 11] while month is [1, 12] const tm _Time = {.tm_mon = static_cast(static_cast(_Val)) - 1}; return _Impl._Write(_FormatCtx, _Val, _Time); } +private: + _Chrono_formatter<_CharT, false> _Impl; +}; + +template +struct formatter<_CHRONO year, _CharT> { + auto parse(basic_format_parse_context<_CharT>& _Parse_ctx) { + return _Impl.template _Parse<_CHRONO year>(_Parse_ctx); + } + + template + auto format(const _CHRONO year& _Val, _FormatContext& _FormatCtx) { + const tm _Time = {.tm_year = static_cast(_Val)}; + return _Impl._Write(_FormatCtx, _Val, _Time); + } + +private: + _Chrono_formatter<_CharT, false> _Impl; +}; + +template +struct formatter<_CHRONO year_month_day, _CharT> { + auto parse(basic_format_parse_context<_CharT>& _Parse_ctx) { + return _Impl.template _Parse<_CHRONO year_month_day>(_Parse_ctx); + } + + template + auto format(const _CHRONO year_month_day& _Val, _FormatContext& _FormatCtx) { + const tm _Time = {.tm_mday = static_cast(static_cast(_Val.day())), + .tm_mon = static_cast(static_cast(_Val.month())) - 1, + .tm_year = static_cast(_Val.year())}; + return _Impl._Write(_FormatCtx, _Val, _Time); + } + private: _Chrono_formatter<_CharT, false> _Impl; }; 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 8c2fccb77c..78ce331bf2 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 @@ -294,7 +294,7 @@ void test_day_formatter() { assert(res == a8); assert(format(STR("{:%d %d %d}"), day{27}) == STR("27 27 27")); - throw_helper(STR("{:%d}"), day{200}); + assert(format(STR("{:%d}"), day{200}) == STR("200")); throw_helper(STR("{:%Ed}"), day{10}); assert(format(STR("{}"), day{0}) == STR("00 is not a valid day")); @@ -316,10 +316,10 @@ void test_month_formatter() { assert(format(STR("{:%m %Om}"), month{1}) == STR("01 01")); // Out of bounds month - throw_helper(STR("{:%m}"), month{0}); + assert(format(STR("{:%m}"), month{0}) == STR("00")); throw_helper(STR("{:%b}"), month{0}); throw_helper(STR("{:%h}"), month{0}); - throw_helper(STR("{::%B}"), month{0}); + throw_helper(STR("{:%B}"), month{0}); // Invalid specs throw_helper(STR("{:%A}"), month{1}); @@ -332,6 +332,31 @@ void test_month_formatter() { stream_helper(STR("20 is not a valid month"), month{20}); } +template +void test_year_formatter() { + assert(format(STR("{}"), year{0}) == STR("1900")); + assert(format(STR("{}"), year{121}) == STR("2021")); + + assert(format(STR("{:%Y %y%C}"), year{12}) == STR("1912 1219")); + assert(format(STR("{:%Y %y%C}"), year{-1900}) == STR("0000 0000")); + // TRANSITION, add tests for EY Oy Ey EC + + stream_helper(STR("1900"), year{0}); + stream_helper(STR("2000"), year{100}); + stream_helper(STR("-30868 is not a valid year"), year{-32768}); +} + +template +void test_year_month_day_formatter() { + auto invalid = year_month_day{year{0}, month{0}, day{1}}; + assert(format(STR("{}"), year_month_day{year{0}, month{1}, day{1}}) == STR("1900-01-01")); + stream_helper(STR("1900-01-01"), year_month_day{year{0}, month{1}, day{1}}); + stream_helper(STR("1900-00-01 is not a valid date"), invalid); + + assert(format(STR("{:%Y %b %d}"), year_month_day{year{0}, month{1}, day{1}}) == STR("1900 Jan 01")); + assert(format(STR("{:%F %D}"), invalid) == STR("1900-00-01 00/01/00")); +} + int main() { test_parse_conversion_spec(); test_parse_conversion_spec(); @@ -344,4 +369,10 @@ int main() { test_month_formatter(); test_month_formatter(); + + test_year_formatter(); + test_year_formatter(); + + test_year_month_day_formatter(); + test_year_month_day_formatter(); } From 8ec251d3e23edeb528b428272feb858f84943479 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Thu, 15 Apr 2021 10:22:20 -0700 Subject: [PATCH 2/7] Clang test --- tests/libcxx/expected_results.txt | 4 ++-- tests/libcxx/skipped_tests.txt | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index 3d5c538b7a..7cdd0131f8 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -283,9 +283,7 @@ std/utilities/time/time.cal/time.cal.mwdlast/time.cal.mwdlast.nonmembers/streami std/utilities/time/time.cal/time.cal.wdidx/time.cal.wdidx.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.wdlast/time.cal.wdlast.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.weekday/time.cal.weekday.nonmembers/streaming.pass.cpp FAIL -std/utilities/time/time.cal/time.cal.year/time.cal.year.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ym/time.cal.ym.nonmembers/streaming.pass.cpp FAIL -std/utilities/time/time.cal/time.cal.ymd/time.cal.ymd.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ymdlast/time.cal.ymdlast.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ymwdlast/time.cal.ymwdlast.nonmembers/streaming.pass.cpp FAIL @@ -882,3 +880,5 @@ std/utilities/function.objects/func.search/func.search.bmh/pred.pass.cpp PASS # Not yet implemented in libcxx and marked as "XFAIL: *" std/utilities/time/time.cal/time.cal.day/time.cal.day.nonmembers/streaming.pass.cpp SKIPPED std/utilities/time/time.cal/time.cal.month/time.cal.month.nonmembers/streaming.pass.cpp SKIPPED +std/utilities/time/time.cal/time.cal.year/time.cal.year.nonmembers/streaming.pass.cpp FAIL +std/utilities/time/time.cal/time.cal.ymd/time.cal.ymd.nonmembers/streaming.pass.cpp FAIL diff --git a/tests/libcxx/skipped_tests.txt b/tests/libcxx/skipped_tests.txt index acda0d4042..0b3515c145 100644 --- a/tests/libcxx/skipped_tests.txt +++ b/tests/libcxx/skipped_tests.txt @@ -283,9 +283,7 @@ utilities\time\time.cal\time.cal.mwdlast\time.cal.mwdlast.nonmembers\streaming.p utilities\time\time.cal\time.cal.wdidx\time.cal.wdidx.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.wdlast\time.cal.wdlast.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.weekday\time.cal.weekday.nonmembers\streaming.pass.cpp -utilities\time\time.cal\time.cal.year\time.cal.year.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ym\time.cal.ym.nonmembers\streaming.pass.cpp -utilities\time\time.cal\time.cal.ymd\time.cal.ymd.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ymdlast\time.cal.ymdlast.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ymwd\time.cal.ymwd.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ymwdlast\time.cal.ymwdlast.nonmembers\streaming.pass.cpp From 84e7c6116452dd969f69bfb962299d8bf96d7b1c Mon Sep 17 00:00:00 2001 From: Elnar D Date: Thu, 15 Apr 2021 10:43:46 -0700 Subject: [PATCH 3/7] Finesse the tests --- tests/libcxx/expected_results.txt | 6 +++--- tests/libcxx/skipped_tests.txt | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/libcxx/expected_results.txt b/tests/libcxx/expected_results.txt index 7cdd0131f8..a82a6c1a5b 100644 --- a/tests/libcxx/expected_results.txt +++ b/tests/libcxx/expected_results.txt @@ -284,7 +284,6 @@ std/utilities/time/time.cal/time.cal.wdidx/time.cal.wdidx.nonmembers/streaming.p std/utilities/time/time.cal/time.cal.wdlast/time.cal.wdlast.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.weekday/time.cal.weekday.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ym/time.cal.ym.nonmembers/streaming.pass.cpp FAIL -std/utilities/time/time.cal/time.cal.ymdlast/time.cal.ymdlast.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ymwd/time.cal.ymwd.nonmembers/streaming.pass.cpp FAIL std/utilities/time/time.cal/time.cal.ymwdlast/time.cal.ymwdlast.nonmembers/streaming.pass.cpp FAIL @@ -880,5 +879,6 @@ std/utilities/function.objects/func.search/func.search.bmh/pred.pass.cpp PASS # Not yet implemented in libcxx and marked as "XFAIL: *" std/utilities/time/time.cal/time.cal.day/time.cal.day.nonmembers/streaming.pass.cpp SKIPPED std/utilities/time/time.cal/time.cal.month/time.cal.month.nonmembers/streaming.pass.cpp SKIPPED -std/utilities/time/time.cal/time.cal.year/time.cal.year.nonmembers/streaming.pass.cpp FAIL -std/utilities/time/time.cal/time.cal.ymd/time.cal.ymd.nonmembers/streaming.pass.cpp FAIL +std/utilities/time/time.cal/time.cal.year/time.cal.year.nonmembers/streaming.pass.cpp SKIPPED +std/utilities/time/time.cal/time.cal.ymd/time.cal.ymd.nonmembers/streaming.pass.cpp SKIPPED +std/utilities/time/time.cal/time.cal.ymdlast/time.cal.ymdlast.nonmembers/streaming.pass.cpp SKIPPED diff --git a/tests/libcxx/skipped_tests.txt b/tests/libcxx/skipped_tests.txt index 0b3515c145..eebe1b513e 100644 --- a/tests/libcxx/skipped_tests.txt +++ b/tests/libcxx/skipped_tests.txt @@ -284,7 +284,6 @@ utilities\time\time.cal\time.cal.wdidx\time.cal.wdidx.nonmembers\streaming.pass. utilities\time\time.cal\time.cal.wdlast\time.cal.wdlast.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.weekday\time.cal.weekday.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ym\time.cal.ym.nonmembers\streaming.pass.cpp -utilities\time\time.cal\time.cal.ymdlast\time.cal.ymdlast.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ymwd\time.cal.ymwd.nonmembers\streaming.pass.cpp utilities\time\time.cal\time.cal.ymwdlast\time.cal.ymwdlast.nonmembers\streaming.pass.cpp From 99f656d123c075fb73d85ef82aa0c9ea89f4db2d Mon Sep 17 00:00:00 2001 From: Elnar D Date: Thu, 15 Apr 2021 11:56:43 -0700 Subject: [PATCH 4/7] review --- stl/inc/chrono | 45 ++++++++++++++++++--------------------------- 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 2fa92b2d3c..1d2517f51e 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5413,57 +5413,47 @@ namespace chrono { // printed properly and treated specially. For example, consider operator<< for day. If day is 40, put_time asserts // but the standard requires that it prints "40 is not a valid day". template - bool _Try_simple_write(basic_ostream<_CharT>& _Os, char _Type, const tm& _Time) { - if (_Type == 'd' || _Type == 'e') { + bool _Try_simple_write(basic_ostream<_CharT>& _Os, const char _Type, const tm& _Time) { + switch (_Type) { + case 'd': + case 'e': if (_Time.tm_mday < 10) { _Os << (_Type == 'd' ? _CharT{'0'} : _CharT{' '}); } _Os << _Time.tm_mday; return true; - } - - if (_Type == 'm') { + case 'm': if (_Time.tm_mon + 1 < 10) { _Os << _CharT{'0'}; } _Os << _Time.tm_mon + 1; return true; - } - - if (_Type == 'Y') { + case 'Y': _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:04}"), _Time.tm_year + 1900); return true; - } - - if (_Type == 'y') { + case 'y': _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), (_Time.tm_year + 1900) % 100); return true; - } - - if (_Type == 'C') { + case 'C': _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), (_Time.tm_year + 1900) / 100); return true; - } - - if (_Type == 'F') { + case 'F': _Try_simple_write(_Os, 'Y', _Time); _Os << _CharT{'-'}; _Try_simple_write(_Os, 'm', _Time); _Os << _CharT{'-'}; _Try_simple_write(_Os, 'd', _Time); return true; - } - - if (_Type == 'D') { + case 'D': _Try_simple_write(_Os, 'm', _Time); _Os << _CharT{'/'}; _Try_simple_write(_Os, 'd', _Time); _Os << _CharT{'/'}; _Try_simple_write(_Os, 'y', _Time); return true; + default: + return false; } - - return false; } /* @@ -5574,7 +5564,7 @@ struct _Chrono_formatter { } for (const auto& _Spec : _List) { - if (_Spec._Type != '\0' && !_Is_type_valid<_Type>(_Spec._Type)) { + if (_Spec._Type != '\0' && !_Is_valid_type<_Type>(_Spec._Type)) { _THROW(format_error("Invalid type.")); } _Check_modifier(_Spec._Type, _Spec._Modifier); @@ -5610,7 +5600,7 @@ struct _Chrono_formatter { } template - _NODISCARD constexpr bool _Is_type_valid(char _Type) { + _NODISCARD constexpr bool _Is_valid_type(const char _Type) noexcept { if constexpr (is_same_v<_Ty, _CHRONO day>) { return _Type == 'd' || _Type == 'e'; } else if constexpr (is_same_v<_Ty, _CHRONO month>) { @@ -5618,8 +5608,8 @@ struct _Chrono_formatter { } else if constexpr (is_same_v<_Ty, _CHRONO year>) { return _Type == 'Y' || _Type == 'y' || _Type == 'C'; } else if constexpr (is_same_v<_Ty, _CHRONO year_month_day>) { - return _Type == 'D' || _Type == 'F' || _Is_type_valid<_CHRONO year>(_Type) - || _Is_type_valid<_CHRONO month>(_Type) || _Is_type_valid<_CHRONO day>(_Type); + return _Type == 'D' || _Type == 'F' || _Is_valid_type<_CHRONO year>(_Type) + || _Is_valid_type<_CHRONO month>(_Type) || _Is_valid_type<_CHRONO day>(_Type); } else { // TRANSITION, remove when all types are added static_assert(_Always_false<_Ty>, "unsupported type"); @@ -5665,11 +5655,12 @@ struct _Chrono_formatter { _Fmt_align::_Left, [&](auto _Out) { return _Fmt_write(_STD move(_Out), _Stream.view()); }); } - _NODISCARD bool _Needs_localization(const _Chrono_specs<_CharT>& _Spec) { + _NODISCARD bool _Needs_localization(const _Chrono_specs<_CharT>& _Spec) noexcept { if (_Spec._Modifier != '\0') { return true; } + // [tab:time.format.spec] switch (_Spec._Type) { case 'a': case 'A': From 8d47d50be4a3b4e470db710b2b6d4baa325e8466 Mon Sep 17 00:00:00 2001 From: Elnar Dakeshov <55715127+eldakesh-ms@users.noreply.github.com> Date: Thu, 15 Apr 2021 12:55:55 -0700 Subject: [PATCH 5/7] Comment update Co-authored-by: mnatsuhara <46756417+mnatsuhara@users.noreply.github.com> --- stl/inc/chrono | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 1d2517f51e..da3ea300ba 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5408,10 +5408,9 @@ _NODISCARD constexpr const _CharT* _Parse_chrono_format_specs( } namespace chrono { - // This functionality already exists in put_time, so why is it duplicated here? - // Unfortunately, put_time does not like to work with illogical dates, but the standard mandates that these are - // printed properly and treated specially. For example, consider operator<< for day. If day is 40, put_time asserts - // but the standard requires that it prints "40 is not a valid day". + // This echoes the functionality of put_time, but is able to handle invalid dates (when !ok()) since the Standard + // mandates that invalid dates still be formatted properly. For example, put_time isn't able to handle a tm_mday of + // 40, but format("{:%d}", day{40}) should return "40" and operator<< for day prints "40 is not a valid day". template bool _Try_simple_write(basic_ostream<_CharT>& _Os, const char _Type, const tm& _Time) { switch (_Type) { From 5722065362f0b72cdcd05326cf827b245ccc2d84 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Thu, 15 Apr 2021 13:56:40 -0700 Subject: [PATCH 6/7] PR fixes --- stl/inc/chrono | 41 +++++++++++-------- .../test.cpp | 23 ++++++----- 2 files changed, 35 insertions(+), 29 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index da3ea300ba..e10179696b 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5409,10 +5409,12 @@ _NODISCARD constexpr const _CharT* _Parse_chrono_format_specs( namespace chrono { // This echoes the functionality of put_time, but is able to handle invalid dates (when !ok()) since the Standard - // mandates that invalid dates still be formatted properly. For example, put_time isn't able to handle a tm_mday of - // 40, but format("{:%d}", day{40}) should return "40" and operator<< for day prints "40 is not a valid day". + // mandates that invalid dates still be formatted properly. For example, put_time isn't able to handle a tm_mday of + // 40, but format("{:%d}", day{40}) should return "40" and operator<< for day prints "40 is not a valid day". template bool _Try_simple_write(basic_ostream<_CharT>& _Os, const char _Type, const tm& _Time) { + const auto _Year = _Time.tm_year + 1900; + const auto _Month = _Time.tm_mon + 1; switch (_Type) { case 'd': case 'e': @@ -5422,19 +5424,26 @@ namespace chrono { _Os << _Time.tm_mday; return true; case 'm': - if (_Time.tm_mon + 1 < 10) { + if (_Month < 10) { _Os << _CharT{'0'}; } - _Os << _Time.tm_mon + 1; + _Os << _Month; return true; case 'Y': - _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:04}"), _Time.tm_year + 1900); + if (_Year < 0) { + _Os << _CharT{'-'}; + } + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:04}"), _STD abs(_Year)); return true; case 'y': - _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), (_Time.tm_year + 1900) % 100); + _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), _Year < 0 ? 100 + (_Year % 100) : _Year % 100); return true; case 'C': - _Os << _STD format(_STATICALLY_WIDEN(_CharT, "{:02}"), (_Time.tm_year + 1900) / 100); + if (_Year < 0) { + _Os << _CharT{'-'}; + } + _Os << _STD format( + _STATICALLY_WIDEN(_CharT, "{:02}"), _STD abs(_Time_parse_fields::_Decompose_year(_Year).first) / 100); return true; case 'F': _Try_simple_write(_Os, 'Y', _Time); @@ -5499,7 +5508,7 @@ namespace chrono { template basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const year& _Val) { - const tm _Time = {.tm_year = static_cast(_Val)}; + const tm _Time = {.tm_year = static_cast(_Val) - 1900}; _Try_simple_write(_Os, 'Y', _Time); if (!_Val.ok()) { _Os << _STATICALLY_WIDEN(_CharT, " is not a valid year"); @@ -5511,7 +5520,7 @@ namespace chrono { basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const year_month_day& _Val) { const tm _Time = {.tm_mday = static_cast(static_cast(_Val.day())), .tm_mon = static_cast(static_cast(_Val.month())) - 1, - .tm_year = static_cast(_Val.year())}; + .tm_year = static_cast(_Val.year()) - 1900}; _Try_simple_write(_Os, 'F', _Time); if (!_Val.ok()) { _Os << _STATICALLY_WIDEN(_CharT, " is not a valid date"); @@ -5629,11 +5638,11 @@ struct _Chrono_formatter { continue; } - if (_Needs_localization(_Spec)) { + if (_Type_needs_bounds_checking(_Spec)) { if (!_Val.ok()) { _THROW(format_error("Cannot localize out-of-bounds time point.")); } - } else if (_CHRONO _Try_simple_write(_Stream, _Spec._Type, _Time)) { + } else if (_Spec._Modifier == '\0' && _CHRONO _Try_simple_write(_Stream, _Spec._Type, _Time)) { continue; } @@ -5654,11 +5663,7 @@ struct _Chrono_formatter { _Fmt_align::_Left, [&](auto _Out) { return _Fmt_write(_STD move(_Out), _Stream.view()); }); } - _NODISCARD bool _Needs_localization(const _Chrono_specs<_CharT>& _Spec) noexcept { - if (_Spec._Modifier != '\0') { - return true; - } - + _NODISCARD bool _Type_needs_bounds_checking(const _Chrono_specs<_CharT>& _Spec) noexcept { // [tab:time.format.spec] switch (_Spec._Type) { case 'a': @@ -5720,7 +5725,7 @@ struct formatter<_CHRONO year, _CharT> { template auto format(const _CHRONO year& _Val, _FormatContext& _FormatCtx) { - const tm _Time = {.tm_year = static_cast(_Val)}; + const tm _Time = {.tm_year = static_cast(_Val) - 1900}; return _Impl._Write(_FormatCtx, _Val, _Time); } @@ -5738,7 +5743,7 @@ struct formatter<_CHRONO year_month_day, _CharT> { auto format(const _CHRONO year_month_day& _Val, _FormatContext& _FormatCtx) { const tm _Time = {.tm_mday = static_cast(static_cast(_Val.day())), .tm_mon = static_cast(static_cast(_Val.month())) - 1, - .tm_year = static_cast(_Val.year())}; + .tm_year = static_cast(_Val.year()) - 1900}; return _Impl._Write(_FormatCtx, _Val, _Time); } 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 78ce331bf2..7e300cf3c0 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 @@ -334,26 +334,27 @@ void test_month_formatter() { template void test_year_formatter() { - assert(format(STR("{}"), year{0}) == STR("1900")); - assert(format(STR("{}"), year{121}) == STR("2021")); + assert(format(STR("{}"), year{0}) == STR("0000")); + assert(format(STR("{}"), year{-200}) == STR("-0200")); + assert(format(STR("{}"), year{121}) == STR("0121")); - assert(format(STR("{:%Y %y%C}"), year{12}) == STR("1912 1219")); - assert(format(STR("{:%Y %y%C}"), year{-1900}) == STR("0000 0000")); + assert(format(STR("{:%Y %y%C}"), year{1912}) == STR("1912 1219")); + assert(format(STR("{:%Y %y%C}"), year{-1912}) == STR("-1912 88-20")); // TRANSITION, add tests for EY Oy Ey EC - stream_helper(STR("1900"), year{0}); - stream_helper(STR("2000"), year{100}); - stream_helper(STR("-30868 is not a valid year"), year{-32768}); + stream_helper(STR("1900"), year{1900}); + stream_helper(STR("2000"), year{2000}); + stream_helper(STR("-32768 is not a valid year"), year{-32768}); } template void test_year_month_day_formatter() { - auto invalid = year_month_day{year{0}, month{0}, day{1}}; - assert(format(STR("{}"), year_month_day{year{0}, month{1}, day{1}}) == STR("1900-01-01")); - stream_helper(STR("1900-01-01"), year_month_day{year{0}, month{1}, day{1}}); + auto invalid = year_month_day{year{1900}, month{0}, day{1}}; + assert(format(STR("{}"), year_month_day{year{1900}, month{1}, day{1}}) == STR("1900-01-01")); + stream_helper(STR("1900-01-01"), year_month_day{year{1900}, month{1}, day{1}}); stream_helper(STR("1900-00-01 is not a valid date"), invalid); - assert(format(STR("{:%Y %b %d}"), year_month_day{year{0}, month{1}, day{1}}) == STR("1900 Jan 01")); + assert(format(STR("{:%Y %b %d}"), year_month_day{year{1900}, month{1}, day{1}}) == STR("1900 Jan 01")); assert(format(STR("{:%F %D}"), invalid) == STR("1900-00-01 00/01/00")); } From c89a21e2885910e0013b97afcdf1f3b50cb23e83 Mon Sep 17 00:00:00 2001 From: Elnar D Date: Thu, 15 Apr 2021 15:02:51 -0700 Subject: [PATCH 7/7] Tests --- stl/inc/chrono | 4 ++-- .../test.cpp | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index e10179696b..773cb5bb6f 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5548,7 +5548,7 @@ struct formatter<_CHRONO sys_time<_Duration>, _CharT> { template struct _Chrono_formatter { - template + template _NODISCARD auto _Parse(basic_format_parse_context<_CharT>& _Parse_ctx) { _Chrono_specs_setter<_CharT, basic_format_parse_context<_CharT>> _Callback{_Specs, _Parse_ctx}; const auto _It = @@ -5572,7 +5572,7 @@ struct _Chrono_formatter { } for (const auto& _Spec : _List) { - if (_Spec._Type != '\0' && !_Is_valid_type<_Type>(_Spec._Type)) { + if (_Spec._Type != '\0' && !_Is_valid_type<_Ty>(_Spec._Type)) { _THROW(format_error("Invalid type.")); } _Check_modifier(_Spec._Type, _Spec._Modifier); 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 7e300cf3c0..b115adce82 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 @@ -349,13 +349,13 @@ void test_year_formatter() { template void test_year_month_day_formatter() { - auto invalid = year_month_day{year{1900}, month{0}, day{1}}; - assert(format(STR("{}"), year_month_day{year{1900}, month{1}, day{1}}) == STR("1900-01-01")); - stream_helper(STR("1900-01-01"), year_month_day{year{1900}, month{1}, day{1}}); - stream_helper(STR("1900-00-01 is not a valid date"), invalid); + year_month_day invalid{year{1234}, month{0}, day{31}}; + assert(format(STR("{}"), year_month_day{year{1900}, month{2}, day{1}}) == STR("1900-02-01")); + stream_helper(STR("1900-02-01"), year_month_day{year{1900}, month{2}, day{1}}); + stream_helper(STR("1234-00-31 is not a valid date"), invalid); - assert(format(STR("{:%Y %b %d}"), year_month_day{year{1900}, month{1}, day{1}}) == STR("1900 Jan 01")); - assert(format(STR("{:%F %D}"), invalid) == STR("1900-00-01 00/01/00")); + assert(format(STR("{:%Y %b %d}"), year_month_day{year{1234}, month{5}, day{6}}) == STR("1234 May 06")); + assert(format(STR("{:%F %D}"), invalid) == STR("1234-00-31 00/31/34")); } int main() {