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

Printed seconds precision does not match C++20 spec #2207

Closed
ecorm opened this issue Apr 1, 2021 · 12 comments
Closed

Printed seconds precision does not match C++20 spec #2207

ecorm opened this issue Apr 1, 2021 · 12 comments

Comments

@ecorm
Copy link

ecorm commented Apr 1, 2021

According to https://eel.is/c++draft/time.format#tab:time.format.spec, under the %S specifier:

If the precision of the input cannot be exactly represented with seconds, then the format is a decimal floating-point number with a fixed format and a precision matching that of the precision of the input (or to a microseconds precision if the conversion to floating-point decimal seconds cannot be made within 18 fractional digits).

When outputting a std::chrono::system_clock::time_point (with resolution in nanoseconds) via the %S format specifier, fmt currently only prints the whole seconds, and does not add the decimal fraction according to the C++20 spec.

Godbolt demo: https://godbolt.org/z/h4rfo5zTr

It outputs 01, where it should be 01.234000000 according to the spec.

Using hinnant/date, I get the expected result:

    using time_type = typename std::chrono::system_clock::time_point;
    time_type t{std::chrono::milliseconds{1234}};
    auto s = date::format(std::locale::classic(), "%S", t);
    fmt::print("{}\n", s); // prints 01.234000000

If one wants to print whole seconds with a system_clock::time_point, they first have to floor it to a sys_time<std::chrono::seconds>.

This problem also affects the %T specifier.

@vitaut
Copy link
Contributor

vitaut commented Apr 2, 2021

Thanks for reporting. This is a known limitation of the current implementation relying on conversion to tm which has a seconds resolution.

The solution would be to parse format specs ourselves and handle %S specially. PRs are welcome!

@brcha
Copy link

brcha commented May 18, 2021

I've just wrote & tested this blob down, and it works in C++11 (and above). I could find where something similar could be plugged into fmt. Probably only the subsecond part is needed, since the rest has already be handled by tm.

https://godbolt.org/z/rPv3oK6WE

@brcha
Copy link

brcha commented May 18, 2021

Here's the PR: #2292

@ecorm
Copy link
Author

ecorm commented May 18, 2021

@brcha The precision is supposed to match that of the duration passed in. For milliseconds, it should be 3 decimal places. As per the C++ spec:

If the precision of the input cannot be exactly represented with seconds, then the format is a decimal floating-point number with a fixed format and a precision matching that of the precision of the input (or to a microseconds precision if the conversion to floating-point decimal seconds cannot be made within 18 fractional digits).

It also says this about giving a precision specifier:

Giving a precision specification in the chrono-format-spec is valid only for std​::​chrono​::​duration types where the representation type Rep is a floating-point type. For all other Rep types, an exception of type format_­error is thrown if the chrono-format-spec contains a precision specification.

Hinnant does compile-time log10 calculations in his library to determine the precision when given the period (i.e. ratio) of a duration with integral rep.

@vitaut
Copy link
Contributor

vitaut commented Dec 9, 2021

Thanks to @matrackif subsecond formatting now works for durations (#2623). We still need to do the same for time points so keeping the issue open.

@patrickroocks
Copy link
Contributor

Added a pull request to format time points: #3115

@patrickroocks
Copy link
Contributor

My pull request #3115 was merged now, so subseconds are now formatted for time points.

@vitaut
Copy link
Contributor

vitaut commented Oct 13, 2022

Thanks, @patrickroocks.

@vitaut vitaut closed this as completed Oct 13, 2022
@eli-b
Copy link

eli-b commented Nov 14, 2022

@vitaut, would you consider releasing a new version that includes this important fix?

I'm embarrassed to say how many hours I spent last week failing to output subsecond-precision timestamps. I settled on printing the %T part of my timestamp as a separate tp.time_since_epoch() formatting parameter.

@vitaut
Copy link
Contributor

vitaut commented Nov 15, 2022

I don't think this warrants making a new release but you can pin to a specific commit where this feature is implemented.

@eli-b
Copy link

eli-b commented Nov 15, 2022

I will, thanks.

For the sake of people who search the issues for something like "printing the milliseconds / microseconds part of a chrono time_point doesn't work", I wrote the text in the quotes so this issue would come up in the search.

@vebjornjr
Copy link

Seconding the wish for a new release. We use this lib via vcpkg so pinning to a specific commit is a hassle. Hoping for a small new release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants