From db3c7a1727ec7aa871fa670953bf0cf36e05a0f0 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Tue, 17 Oct 2023 19:42:01 -0700 Subject: [PATCH 01/20] chrono cleanups: Make `_THROW` a variadic macro so we never need extra parens. --- stl/inc/chrono | 4 ++-- stl/inc/yvals.h | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index d57acea0a2e..b2f65eb7c5c 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -1712,7 +1712,7 @@ namespace chrono { template [[noreturn]] void _Throw_nonexistent_local_time(const local_time<_Duration>& _Tp, const local_info& _Info) { - _THROW((nonexistent_local_time{_Tp, _Info})); + _THROW(nonexistent_local_time{_Tp, _Info}); } _EXPORT_STD class ambiguous_local_time : public runtime_error { @@ -1735,7 +1735,7 @@ namespace chrono { template [[noreturn]] void _Throw_ambiguous_local_time(const local_time<_Duration>& _Tp, const local_info& _Info) { - _THROW((ambiguous_local_time{_Tp, _Info})); + _THROW(ambiguous_local_time{_Tp, _Info}); } // [time.zone.timezone] diff --git a/stl/inc/yvals.h b/stl/inc/yvals.h index 03d40d869bc..e032fa5c8f3 100644 --- a/stl/inc/yvals.h +++ b/stl/inc/yvals.h @@ -451,8 +451,8 @@ class _CRTIMP2_PURE_IMPORT _EmptyLockit { // empty lock class used for bin compa catch (...) { #define _CATCH_END } -#define _RERAISE throw -#define _THROW(x) throw x +#define _RERAISE throw +#define _THROW(...) throw __VA_ARGS__ #else // ^^^ _HAS_EXCEPTIONS / !_HAS_EXCEPTIONS vvv #define _TRY_BEGIN \ @@ -475,7 +475,7 @@ class _CRTIMP2_PURE_IMPORT _EmptyLockit { // empty lock class used for bin compa #endif #define _RERAISE -#define _THROW(x) x._Raise() +#define _THROW(...) __VA_ARGS__._Raise() #endif // ^^^ !_HAS_EXCEPTIONS ^^^ _STD_END From af7771179bd3c8c5298282f729b5f6de01a04ab0 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 17:48:56 -0700 Subject: [PATCH 02/20] chrono cleanups: Replace `decltype(tzdb::zones)` and `decltype(tzdb::links)` with their actual types. Works towards GH 1805. These were relics of an early attempt to use special allocators, which was foiled by the Standard's insistence on `std::allocator`. --- stl/inc/chrono | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index b2f65eb7c5c..723d21f8952 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2094,7 +2094,7 @@ namespace chrono { } }; - _NODISCARD inline tuple _Tzdb_generate_time_zones() { + _NODISCARD inline tuple, vector> _Tzdb_generate_time_zones() { unique_ptr<__std_tzdb_time_zones_info, _Tzdb_deleter<__std_tzdb_time_zones_info>> _Info{ __std_tzdb_get_time_zones()}; if (_Info == nullptr) { @@ -2105,8 +2105,8 @@ namespace chrono { _Xruntime_error("Internal error loading IANA database information"); } - decltype(tzdb::zones) _Time_zones; - decltype(tzdb::links) _Time_zone_links; + vector _Time_zones; + vector _Time_zone_links; for (size_t _Idx = 0; _Idx < _Info->_Num_time_zones; ++_Idx) { const string_view _Name{_Info->_Names[_Idx]}; if (_Info->_Links[_Idx] == nullptr) { From 3cb601fa07184b1663469312dc235ac8be8034b6 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 17:57:45 -0700 Subject: [PATCH 03/20] chrono cleanups: `_Crt_allocator` should provide equality. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/__msvc_tzdb.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/stl/inc/__msvc_tzdb.hpp b/stl/inc/__msvc_tzdb.hpp index c8c72719047..f571becdb74 100644 --- a/stl/inc/__msvc_tzdb.hpp +++ b/stl/inc/__msvc_tzdb.hpp @@ -151,6 +151,11 @@ class _Crt_allocator { void deallocate(_Ty* const _Ptr, size_t) noexcept { __std_free_crt(_Ptr); } + + template + _NODISCARD bool operator==(const _Crt_allocator<_Other>&) const noexcept { + return true; + } }; _STD_END From e6f5cb3c636f602903ae80aaa6e2cfdd2d2ff68e Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 18:09:34 -0700 Subject: [PATCH 04/20] chrono cleanups: `time_zone` and `time_zone_link`'s internal constructors should be tagged. We also need to update some test code that was using these internal constructors. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 21 +++++++++++++------ .../test.cpp | 7 ++++--- .../test.cpp | 7 ++++--- .../test.cpp | 12 +++++------ 4 files changed, 29 insertions(+), 18 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 723d21f8952..65a337c049d 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -1742,9 +1742,13 @@ namespace chrono { _EXPORT_STD enum class choose { earliest, latest }; + struct _Secret_time_zone_construct_tag { + explicit _Secret_time_zone_construct_tag() = default; + }; + _EXPORT_STD class time_zone { public: - explicit time_zone(string_view _Name_) : _Name(_Name_) {} + explicit time_zone(_Secret_time_zone_construct_tag, string_view _Name_) : _Name(_Name_) {} time_zone(time_zone&&) = default; time_zone& operator=(time_zone&&) = default; @@ -2012,9 +2016,14 @@ namespace chrono { // [time.zone.link] + struct _Secret_time_zone_link_construct_tag { + explicit _Secret_time_zone_link_construct_tag() = default; + }; + _EXPORT_STD class time_zone_link { public: - explicit time_zone_link(string_view _Name_, string_view _Target_) : _Name(_Name_), _Target(_Target_) {} + explicit time_zone_link(_Secret_time_zone_link_construct_tag, string_view _Name_, string_view _Target_) + : _Name(_Name_), _Target(_Target_) {} time_zone_link(time_zone_link&&) = default; time_zone_link& operator=(time_zone_link&&) = default; @@ -2110,10 +2119,10 @@ namespace chrono { for (size_t _Idx = 0; _Idx < _Info->_Num_time_zones; ++_Idx) { const string_view _Name{_Info->_Names[_Idx]}; if (_Info->_Links[_Idx] == nullptr) { - _Time_zones.emplace_back(_Name); + _Time_zones.emplace_back(_Secret_time_zone_construct_tag{}, _Name); } else { const string_view _Target{_Info->_Links[_Idx]}; - _Time_zone_links.emplace_back(_Name, _Target); + _Time_zone_links.emplace_back(_Secret_time_zone_link_construct_tag{}, _Name, _Target); } } @@ -2259,13 +2268,13 @@ namespace chrono { vector _Zones; _Zones.reserve(_Tzdb.zones.size()); for (const auto& _Tz : _Tzdb.zones) { - _Zones.emplace_back(_Tz.name()); + _Zones.emplace_back(_Secret_time_zone_construct_tag{}, _Tz.name()); } vector _Links; _Links.reserve(_Tzdb.links.size()); for (const auto& _Link : _Tzdb.links) { - _Links.emplace_back(_Link.name(), _Link.target()); + _Links.emplace_back(_Secret_time_zone_link_construct_tag{}, _Link.name(), _Link.target()); } auto _Version = _Tzdb_update_version(_Tzdb.version, _Leap_sec.size()); diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_clocks/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_clocks/test.cpp index ab019ad78e5..9ed77e4bca7 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_clocks/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_clocks/test.cpp @@ -385,10 +385,11 @@ tzdb copy_tzdb() { const auto& my_tzdb = get_tzdb_list().front(); vector zones; vector links; - transform(my_tzdb.zones.begin(), my_tzdb.zones.end(), back_inserter(zones), - [](const auto& tz) { return time_zone{tz.name()}; }); + transform(my_tzdb.zones.begin(), my_tzdb.zones.end(), back_inserter(zones), [](const auto& tz) { + return time_zone{_Secret_time_zone_construct_tag{}, tz.name()}; + }); transform(my_tzdb.links.begin(), my_tzdb.links.end(), back_inserter(links), [](const auto& link) { - return time_zone_link{link.name(), link.target()}; + return time_zone_link{_Secret_time_zone_link_construct_tag{}, link.name(), link.target()}; }); return {my_tzdb.version, move(zones), move(links), my_tzdb.leap_seconds, my_tzdb._All_ls_positive}; diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp index 40f0f1965b4..64fbc3c2292 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp @@ -921,10 +921,11 @@ tzdb copy_tzdb() { const auto& my_tzdb = get_tzdb_list().front(); vector zones; vector links; - transform(my_tzdb.zones.begin(), my_tzdb.zones.end(), back_inserter(zones), - [](const auto& tz) { return time_zone{tz.name()}; }); + transform(my_tzdb.zones.begin(), my_tzdb.zones.end(), back_inserter(zones), [](const auto& tz) { + return time_zone{_Secret_time_zone_construct_tag{}, tz.name()}; + }); transform(my_tzdb.links.begin(), my_tzdb.links.end(), back_inserter(links), [](const auto& link) { - return time_zone_link{link.name(), link.target()}; + return time_zone_link{_Secret_time_zone_link_construct_tag{}, link.name(), link.target()}; }); return {my_tzdb.version, move(zones), move(links), my_tzdb.leap_seconds, my_tzdb._All_ls_positive}; diff --git a/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp b/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp index a3811c95cab..a55118c262d 100644 --- a/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp +++ b/tests/std/tests/P0355R7_calendars_and_time_zones_time_zones/test.cpp @@ -109,9 +109,9 @@ void timezone_names_test() { try_locate_invalid_zone(my_tzdb, "AEST"); // Comparison operators - const time_zone tz1{"Earlier"}; - const time_zone tz2{"Earlier"}; - const time_zone tz3{"Later"}; + const time_zone tz1{_Secret_time_zone_construct_tag{}, "Earlier"}; + const time_zone tz2{_Secret_time_zone_construct_tag{}, "Earlier"}; + const time_zone tz3{_Secret_time_zone_construct_tag{}, "Later"}; assert(tz1 == tz2); assert(tz1 != tz3); #ifdef __cpp_lib_concepts @@ -120,9 +120,9 @@ void timezone_names_test() { assert(tz3 <=> tz1 == strong_ordering::greater); #endif // __cpp_lib_concepts - const time_zone_link link1{"Earlier", "Target"}; - const time_zone_link link2{"Earlier", "Is"}; - const time_zone_link link3{"Later", "Ignored"}; + const time_zone_link link1{_Secret_time_zone_link_construct_tag{}, "Earlier", "Target"}; + const time_zone_link link2{_Secret_time_zone_link_construct_tag{}, "Earlier", "Is"}; + const time_zone_link link3{_Secret_time_zone_link_construct_tag{}, "Later", "Ignored"}; assert(link1 == link2); assert(link1 != link3); #ifdef __cpp_lib_concepts From 67ab97d56559c88dba95df3320b546c2c4b5e9c5 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 18:18:48 -0700 Subject: [PATCH 05/20] chrono cleanups: Return `const char*` as `string` without braces. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 65a337c049d..057b92b56b7 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -2065,7 +2065,7 @@ namespace chrono { _Xruntime_error("Internal error loading IANA database information"); } - return {_Info->_Tz_name}; + return _Info->_Tz_name; } template From de8760780ccc33396b54e21f758f56e1601bbccc Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 18:51:24 -0700 Subject: [PATCH 06/20] chrono cleanups: Extract `_Make_unique_tzdb_info()`. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 51 +++++++++++++++++++++++--------------------------- 1 file changed, 23 insertions(+), 28 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 057b92b56b7..d61e4282eae 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -1740,6 +1740,25 @@ namespace chrono { // [time.zone.timezone] + template + _NODISCARD auto _Make_unique_tzdb_info(_Types&&... _Args) { + const auto _Raw_ptr = _Get_tzdb_info(_STD forward<_Types>(_Args)...); + + using _Tzdb_info = remove_pointer_t; + + unique_ptr<_Tzdb_info, _Tzdb_deleter<_Tzdb_info>> _Info{_Raw_ptr}; + + if (_Info == nullptr) { + _Xbad_alloc(); + } else if (_Info->_Err == __std_tzdb_error::_Win_error) { + _XGetLastError(); + } else if (_Info->_Err == __std_tzdb_error::_Icu_error) { + _Xruntime_error("Internal error loading IANA database information"); + } + + return _Info; + } + _EXPORT_STD enum class choose { earliest, latest }; struct _Secret_time_zone_construct_tag { @@ -1815,16 +1834,8 @@ namespace chrono { const auto _Tz_len = _Name.length(); - const unique_ptr<__std_tzdb_sys_info, _Tzdb_deleter<__std_tzdb_sys_info>> _Info{ - __std_tzdb_get_sys_info(_Tz_arg.c_str(), _Tz_len, _Internal_dur.count())}; - - if (_Info == nullptr) { - _Xbad_alloc(); - } else if (_Info->_Err == __std_tzdb_error::_Win_error) { - _XGetLastError(); - } else if (_Info->_Err == __std_tzdb_error::_Icu_error) { - _Xruntime_error("Internal error loading IANA database information"); - } + const auto _Info = + _Make_unique_tzdb_info<__std_tzdb_get_sys_info>(_Tz_arg.c_str(), _Tz_len, _Internal_dur.count()); constexpr auto _Min_internal = _CHRONO duration_cast<_Internal_duration>(_Min_seconds.time_since_epoch()).count(); @@ -2055,15 +2066,7 @@ namespace chrono { // [time.zone.db] _NODISCARD inline string _Tzdb_generate_current_zone() { - unique_ptr<__std_tzdb_current_zone_info, _Tzdb_deleter<__std_tzdb_current_zone_info>> _Info{ - __std_tzdb_get_current_zone()}; - if (_Info == nullptr) { - _Xbad_alloc(); - } else if (_Info->_Err == __std_tzdb_error::_Win_error) { - _XGetLastError(); - } else if (_Info->_Err == __std_tzdb_error::_Icu_error) { - _Xruntime_error("Internal error loading IANA database information"); - } + auto _Info = _Make_unique_tzdb_info<__std_tzdb_get_current_zone>(); return _Info->_Tz_name; } @@ -2104,15 +2107,7 @@ namespace chrono { }; _NODISCARD inline tuple, vector> _Tzdb_generate_time_zones() { - unique_ptr<__std_tzdb_time_zones_info, _Tzdb_deleter<__std_tzdb_time_zones_info>> _Info{ - __std_tzdb_get_time_zones()}; - if (_Info == nullptr) { - _Xbad_alloc(); - } else if (_Info->_Err == __std_tzdb_error::_Win_error) { - _XGetLastError(); - } else if (_Info->_Err == __std_tzdb_error::_Icu_error) { - _Xruntime_error("Internal error loading IANA database information"); - } + auto _Info = _Make_unique_tzdb_info<__std_tzdb_get_time_zones>(); vector _Time_zones; vector _Time_zone_links; From aa16fb51a66b29ce7f4d1374d9aaf732a26b48a5 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 19:47:08 -0700 Subject: [PATCH 07/20] chrono cleanups: `exchange()` the contained value instead of the entire `optional`. We know `_Val.has_value()` here, so we can directly reach into the `optional`. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index d61e4282eae..85266abb68e 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -3174,7 +3174,7 @@ namespace chrono { _Val = _New; return true; } else { - return _STD exchange(_Val, _New) == _New; + return _STD exchange(*_Val, _New) == _New; } } From 423374f52ea97dc82e9eba687c04ab77f57cff20 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 19:53:09 -0700 Subject: [PATCH 08/20] chrono cleanups: Use positive test and early `return`. Not mentioned in GH 1789, just noticed myself. The negative test wasn't providing any value here. Additionally, the two cases are different enough that there's no symmetry advantage to keeping the `else`, so I believe this is clearer with an early `return`. --- stl/inc/chrono | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 85266abb68e..c0bc20d3197 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -3170,12 +3170,12 @@ namespace chrono { } } - if (!_Val.has_value()) { - _Val = _New; - return true; - } else { + if (_Val.has_value()) { return _STD exchange(*_Val, _New) == _New; } + + _Val = _New; + return true; } _NODISCARD static pair _Decompose_year(const int _Year) { From d6179a17cf4e17b88e4443d1a2748182ec637e85 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 19:55:53 -0700 Subject: [PATCH 09/20] chrono cleanups: Drop empty line at the top of `_Make_time_point()`. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index c0bc20d3197..8089313721d 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -3585,7 +3585,6 @@ namespace chrono { template _NODISCARD bool _Make_time_point(_DurationType& _Dur, _Leap_second_rep _Leap) { - const bool _Consistent{_Calculate_hour24() && _Calculate_year_fields() && _Calculate_ymd()}; if (!_Consistent || !_Apply_duration_fields<_Parse_tp_or_duration::_Time_point>(_Dur)) { return false; From b3a28bdf1207e9ae6e5d13ea752f637697c5354b Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 19:57:40 -0700 Subject: [PATCH 10/20] chrono cleanups: Consistently say "leap second deletion" in comments. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 8089313721d..c9893bcc68e 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -3612,7 +3612,7 @@ namespace chrono { return false; } else { // It's possible that the parsed time doesn't exist because (a) _Seconds == 60 and there *isn't* a - // leap second insertion or (b) _Seconds >= 59 and there *is* a leap second subtraction. + // leap second insertion or (b) _Seconds >= 59 and there *is* a leap second deletion. const auto& _Tzdb{_CHRONO get_tzdb()}; const bool _Possible_insertion{_Second == 60}; From aff96428bfe3de69486ab68fbe3ffadf4dc4822a Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:15:40 -0700 Subject: [PATCH 11/20] chrono cleanups: `_InIt` is always `istreambuf_iterator`, not an arbitrary input iterator. I decided that making the code specific to `istreambuf_iterator` was better than just renaming the parameters to `_Iter`. Accordingly, we can simplify the code because we know that the `value_type` will be `_CharT`. Mentioned in GH 1789, works towards GH 1805. --- stl/inc/chrono | 51 +++++++++++++++++++++++++------------------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index c9893bcc68e..0544e5e1c0f 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -3728,15 +3728,14 @@ namespace chrono { } } - template - _NODISCARD _InIt _Parse_time_field(_InIt _First, ios_base& _Iosbase, ios_base::iostate& _State, - const char _Flag, const char _Modifier, const unsigned int _Width, - const unsigned int _Subsecond_precision) { - using _CharT = _InIt::value_type; + template + _NODISCARD istreambuf_iterator<_CharT, _Traits> _Parse_time_field(istreambuf_iterator<_CharT, _Traits> _First, + ios_base& _Iosbase, ios_base::iostate& _State, const char _Flag, const char _Modifier, + const unsigned int _Width, const unsigned int _Subsecond_precision) { const auto& _Ctype_fac = _STD use_facet>(_Iosbase.getloc()); const auto& _Time_fac = _STD use_facet>(_Iosbase.getloc()); - constexpr _InIt _Last{}; + constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; int _Val{0}; switch (_Flag) { @@ -4054,10 +4053,11 @@ namespace chrono { return _First; } - template - _NODISCARD _InIt _Parse_time_field_restricted(_InIt _First, ios_base& _Iosbase, ios_base::iostate& _State, + template + _NODISCARD istreambuf_iterator<_CharT, _Traits> _Parse_time_field_restricted( + istreambuf_iterator<_CharT, _Traits> _First, ios_base& _Iosbase, ios_base::iostate& _State, const char* _Fmt, const unsigned int _Subsecond_precision = 0) { - using _Ctype = ctype; + using _Ctype = ctype<_CharT>; // Parses a restricted format string. It generally doesn't handle anything parsed outside of // _Parse_time_field: // (a) any whitespace (' ', %n, %t) @@ -4066,7 +4066,7 @@ namespace chrono { // (d) width parameter // It also assumes a valid format string, specifically that '%' is always followed by a flag. const _Ctype& _Ctype_fac{_STD use_facet<_Ctype>(_Iosbase.getloc())}; - constexpr _InIt _Last{}; + constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; while (*_Fmt != '\0' && (_State & ~ios_base::eofbit) == ios_base::goodbit) { if (_First == _Last) { @@ -4082,11 +4082,10 @@ namespace chrono { return _First; } - template - _NODISCARD ios_base::iostate _Get_fixed(_InIt& _First, unsigned int _Width, - const ctype& _Ctype_fac, - const numpunct& _Numpunct_fac) { - constexpr _InIt _Last{}; + template + _NODISCARD ios_base::iostate _Get_fixed(istreambuf_iterator<_CharT, _Traits>& _First, unsigned int _Width, + const ctype<_CharT>& _Ctype_fac, const numpunct<_CharT>& _Numpunct_fac) { + constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; while (_First != _Last && _Ctype_fac.is(ctype_base::space, *_First) && _Width > 0) { ++_First; @@ -4135,10 +4134,10 @@ namespace chrono { return _State; } - template - _NODISCARD ios_base::iostate _Get_int( - _InIt& _First, unsigned int _Width, int& _Val, const ctype& _Ctype_fac) { - constexpr _InIt _Last{}; + template + _NODISCARD ios_base::iostate _Get_int(istreambuf_iterator<_CharT, _Traits>& _First, unsigned int _Width, + int& _Val, const ctype<_CharT>& _Ctype_fac) { + constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; while (_First != _Last && _Ctype_fac.is(ctype_base::space, *_First) && _Width > 0) { ++_First; @@ -4197,10 +4196,10 @@ namespace chrono { return _State; } - template - _NODISCARD ios_base::iostate _Get_tz_offset( - _InIt& _First, const ctype& _Ctype_fac, const bool _Is_modified, int& _Offset) { - constexpr _InIt _Last{}; + template + _NODISCARD ios_base::iostate _Get_tz_offset(istreambuf_iterator<_CharT, _Traits>& _First, + const ctype<_CharT>& _Ctype_fac, const bool _Is_modified, int& _Offset) { + constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; if (_First == _Last) { return ios_base::eofbit; } @@ -4270,10 +4269,10 @@ namespace chrono { return ios_base::goodbit; } - template + template _NODISCARD ios_base::iostate _Get_tz_name( - _InIt& _First, const ctype& _Ctype_fac, string& _Tz_name) { - constexpr _InIt _Last{}; + istreambuf_iterator<_CharT, _Traits>& _First, const ctype<_CharT>& _Ctype_fac, string& _Tz_name) { + constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; _Tz_name.clear(); while (_First != _Last) { const char _Ch{_Ctype_fac.narrow(*_First)}; From 29ee886d52f56950e96ab9ee76ada719fd96bc34 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:18:54 -0700 Subject: [PATCH 12/20] chrono cleanups: Use `const auto&` to avoid `_Ctype` typedef. Now that `ctype<_CharT>` is so short, we can just mention it in-place. Not mentioned in GH 1789, just noticed myself. --- stl/inc/chrono | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 0544e5e1c0f..84e46bb1624 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -4057,7 +4057,6 @@ namespace chrono { _NODISCARD istreambuf_iterator<_CharT, _Traits> _Parse_time_field_restricted( istreambuf_iterator<_CharT, _Traits> _First, ios_base& _Iosbase, ios_base::iostate& _State, const char* _Fmt, const unsigned int _Subsecond_precision = 0) { - using _Ctype = ctype<_CharT>; // Parses a restricted format string. It generally doesn't handle anything parsed outside of // _Parse_time_field: // (a) any whitespace (' ', %n, %t) @@ -4065,7 +4064,7 @@ namespace chrono { // (c) E or O modifiers // (d) width parameter // It also assumes a valid format string, specifically that '%' is always followed by a flag. - const _Ctype& _Ctype_fac{_STD use_facet<_Ctype>(_Iosbase.getloc())}; + const auto& _Ctype_fac{_STD use_facet>(_Iosbase.getloc())}; constexpr istreambuf_iterator<_CharT, _Traits> _Last{}; while (*_Fmt != '\0' && (_State & ~ios_base::eofbit) == ios_base::goodbit) { From d99cc4fb9dfae7e849449b3f48139229249fa1fd Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:34:28 -0700 Subject: [PATCH 13/20] chrono cleanups: `'q'` and `'Q'` should always be given `duration`s, otherwise there's a logic error in `_Is_valid_type`. Mentioned in GH 1870, works towards GH 1805. --- stl/inc/chrono | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/stl/inc/chrono b/stl/inc/chrono index 84e46bb1624..fb8d56c5b82 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5550,11 +5550,15 @@ namespace chrono { case 'q': if constexpr (_Is_specialization_v<_Ty, duration>) { _Write_unit_suffix(_Os); + } else { + _STL_INTERNAL_CHECK(false); } return true; case 'Q': if constexpr (_Is_specialization_v<_Ty, duration>) { _Os << _STD abs(_Val.count()); + } else { + _STL_INTERNAL_CHECK(false); } return true; case 'm': // Print months as a decimal, even if invalid. From 386d32f5c417cc38a48ba724b8e8f8be8cb529b4 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:40:14 -0700 Subject: [PATCH 14/20] chrono cleanups: Avoid dead `return`s after `if constexpr`. Unlike runtime `if`-statements where early `return`s are often desirable, `if constexpr` should avoid emitting dead code. Not mentioned in GH 1870, just noticed myself. --- stl/inc/chrono | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index fb8d56c5b82..e3781291382 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5525,9 +5525,9 @@ namespace chrono { _CharT _Fmt_str[4]; _Os << _STD put_time(&_Time, _Fmt_string({._Type = _Gregorian_type}, _Fmt_str)); return true; + } else { + return false; } - - return false; case 'r': // put_time uses _Strftime in order to bypass reference-counting that locale uses. This function // takes the locale information by pointer, but the pointer (from _Gettnames) returns a copy. @@ -5545,8 +5545,9 @@ namespace chrono { if constexpr (_Is_specialization_v<_Ty, duration>) { _Os << _STD abs(_Duration_cast_underflow_to_zero(_Val).count()); return true; + } else { + return false; } - return false; case 'q': if constexpr (_Is_specialization_v<_Ty, duration>) { _Write_unit_suffix(_Os); From 12caeeb12428981c6013a38848973fc61d91cc55 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:43:10 -0700 Subject: [PATCH 15/20] chrono cleanups: `_Is_any_of_v` => `is_same_v` for one type. Mentioned in GH 1870, works towards GH 1805. --- stl/inc/chrono | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index e3781291382..ca252badaa4 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -5721,7 +5721,7 @@ namespace chrono { case 'Y': if constexpr (is_same_v<_Ty, year>) { return _Val.ok(); - } else if constexpr (_Is_any_of_v<_Ty, year_month> || _Is_ymd) { + } else if constexpr (is_same_v<_Ty, year_month> || _Is_ymd) { return _Val.year().ok(); } break; From 1fdf9b5e604d61500c59149b8af5e471bf1594d3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:47:41 -0700 Subject: [PATCH 16/20] chrono test cleanups: Drop separate `res` variable used during debugging. Mentioned in GH 1870, works towards GH 1805. --- .../test.cpp | 45 +++++++------------ 1 file changed, 15 insertions(+), 30 deletions(-) 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 29067af6051..e2558200f02 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 @@ -385,51 +385,36 @@ void test_day_formatter() { // 2 digits day d0{27}; - auto res = format(s0, d0); - assert(res == a0); - res = format(s1, d0); - assert(res == a0); + assert(format(s0, d0) == a0); + assert(format(s1, d0) == a0); // 1 digit day d1{5}; - res = format(s0, d1); - assert(res == a1); - res = format(s1, d1); - assert(res == a2); + assert(format(s0, d1) == a1); + assert(format(s1, d1) == a2); // O modifier - res = format(s2, d0); - assert(res == a0); - res = format(s3, d0); - assert(res == a0); - res = format(s2, d1); - assert(res == a1); - res = format(s3, d1); - assert(res == a2); + assert(format(s2, d0) == a0); + assert(format(s3, d0) == a0); + assert(format(s2, d1) == a1); + assert(format(s3, d1) == a2); // [time.format]/6 day d2{50}; - res = format(s4, d0); - assert(res == a0); - res = format(s4, d2); - assert(res == a3); + assert(format(s4, d0) == a0); + assert(format(s4, d2) == a3); // width/align - res = format(s5, d0); - assert(res == a4); - res = format(s5, d1); - assert(res == a5); - res = format(s5, d2); - assert(res == a3); + assert(format(s5, d0) == a4); + assert(format(s5, d1) == a5); + assert(format(s5, d2) == a3); // chrono-spec must begin with conversion-spec throw_helper(s6, d0); // lit chars - res = format(s7, d0); - assert(res == a7); - res = format(s8, d0); - assert(res == a8); + assert(format(s7, d0) == a7); + assert(format(s8, d0) == a8); assert(format(STR("{:%d %d %d}"), day{27}) == STR("27 27 27")); assert(format(STR("{:%d}"), day{200}) == STR("200")); From 5a51fcc179d1266cfc344970938370f6c2ee53f3 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:49:23 -0700 Subject: [PATCH 17/20] chrono test cleanups: Consistently make `invalid` variables `constexpr`. Mentioned in GH 1870, works towards GH 1805. --- .../P0355R7_calendars_and_time_zones_formatting/test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 e2558200f02..bf0baaa112b 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 @@ -469,7 +469,7 @@ void test_year_formatter() { template void test_weekday_formatter() { - weekday invalid{10}; + constexpr weekday invalid{10}; empty_braces_helper(weekday{3}, STR("Wed")); empty_braces_helper(invalid, STR("10 is not a valid weekday")); @@ -607,7 +607,7 @@ void test_year_month_formatter() { template void test_year_month_day_formatter() { - year_month_day invalid{year{1234}, month{0}, day{31}}; + constexpr year_month_day invalid{year{1234}, month{0}, day{31}}; empty_braces_helper(year_month_day{year{1900}, month{2}, day{1}}, STR("1900-02-01")); empty_braces_helper(invalid, STR("1234-00-31 is not a valid date")); From 29195342bd99e884a12f5ad6f24f33c2f20ee2b7 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 20:55:42 -0700 Subject: [PATCH 18/20] chrono cleanups: Drop performance note. I don't believe that this comment is worth keeping. Mentioned in GH 1870, works towards GH 1805. --- stl/inc/chrono | 1 - 1 file changed, 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index ca252badaa4..0926abcf640 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -4787,7 +4787,6 @@ public: } constexpr void _On_conversion_spec(char _Modifier, _CharT _Type) { - // NOTE: same performance note from _Basic_format_specs also applies here if (_Modifier != '\0' && _Modifier != 'E' && _Modifier != 'O') { _Throw_format_error("Invalid modifier specification."); } From cde007a4af8be90d47a47887bcbe08ef80d572c9 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Fri, 20 Oct 2023 23:52:20 -0700 Subject: [PATCH 19/20] chrono cleanups: Comment why `++_Begin` is safe. Mentioned in GH 1870, works towards GH 1805. --- stl/inc/chrono | 2 ++ 1 file changed, 2 insertions(+) diff --git a/stl/inc/chrono b/stl/inc/chrono index 0926abcf640..34894b9e0e7 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -4880,6 +4880,8 @@ _NODISCARD constexpr const _CharT* _Parse_chrono_format_specs( // chrono-spec while (_Begin != _End && *_Begin != '}') { + // Note that in this loop, ++_Begin is safe (and we don't need _Fmt_codec) + // because '%' isn't used as a non-lead-byte in any supported multibyte encoding. if (*_Begin == '%') { // conversion-spec if (++_Begin == _End) { _Throw_format_error("Invalid format string - missing type after %"); From 15223edd87e594b47e7b9a5f0a7a2f3ee4a20485 Mon Sep 17 00:00:00 2001 From: "Stephan T. Lavavej" Date: Sat, 21 Oct 2023 20:10:11 -0700 Subject: [PATCH 20/20] chrono cleanups: Use `ios_base` with `_CATCH_IO_`. The first parameter of `_CATCH_IO_` is used to qualify `badbit`. It's usually `ios_base`, but is allowed to vary for headers that are only dragging in ``. `chrono` has the full definition, so it should just say `ios_base` here. This was the only affected line. Found myself during code review. --- stl/inc/chrono | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stl/inc/chrono b/stl/inc/chrono index 34894b9e0e7..8b730695e63 100644 --- a/stl/inc/chrono +++ b/stl/inc/chrono @@ -4389,7 +4389,7 @@ namespace chrono { } } - _CATCH_IO_(_Myis, _Istr) + _CATCH_IO_(ios_base, _Istr) } if (!_Is_complete()) {