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.

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

<chrono> formatting: fix UB, various cleanups #1848

Merged
merged 2 commits into from
Apr 17, 2021

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented Apr 17, 2021

  • Adjust spacing in _Has_ok.
  • Avoid undefined behavior 🙀 when accessing _Last_day_table. _Month could be bogus, so we need to mask the value.
  • Remove unnecessary parentheses.
  • Instead of telling _Chrono_formatter whether to _Allow_precision, teach its _Parse to determine that by inspecting _Ty. This avoids potential mistakes, and allows us to customize the format_error message. (We need separate cases for integral-duration and non-duration, due to the need to inspect typename _Ty::rep.)
  • Add a constructor to _Chrono_formatter so we can provide a time zone abbreviation, instead of setting it later. This slightly reduces verbosity in the formatters below. (Eventually, _Chrono_formatter could probably lock down its access control instead of having public data members, but I'm not changing that here.)
  • Add newlines between non-chained if-statements.
  • Style: Rearrange the hh_mm_ss range check to follow the number line (most negative on the left).
  • Rephrase the hh_mm_ss format_error: exact -24h and 24h are errors.
  • Provide a data member initializer for _Time_zone_abbreviation{};. This isn't necessary, but makes it clear that we haven't forgotten about it.
  • Use constexpr offsets when formatting tai_time and gps_time.
    • This follows the Standard's depicted code (except that it avoids UDLs, which are defined at the bottom of <chrono> and I didn't want to bother with moving them).
    • I believe this is slightly more readable, and it's more efficient for debug codegen: the offset is computed at compile time with no function calls.

@StephanTLavavej StephanTLavavej added bug Something isn't working chrono C++20 chrono labels Apr 17, 2021
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner April 17, 2021 02:15
Co-authored-by: MattStephanson <68978048+MattStephanson@users.noreply.github.com>
@StephanTLavavej StephanTLavavej merged commit 6c06f23 into microsoft:chronat2 Apr 17, 2021
@StephanTLavavej StephanTLavavej deleted the chronat2_fix_ub branch April 17, 2021 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chrono C++20 chrono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants