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.

Sign up for GitHub

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

Jump to bottom

LWG-3554 Add const charT* overloads for chrono::parse #2261

Merged
merged 6 commits into from
Nov 3, 2021
Merged

LWG-3554 Add const charT* overloads for chrono::parse #2261

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9d12685
Implement LWG-3554
cpplearner Oct 6, 2021
cf46939
Avoid ABI break
cpplearner Oct 6, 2021
58af915
Address review comments from MattStephanson
cpplearner Oct 7, 2021
40ec532
Apply suggestions from code review
cpplearner Oct 27, 2021
6f71e78
call `from_stream` unqualified
cpplearner Oct 27, 2021
a27e977
Address review comments from StephanTLavavej
cpplearner Oct 28, 2021
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
71 changes: 58 additions & 13 deletions stl/inc/chrono
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5136,55 +5136,100 @@ namespace chrono {
return _Istr;
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable>
struct _Time_parse_iomanip_c_str {
_Time_parse_iomanip_c_str(const _CharT* _Fmt_, _Parsable& _Tp_,
basic_string<_CharT, _Traits, _Alloc>* _Abbrev_ = nullptr, minutes* _Offset_ = nullptr)
: _Fmt{_Fmt_}, _Tp{_Tp_}, _Abbrev{_Abbrev_}, _Offset{_Offset_} {}

_Time_parse_iomanip_c_str(_Time_parse_iomanip_c_str&&) = delete;

const _CharT* _Fmt;
_Parsable& _Tp;
basic_string<_CharT, _Traits, _Alloc>* _Abbrev;
minutes* _Offset;
};

template <class _CharT, class _Traits, class _Alloc, class _Parsable>
struct _Time_parse_iomanip {
Copy link
Contributor

@MattStephanson MattStephanson Oct 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a // TRANSITION, ABI comment to the effect that this can be replaced with _Time_parse_iomanip_c_str in the future?

Edit: this is showing more context than I intended. I was referring specifically to the struct _Time_parse_iomanip { line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The small code duplication is not bad enough to worry me, personally. Storing a reference to basic_string also has a tiny benefit, that is the format string is not invalidated if the basic_string is resized after the call to parse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the manipulator is supposed to live long enought for the format string to be resized, but fair enough regarding the amount of code duplication.

_Time_parse_iomanip(const basic_string<_CharT, _Traits, _Alloc>& _Fmt_, _Parsable& _Tp_,
basic_string<_CharT, _Traits, _Alloc>* _Abbrev_ = nullptr, minutes* _Offset_ = nullptr)
: _Fmt{_Fmt_}, _Tp{_Tp_}, _Abbrev{_Abbrev_}, _Offset{_Offset_} {}

_Time_parse_iomanip(_Time_parse_iomanip&&) = delete;

const basic_string<_CharT, _Traits, _Alloc>& _Fmt;
_Parsable& _Tp;
basic_string<_CharT, _Traits, _Alloc>* _Abbrev;
minutes* _Offset;
};

template <class _CharT, class _Traits, class _Parsable, class... _Rest>
using _Has_from_stream = void_t<decltype(from_stream(_STD declval<basic_istream<_CharT, _Traits>&>(),
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
_STD declval<const _CharT*>(), _STD declval<_Parsable&>(), _STD declval<_Rest>()...))>;

template <class _CharT, class _Parsable, class = _Has_from_stream<_CharT, char_traits<_CharT>, _Parsable>>
_NODISCARD auto parse(const _CharT* _Fmt, _Parsable& _Tp) {
return _Time_parse_iomanip_c_str<_CharT, char_traits<_CharT>, allocator<_CharT>, _Parsable>{_Fmt, _Tp};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable,
class = void_t<decltype(_CHRONO from_stream(_STD declval<basic_istream<_CharT, _Traits>&>(),
_STD declval<const _CharT*>(), _STD declval<_Parsable&>()))>>
class = _Has_from_stream<_CharT, _Traits, _Parsable>>
_NODISCARD auto parse(const basic_string<_CharT, _Traits, _Alloc>& _Fmt, _Parsable& _Tp) {
return _Time_parse_iomanip{_Fmt, _Tp};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable,
class = void_t<decltype(_CHRONO from_stream(_STD declval<basic_istream<_CharT, _Traits>&>(),
_STD declval<const _CharT*>(), _STD declval<_Parsable&>(),
_STD declval<basic_string<_CharT, _Traits, _Alloc>*>()))>>
class = _Has_from_stream<_CharT, _Traits, _Parsable, basic_string<_CharT, _Traits, _Alloc>*>>
_NODISCARD auto parse(const _CharT* _Fmt, _Parsable& _Tp, basic_string<_CharT, _Traits, _Alloc>& _Abbrev) {
return _Time_parse_iomanip_c_str{_Fmt, _Tp, _STD addressof(_Abbrev)};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable,
class = _Has_from_stream<_CharT, _Traits, _Parsable, basic_string<_CharT, _Traits, _Alloc>*>>
_NODISCARD auto parse(const basic_string<_CharT, _Traits, _Alloc>& _Fmt, _Parsable& _Tp,
basic_string<_CharT, _Traits, _Alloc>& _Abbrev) {
return _Time_parse_iomanip{_Fmt, _Tp, _STD addressof(_Abbrev)};
}

template <class _CharT, class _Parsable,
class = _Has_from_stream<_CharT, char_traits<_CharT>, _Parsable, basic_string<_CharT>*, minutes*>>
_NODISCARD auto parse(const _CharT* _Fmt, _Parsable& _Tp, minutes& _Offset) {
return _Time_parse_iomanip_c_str{_Fmt, _Tp, static_cast<basic_string<_CharT>*>(nullptr), &_Offset};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable,
class = void_t<decltype(_CHRONO from_stream(_STD declval<basic_istream<_CharT, _Traits>&>(),
_STD declval<const _CharT*>(), _STD declval<_Parsable&>(),
_STD declval<basic_string<_CharT, _Traits, _Alloc>*>(), _STD declval<minutes*>()))>>
class = _Has_from_stream<_CharT, _Traits, _Parsable, basic_string<_CharT, _Traits, _Alloc>*, minutes*>>
_NODISCARD auto parse(const basic_string<_CharT, _Traits, _Alloc>& _Fmt, _Parsable& _Tp, minutes& _Offset) {
return _Time_parse_iomanip{_Fmt, _Tp, static_cast<basic_string<_CharT, _Traits, _Alloc>*>(nullptr), &_Offset};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable,
class = void_t<decltype(_CHRONO from_stream(_STD declval<basic_istream<_CharT, _Traits>&>(),
_STD declval<const _CharT*>(), _STD declval<_Parsable&>(),
_STD declval<basic_string<_CharT, _Traits, _Alloc>*>(), _STD declval<minutes*>()))>>
class = _Has_from_stream<_CharT, _Traits, _Parsable, basic_string<_CharT, _Traits, _Alloc>*, minutes*>>
_NODISCARD auto parse(
const _CharT* _Fmt, _Parsable& _Tp, basic_string<_CharT, _Traits, _Alloc>& _Abbrev, minutes& _Offset) {
return _Time_parse_iomanip_c_str{_Fmt, _Tp, _STD addressof(_Abbrev), &_Offset};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable,
class = _Has_from_stream<_CharT, _Traits, _Parsable, basic_string<_CharT, _Traits, _Alloc>*, minutes*>>
_NODISCARD auto parse(const basic_string<_CharT, _Traits, _Alloc>& _Fmt, _Parsable& _Tp,
basic_string<_CharT, _Traits, _Alloc>& _Abbrev, minutes& _Offset) {
return _Time_parse_iomanip{_Fmt, _Tp, _STD addressof(_Abbrev), &_Offset};
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable>
basic_istream<_CharT, _Traits>& operator>>(
basic_istream<_CharT, _Traits>& _Is, const _Time_parse_iomanip<_CharT, _Traits, _Alloc, _Parsable>& _Tpi) {
return _CHRONO from_stream(_Is, _Tpi._Fmt.c_str(), _Tpi._Tp, _Tpi._Abbrev, _Tpi._Offset);
basic_istream<_CharT, _Traits>& _Is, _Time_parse_iomanip_c_str<_CharT, _Traits, _Alloc, _Parsable>&& _Tpi) {
from_stream(_Is, _Tpi._Fmt, _Tpi._Tp, _Tpi._Abbrev, _Tpi._Offset); // intentional ADL
return _Is;
}

template <class _CharT, class _Traits, class _Alloc, class _Parsable>
basic_istream<_CharT, _Traits>& operator>>(
basic_istream<_CharT, _Traits>& _Is, _Time_parse_iomanip<_CharT, _Traits, _Alloc, _Parsable>&& _Tpi) {
from_stream(_Is, _Tpi._Fmt.c_str(), _Tpi._Tp, _Tpi._Abbrev, _Tpi._Offset); // intentional ADL
return _Is;
}

#endif // _HAS_CXX20
Expand Down
73 changes: 37 additions & 36 deletions tests/std/tests/P0355R7_calendars_and_time_zones_io/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ bool test_duration_basic_out(const duration<Rep, Period>& d, const CharT* expect
return ss.str() == expected;
}

#define WIDEN(TYPE, STR) get<const TYPE*>(pair{STR, L##STR});
#define WIDEN(TYPE, STR) get<const TYPE*>(pair{STR, L##STR})

template <class CharT>
bool test_duration_locale_out() {
Expand Down Expand Up @@ -64,8 +64,6 @@ bool test_duration_locale_out() {
return ss.str() == expected;
}

#undef WIDEN

void test_duration_output() {
using LongRatio = ratio<INTMAX_MAX - 1, INTMAX_MAX>;
assert(test_duration_basic_out(duration<int, atto>{1}, "1as"));
Expand Down Expand Up @@ -128,9 +126,9 @@ void test_duration_output() {
}


template <class CharT, class Parsable>
void test_parse(const CharT* str, const CharT* fmt, Parsable& p, type_identity_t<basic_string<CharT>*> abbrev = nullptr,
minutes* offset = nullptr) {
template <class CharT, class CStringOrStdString, class Parsable>
void test_parse(const CharT* str, CStringOrStdString fmt, Parsable& p,
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
type_identity_t<basic_string<CharT>*> abbrev = nullptr, minutes* offset = nullptr) {
p = Parsable{};
if (abbrev) {
if constexpr (is_same_v<CharT, char>) {
Expand All @@ -147,24 +145,24 @@ void test_parse(const CharT* str, const CharT* fmt, Parsable& p, type_identity_t
basic_stringstream<CharT> sstr{str};
if (abbrev) {
if (offset) {
sstr >> parse(basic_string<CharT>{fmt}, p, *abbrev, *offset);
sstr >> parse(fmt, p, *abbrev, *offset);
cpplearner marked this conversation as resolved.
Show resolved Hide resolved
} else {
sstr >> parse(basic_string<CharT>{fmt}, p, *abbrev);
sstr >> parse(fmt, p, *abbrev);
}
} else {
if (offset) {
sstr >> parse(basic_string<CharT>{fmt}, p, *offset);
sstr >> parse(fmt, p, *offset);
} else {
sstr >> parse(basic_string<CharT>{fmt}, p);
sstr >> parse(fmt, p);
}
}

assert(sstr);
}

template <class CharT, class Parsable>
void fail_parse(const CharT* str, const CharT* fmt, Parsable& p, type_identity_t<basic_string<CharT>*> abbrev = nullptr,
minutes* offset = nullptr) {
template <class CharT, class CStringOrStdString, class Parsable>
void fail_parse(const CharT* str, CStringOrStdString fmt, Parsable& p,
type_identity_t<basic_string<CharT>*> abbrev = nullptr, minutes* offset = nullptr) {
p = Parsable{};
if (abbrev) {
if constexpr (is_same_v<CharT, char>) {
Expand All @@ -181,15 +179,15 @@ void fail_parse(const CharT* str, const CharT* fmt, Parsable& p, type_identity_t
basic_stringstream<CharT> sstr{str};
if (abbrev) {
if (offset) {
sstr >> parse(basic_string<CharT>{fmt}, p, *abbrev, *offset);
sstr >> parse(fmt, p, *abbrev, *offset);
} else {
sstr >> parse(basic_string<CharT>{fmt}, p, *abbrev);
sstr >> parse(fmt, p, *abbrev);
}
} else {
if (offset) {
sstr >> parse(basic_string<CharT>{fmt}, p, *offset);
sstr >> parse(fmt, p, *offset);
} else {
sstr >> parse(basic_string<CharT>{fmt}, p);
sstr >> parse(fmt, p);
}
}

Expand Down Expand Up @@ -227,19 +225,19 @@ void test_lwg_3536() {

{
istringstream iss{"2:2:30"};
iss >> parse(string{"%H:%M:%S"}, mm);
iss >> parse("%H:%M:%S", mm);
assert(iss.fail() && mm == 20min);
}

{
istringstream iss{"June"};
iss >> parse(string{"%B"}, mm);
iss >> parse("%B", mm);
assert(iss.fail() && mm == 20min);
}

{
istringstream iss{""};
iss >> parse(string{"%B"}, mm);
iss >> parse("%B", mm);
assert(iss.fail() && mm == 20min);
}
}
Expand Down Expand Up @@ -1172,36 +1170,37 @@ void parse_timepoints() {
test_gh_1952();
}

void parse_wchar() {
template <class CharT, class CStringOrStdString>
void test_wchar_and_lwg_3554() {
seconds time;
test_parse(L"12", L"%S", time);
test_parse(WIDEN(CharT, "12"), CStringOrStdString{WIDEN(CharT, "%S")}, time);
assert(time == 12s);
test_parse(L"12", L"%M", time);
test_parse(WIDEN(CharT, "12"), CStringOrStdString{WIDEN(CharT, "%M")}, time);
assert(time == 12min);
test_parse(L"30", L"%H", time);
test_parse(WIDEN(CharT, "30"), CStringOrStdString{WIDEN(CharT, "%H")}, time);
assert(time == 30h);
test_parse(L" 1:23:42", L"%T", time);
test_parse(WIDEN(CharT, " 1:23:42"), CStringOrStdString{WIDEN(CharT, "%T")}, time);
assert(time == 1h + 23min + 42s);
wstring tz_name;
test_parse(L"Etc/GMT+11", L"%Z", time, &tz_name);
assert(tz_name == L"Etc/GMT+11");
fail_parse(L"Not_valid! 00", L"%Z %H", time, &tz_name);
basic_string<CharT> tz_name;
test_parse(WIDEN(CharT, "Etc/GMT+11"), CStringOrStdString{WIDEN(CharT, "%Z")}, time, &tz_name);
assert(tz_name == WIDEN(CharT, "Etc/GMT+11"));
fail_parse(WIDEN(CharT, "Not_valid! 00"), CStringOrStdString{WIDEN(CharT, "%Z %H")}, time, &tz_name);

weekday wd;
test_parse(L"wedNesday", L"%A", wd);
test_parse(WIDEN(CharT, "wedNesday"), CStringOrStdString{WIDEN(CharT, "%A")}, wd);
assert(wd == Wednesday);

month m;
test_parse(L"deCeMbeR", L"%b", m);
test_parse(WIDEN(CharT, "deCeMbeR"), CStringOrStdString{WIDEN(CharT, "%b")}, m);
assert(m == December);

sys_seconds st;
test_parse(L"oct 29 19:01:42 2020", L"%c", st);
test_parse(WIDEN(CharT, "oct 29 19:01:42 2020"), CStringOrStdString{WIDEN(CharT, "%c")}, st);
assert(st == sys_days{2020y / October / 29d} + 19h + 1min + 42s);

fail_parse(L"ab", L"a%nb", time);
test_parse(L"a b", L"a%nb", time);
fail_parse(L"a b", L"a%nb", time);
fail_parse(WIDEN(CharT, "ab"), CStringOrStdString{WIDEN(CharT, "a%nb")}, time);
test_parse(WIDEN(CharT, "a b"), CStringOrStdString{WIDEN(CharT, "a%nb")}, time);
fail_parse(WIDEN(CharT, "a b"), CStringOrStdString{WIDEN(CharT, "a%nb")}, time);
}

void test_parse() {
Expand All @@ -1216,7 +1215,9 @@ void test_parse() {
parse_other_week_date();
parse_whitespace();
parse_timepoints();
parse_wchar();
test_wchar_and_lwg_3554<wchar_t, const wchar_t*>();
test_wchar_and_lwg_3554<char, string>();
test_wchar_and_lwg_3554<wchar_t, wstring>();
StephanTLavavej marked this conversation as resolved.
Show resolved Hide resolved
}

void test() {
Expand Down