public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds
@ 2024-03-05 17:23 howard.hinnant at gmail dot com
2024-03-05 20:49 ` [Bug libstdc++/114244] " redi at gcc dot gnu.org
` (7 more replies)
0 siblings, 8 replies; 9+ messages in thread
From: howard.hinnant at gmail dot com @ 2024-03-05 17:23 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
Bug ID: 114244
Summary: Need to use round when parsing fractional seconds
Product: gcc
Version: 14.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: howard.hinnant at gmail dot com
Target Milestone: ---
I'm not sure if this is a bug or just QOI. But consider:
#include <chrono>
#include <string>
#include <iostream>
using time_point_t = std::chrono::sys_time<std::chrono::milliseconds>;
time_point_t decrypt(std::string s) {
time_point_t tp;
std::istringstream in(s);
in >> parse("%Y%m%d-%T", tp);
return tp;
}
int main() {
std::cout << decrypt("20240304-13:00:00.002");
}
I expect the output to be:
2024-03-04 13:00:00.002
but it is:
2024-03-04 13:00:00.001
I believe the issue is the use of duration_cast<milliseconds> or
time_point_cast<milliseconds> somewhere under the parse (not sure exactly
where). Since 0.002 is not exactly representable in floating point, we're
getting a truncate towards zero because this parses as just barely under 0.002.
I highly recommend the use of round<Duration> when converting floating point
based durations and time_points to integral based.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
@ 2024-03-05 20:49 ` redi at gcc dot gnu.org
2024-03-07 0:30 ` redi at gcc dot gnu.org
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: redi at gcc dot gnu.org @ 2024-03-05 20:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Ever confirmed|0 |1
Status|UNCONFIRMED |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |redi at gcc dot gnu.org
Last reconfirmed| |2024-03-05
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
2024-03-05 20:49 ` [Bug libstdc++/114244] " redi at gcc dot gnu.org
@ 2024-03-07 0:30 ` redi at gcc dot gnu.org
2024-03-07 0:32 ` redi at gcc dot gnu.org
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: redi at gcc dot gnu.org @ 2024-03-07 0:30 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yup, the seconds part "00.002" is parsed using std::numpunct (in order to
handle the locale's decimal point) and then converted to milliseconds using
duration_cast:
auto& __ng = use_facet<num_get<_CharT>>(__loc);
long double __val;
ios_base::iostate __err2{};
__ng.get(__buf, {}, __buf, __err2, __val);
if (__is_failed(__err2)) [[unlikely]]
__err |= __err2;
else
{
duration<long double> __fs(__val);
__s = duration_cast<_Duration>(__fs);
}
We also use duration_cast when parsing %c and %X using std::time_get, but
that's an integer conversion there (but the duration_cast should have been
qualified to prevent ADL):
__h = hours(__tm.tm_hour);
__min = minutes(__tm.tm_min);
__s = duration_cast<_Duration>(seconds(__tm.tm_sec));
and another duration_cast in chrono::from_stream for durations. That one could
be used with either integral or floating-point reps.
Do we want to parse "00:00:31" as minutes(1)? I don't think we do, so only the
first case where converting long double to milliseconds should be rounded?
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
2024-03-05 20:49 ` [Bug libstdc++/114244] " redi at gcc dot gnu.org
2024-03-07 0:30 ` redi at gcc dot gnu.org
@ 2024-03-07 0:32 ` redi at gcc dot gnu.org
2024-03-07 0:37 ` redi at gcc dot gnu.org
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: redi at gcc dot gnu.org @ 2024-03-07 0:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> and another duration_cast in chrono::from_stream for durations. That one
> could be used with either integral or floating-point reps.
Ah, but we're converting from common_type_t<Duration, seconds> to Duration, so
either it's float-to-float or integer-to-integer, and duration_cast is OK if
we're fine with truncating.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
` (2 preceding siblings ...)
2024-03-07 0:32 ` redi at gcc dot gnu.org
@ 2024-03-07 0:37 ` redi at gcc dot gnu.org
2024-03-07 1:01 ` howard.hinnant at gmail dot com
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: redi at gcc dot gnu.org @ 2024-03-07 0:37 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #1)
> Yup, the seconds part "00.002" is parsed using std::numpunct (in order to
Oops, std::num_get obviously.
> handle the locale's decimal point) and then converted to milliseconds using
> duration_cast:
>
> auto& __ng = use_facet<num_get<_CharT>>(__loc);
> long double __val;
> ios_base::iostate __err2{};
> __ng.get(__buf, {}, __buf, __err2, __val);
> if (__is_failed(__err2)) [[unlikely]]
> __err |= __err2;
> else
> {
> duration<long double> __fs(__val);
> __s = duration_cast<_Duration>(__fs);
> }
As an unrelated optimization, when the locale is "C" we could use
std::from_chars or std::stod instead of std::num_get.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
` (3 preceding siblings ...)
2024-03-07 0:37 ` redi at gcc dot gnu.org
@ 2024-03-07 1:01 ` howard.hinnant at gmail dot com
2024-03-07 1:21 ` redi at gcc dot gnu.org
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: howard.hinnant at gmail dot com @ 2024-03-07 1:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
--- Comment #4 from Howard Hinnant <howard.hinnant at gmail dot com> ---
Not positive (because I don't know your code that well), but I think:
__s = round<_Duration>(__fs);
will do it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
` (4 preceding siblings ...)
2024-03-07 1:01 ` howard.hinnant at gmail dot com
@ 2024-03-07 1:21 ` redi at gcc dot gnu.org
2024-03-07 23:45 ` cvs-commit at gcc dot gnu.org
2024-03-07 23:46 ` redi at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: redi at gcc dot gnu.org @ 2024-03-07 1:21 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yup, that's what I have in my local tree now.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
` (5 preceding siblings ...)
2024-03-07 1:21 ` redi at gcc dot gnu.org
@ 2024-03-07 23:45 ` cvs-commit at gcc dot gnu.org
2024-03-07 23:46 ` redi at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-03-07 23:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jonathan Wakely <redi@gcc.gnu.org>:
https://gcc.gnu.org/g:5f9d7a5b6cf64639274e63051caf70fbc8418ea2
commit r14-9376-g5f9d7a5b6cf64639274e63051caf70fbc8418ea2
Author: Jonathan Wakely <jwakely@redhat.com>
Date: Thu Mar 7 13:15:41 2024 +0000
libstdc++: Fix parsing of fractional seconds [PR114244]
When converting a chrono::duration<long double> to a result type with an
integer representation we should use chrono::round<_Duration> so that we
don't truncate towards zero. Rounding ensures that e.g. 0.001999s
becomes 2ms not 1ms.
We can also remove some redundant uses of chrono::duration_cast to
convert from seconds to _Duration, because the _Parser class template
requires _Duration type to be able to represent seconds without loss of
precision.
This also fixes a bug where no fractional part would be parsed for
chrono::duration<long double> because its period is ratio<1>. We should
also consider treat_as_floating_point<rep> when deciding whether to skip
reading a fractional part.
libstdc++-v3/ChangeLog:
PR libstdc++/114244
* include/bits/chrono_io.h (_Parser::operator()): Remove
redundant uses of duration_cast. Use chrono::round to convert
long double value to durations with integer representations.
Check represenation type when deciding whether to skip parsing
fractional seconds.
* testsuite/20_util/duration/114244.cc: New test.
* testsuite/20_util/duration/io.cc: Check that a floating-point
duration with ratio<1> precision can be parsed.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Bug libstdc++/114244] Need to use round when parsing fractional seconds
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
` (6 preceding siblings ...)
2024-03-07 23:45 ` cvs-commit at gcc dot gnu.org
@ 2024-03-07 23:46 ` redi at gcc dot gnu.org
7 siblings, 0 replies; 9+ messages in thread
From: redi at gcc dot gnu.org @ 2024-03-07 23:46 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114244
Jonathan Wakely <redi at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |13.3
--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Fixed on trunk so far.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-03-07 23:46 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-05 17:23 [Bug libstdc++/114244] New: Need to use round when parsing fractional seconds howard.hinnant at gmail dot com
2024-03-05 20:49 ` [Bug libstdc++/114244] " redi at gcc dot gnu.org
2024-03-07 0:30 ` redi at gcc dot gnu.org
2024-03-07 0:32 ` redi at gcc dot gnu.org
2024-03-07 0:37 ` redi at gcc dot gnu.org
2024-03-07 1:01 ` howard.hinnant at gmail dot com
2024-03-07 1:21 ` redi at gcc dot gnu.org
2024-03-07 23:45 ` cvs-commit at gcc dot gnu.org
2024-03-07 23:46 ` redi at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).