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

chronat: Add year and year_month_day formatting #1840

Merged
merged 7 commits into from
Apr 15, 2021
Merged

chronat: Add year and year_month_day formatting #1840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e6708c5
<chronat>: Add year and year_month_day formatting
eldakesh-ms Apr 15, 2021
8ec251d
Clang test
eldakesh-ms Apr 15, 2021
84e7c61
Finesse the tests
eldakesh-ms Apr 15, 2021
99f656d
review
eldakesh-ms Apr 15, 2021
8d47d50
Comment update
eldakesh-ms Apr 15, 2021
5722065
PR fixes
eldakesh-ms Apr 15, 2021
c89a21e
Tests
eldakesh-ms Apr 15, 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
184 changes: 166 additions & 18 deletions stl/inc/chrono
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5408,6 +5408,62 @@ _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".
template <class _CharT>
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':
if (_Time.tm_mday < 10) {
_Os << (_Type == 'd' ? _CharT{'0'} : _CharT{' '});
}
_Os << _Time.tm_mday;
return true;
case 'm':
if (_Month < 10) {
_Os << _CharT{'0'};
}
_Os << _Month;
return true;
case 'Y':
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}"), _Year < 0 ? 100 + (_Year % 100) : _Year % 100);
return true;
case 'C':
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);
_Os << _CharT{'-'};
_Try_simple_write(_Os, 'm', _Time);
_Os << _CharT{'-'};
_Try_simple_write(_Os, 'd', _Time);
return true;
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;
}
}

