-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix chrono printing #2814
Fix chrono printing #2814
Conversation
{} is specified to be equivalent to formatting using iostreams <<, but for very small durations we would fail to compile such code because of conversions to longer durations failing (since we form the hh_mm_ss structure for all formats, even though in this case it isn't used. This change should make some of the specifiers that _do_ use that structure work as well (for example {:%H}, attoseconds). This problem occurs for any chrono conversion where the source type can not represent even one period of the target type.
7a18f08
to
c27cb81
Compare
…pecifier with time-points, as well as raw durations.
This comment was marked as resolved.
This comment was marked as resolved.
Thanks, this looks good! I verified that the underflow check is correctly using the GCDs to reduce. I validated and pushed a conflict-free merge with |
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Thanks for fixing these user-reported compiler errors! ⏱️ 💥 😺 |
…soft#2814) Co-authored-by: Stephan T. Lavavej <stl@nuwen.net>
There may be more cases of this behavior that are not fixed here, but this should improve things for basic formatting with {}.
There's a bunch of standardize that appears unfortunate when considering types that "underflow" in this manner, the wording tries to make sure you don't truncate when converting from large periods to tiny ones, but the reverse is also prevented, even though it's not a big deal and is similar to the "rounding" that already happens when converting from say, seconds, to days with duration_cast.
For example, chrono::ceil and chrono::floor makes sense in this case.
Fixes VSO-1521926/AB#1521926.