<chrono>: Fine grained bounds checking #1871
Conversation
The motivation of this PR is to loosen some specifier constraints by only checking relavent portions of types instead of a blanket `ok()` before attempting to print. This should cover all the specifiers. I've also moved check-only custom prints into this new method.
stl/inc/chrono
Outdated
| if constexpr ( | ||
| _Is_specialization_v<_Ty, duration> | ||
| || is_same_v<_Ty, sys_info> || _Is_specialization_v<_Ty, time_point> | ||
| || _Is_specialization_v<_Ty, _Local_time_format_t>) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Clang format messes this up, but it can be formatted a little more inline (a few clauses got removed over time).
My understanding is that these types are always valid by design, so they just work.
| if constexpr (_Is_specialization_v<_Ty, hh_mm_ss>) { | ||
| if (_Spec._Type == 'H' && _Val.hours() >= hours{24}) { | ||
| _THROW(format_error("Cannot localize hh_mm_ss with an absolute value of 24 hours or more.")); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Apart from H, my understanding is that hh_mm_ss is always valid (for it's specifiers).
| case 'a': | ||
| case 'A': | ||
| case 'u': | ||
| case 'w': | ||
| if constexpr (_Is_any_of_v<_Ty, weekday, weekday_last>) { | ||
| return _Val.ok(); | ||
| } else if constexpr (_Is_any_of_v<_Ty, weekday_indexed, year_month_weekday, | ||
| year_month_weekday_last>) { | ||
| return _Val.weekday().ok(); | ||
| } else if constexpr (is_same_v<_Ty, month_weekday>) { | ||
| return _Val.weekday_indexed().weekday().ok(); | ||
| } else if constexpr (is_same_v<_Ty, month_weekday_last>) { | ||
| return _Val.weekday_last().ok(); | ||
| } else if constexpr (_Is_any_of_v<_Ty, year_month_day, year_month_day_last>) { | ||
| return _Val.ok(); | ||
| } | ||
| break; |
There was a problem hiding this comment.
By far the finickiest specifier, because almost everything can go wrong and you can still get a weekday.
| } | ||
| return true; | ||
| } | ||
| _STL_INTERNAL_CHECK(false); |
There was a problem hiding this comment.
The idea is that if a specifier is being checked manually, we should check it for every type that is valid for that specifier (apart from the always correct types at the top). This way, if we miss specific handling for a type-specifier combo, we can catch it internally.
Add newline between non-chained if-statements. Clarify comments. _Validate_specifiers can be static. In test_month_day_formatter, verify that a month_day with a bogus month but valid day can't be printed as a month.
mnatsuhara
left a comment
mnatsuhara
left a comment
There was a problem hiding this comment.
Feel free to ignore the nitpick, but the question about the year_month_day and friends' ok()ness remains...
Co-authored-by: mnatsuhara <46756417+mnatsuhara@users.noreply.github.com>
3 participants
The motivation of this PR is to loosen some specifier constraints by
only checking relavent portions of types instead of a blanket
ok()before attempting to print. This should cover all the specifiers.
I've also moved check-only custom prints into this new method.