/*
template <class _CharT, class _Traits, class _Duration>
// clang-format off
Expand All @@ -5431,10 +5487,8 @@ namespace chrono {

template <class _CharT, class _Traits>
basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const day& _Val) {
if (static_cast<unsigned int>(_Val) < 10) {
_Os << _CharT{'0'};
}
_Os << static_cast<unsigned int>(_Val);
const tm _Time = {.tm_mday = static_cast<int>(static_cast<unsigned int>(_Val))};
_Try_simple_write(_Os, 'd', _Time);
if (!_Val.ok()) {
_Os << _STATICALLY_WIDEN(_CharT, " is not a valid day");
}
Expand All @@ -5451,6 +5505,28 @@ namespace chrono {
}
return _Os << static_cast<unsigned int>(_Val) << _STATICALLY_WIDEN(_CharT, " is not a valid month");
}

template <class _CharT, class _Traits>
basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const year& _Val) {
const tm _Time = {.tm_year = static_cast<int>(_Val) - 1900};
_Try_simple_write(_Os, 'Y', _Time);
if (!_Val.ok()) {
_Os << _STATICALLY_WIDEN(_CharT, " is not a valid year");
}
return _Os;
}

template <class _CharT, class _Traits>
basic_ostream<_CharT, _Traits>& operator<<(basic_ostream<_CharT, _Traits>& _Os, const year_month_day& _Val) {
const tm _Time = {.tm_mday = static_cast<int>(static_cast<unsigned int>(_Val.day())),
.tm_mon = static_cast<int>(static_cast<unsigned int>(_Val.month())) - 1,
.tm_year = static_cast<int>(_Val.year()) - 1900};
_Try_simple_write(_Os, 'F', _Time);
if (!_Val.ok()) {
_Os << _STATICALLY_WIDEN(_CharT, " is not a valid date");
}
return _Os;
}
} // namespace chrono

/*
Expand All @@ -5472,7 +5548,8 @@ struct formatter<_CHRONO sys_time<_Duration>, _CharT> {

template <class _CharT, bool _Allow_precision>
struct _Chrono_formatter {
auto _Parse(basic_format_parse_context<_CharT>& _Parse_ctx, string_view _Valid_types) {
template <class _Ty>
_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);
Expand All @@ -5495,7 +5572,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_valid_type<_Ty>(_Spec._Type)) {
_THROW(format_error("Invalid type."));
}
_Check_modifier(_Spec._Type, _Spec._Modifier);
Expand All @@ -5516,7 +5593,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;

Expand All @@ -5529,8 +5607,25 @@ struct _Chrono_formatter {
_THROW(format_error("Incompatible modifier for type"));
}

template <class _Ty>
_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>) {
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_valid_type<_CHRONO year>(_Type)
|| _Is_valid_type<_CHRONO month>(_Type) || _Is_valid_type<_CHRONO day>(_Type);
Comment on lines +5619 to +5620
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if the specifiers that are valid for weekday are also valid here since a specific year, month, and weekday would give us a particular weekday. Similarly, would 'U' and 'V' be valid specifiers since we could get a particular week from a year_month_day?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not have a weekday. I based which specifiers are legal from put_time, and since it isn't trivial to compute the weekday from just a year/month/day, I think it doesn't apply. I could be very wrong though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After discussing on discord, I agree it makes sense to be able to derive some more information from a year_month_day than I originally implemented.

} else {
// TRANSITION, remove when all types are added
static_assert(_Always_false<_Ty>, "unsupported type");
}
}

template <class _FormatContext, class _Ty>
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) {
Expand All @@ -5543,10 +5638,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 (_Type_needs_bounds_checking(_Spec)) {
if (!_Val.ok()) {
_THROW(format_error("Cannot localize out-of-bounds time point."));
}
} else if (_Spec._Modifier == '\0' && _CHRONO _Try_simple_write(_Stream, _Spec._Type, _Time)) {
continue;
}

_CharT _Fmt_str[4];
Expand All @@ -5566,18 +5663,35 @@ struct _Chrono_formatter {
_Fmt_align::_Left, [&](auto _Out) { return _Fmt_write(_STD move(_Out), _Stream.view()); });
}

_NODISCARD bool _Type_needs_bounds_checking(const _Chrono_specs<_CharT>& _Spec) noexcept {
// [tab:time.format.spec]
switch (_Spec._Type) {
case 'a':
case 'A':
case 'b':
case 'B':
case 'h':
case 'z':
case 'Z':
Comment on lines +5669 to +5675
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to just populate this with all of the type specifiers that will end up here? Or were you planning to add to this as we added formatters/operator<<s for each type? I think we'll also need to include %c, %x, %X looking at Table 101.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I think we need to rename this function. It was conceived as a way to figure out specifiers can throw, so we can explicit about ok() checking. %c uses locale, but I don't think it can ever be out of bounds. Maybe _Type_needs_bounds_checking?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think %c will actually require more extensive bounds-checking because put_time [says that %c uses all fields of tm so there is even more opportunity for out-of-bounds issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to say that %c is used for a point in time, measured from some base. You can't have an invalid seconds, because you derive the seconds from the amount of time elapsed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm trying to work out what type would be used to format %c and I think it would probably be time_points right? (e.g., sys_time, etc.). If we would just use time_point::time_since_epoch() then I think you're right.

return true;

default:
return false;
}
}

_Chrono_format_specs<_CharT> _Specs{};
bool _No_chrono_specs = false;
};

template <class _CharT>
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 <class _FormatContext>
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<int>(static_cast<unsigned int>(_Val))};
return _Impl._Write(_FormatCtx, _Val, _Time);
}
Expand All @@ -5588,17 +5702,51 @@ private:

template <class _CharT>
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 <class _FormatContext>
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<int>(static_cast<unsigned int>(_Val)) - 1};
return _Impl._Write(_FormatCtx, _Val, _Time);
}

private:
_Chrono_formatter<_CharT, false> _Impl;
};

template <class _CharT>
struct formatter<_CHRONO year, _CharT> {
auto parse(basic_format_parse_context<_CharT>& _Parse_ctx) {
return _Impl.template _Parse<_CHRONO year>(_Parse_ctx);
}

template <class _FormatContext>
auto format(const _CHRONO year& _Val, _FormatContext& _FormatCtx) {
const tm _Time = {.tm_year = static_cast<int>(_Val) - 1900};
return _Impl._Write(_FormatCtx, _Val, _Time);
}

private:
_Chrono_formatter<_CharT, false> _Impl;
};

template <class _CharT>
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 <class _FormatContext>
auto format(const _CHRONO year_month_day& _Val, _FormatContext& _FormatCtx) {
const tm _Time = {.tm_mday = static_cast<int>(static_cast<unsigned int>(_Val.day())),
.tm_mon = static_cast<int>(static_cast<unsigned int>(_Val.month())) - 1,
.tm_year = static_cast<int>(_Val.year()) - 1900};
return _Impl._Write(_FormatCtx, _Val, _Time);
}

private:
_Chrono_formatter<_CharT, false> _Impl;
};
Expand Down
6 changes: 3 additions & 3 deletions tests/libcxx/expected_results.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +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

Expand Down Expand Up @@ -882,3 +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 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
3 changes: 0 additions & 3 deletions tests/libcxx/skipped_tests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,10 +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

Expand Down
38 changes: 35 additions & 3 deletions tests/std/tests/P0355R7_calendars_and_time_zones_formatting/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"));

Expand All @@ -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});
Expand All @@ -332,6 +332,32 @@ void test_month_formatter() {
stream_helper(STR("20 is not a valid month"), month{20});
}

template <typename CharT>
void test_year_formatter() {
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{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{1900});
stream_helper(STR("2000"), year{2000});
stream_helper(STR("-32768 is not a valid year"), year{-32768});
}

template <typename CharT>
void test_year_month_day_formatter() {
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{1234}, month{5}, day{6}}) == STR("1234 May 06"));
assert(format(STR("{:%F %D}"), invalid) == STR("1234-00-31 00/31/34"));
}

int main() {
test_parse_conversion_spec<char>();
test_parse_conversion_spec<wchar_t>();
Expand All @@ -344,4 +370,10 @@ int main() {

test_month_formatter<char>();
test_month_formatter<wchar_t>();

test_year_formatter<char>();
test_year_formatter<wchar_t>();

test_year_month_day_formatter<char>();
test_year_month_day_formatter<wchar_t>();
}