diff options
author | Jonathan Wakely <jwakely@redhat.com> | 2024-09-06 21:41:47 +0100 |
---|---|---|
committer | Jonathan Wakely <redi@gcc.gnu.org> | 2024-10-02 11:36:17 +0100 |
commit | 5cf26f2569bf007a2c9c058e43ddfe9a5f67da42 (patch) | |
tree | e451d2345b8e5efcc46b9d333c8ca320262e288d | |
parent | 05b7ab86e7d70e6f71a3012b08f5704b056a69fe (diff) | |
download | gcc-5cf26f2569bf007a2c9c058e43ddfe9a5f67da42.zip gcc-5cf26f2569bf007a2c9c058e43ddfe9a5f67da42.tar.gz gcc-5cf26f2569bf007a2c9c058e43ddfe9a5f67da42.tar.bz2 |
libstdc++: Fix rounding in chrono::parse
I noticed that chrono::parse was using duration_cast and time_point_cast
to convert the parsed value to the result. Those functions truncate
towards zero, which is not generally what you want. Especially for
negative times before the epoch, where truncating towards zero rounds
"up" towards the next duration/time_point. Using chrono::round is
typically better, as that rounds to nearest.
However, while testing the fix I realised that rounding to the nearest
can give surprising results in some cases. For example if we parse a
chrono::sys_days using chrono::parse("F %T", "2024-09-22 18:34:56", tp)
then we will round up to the next day, i.e. sys_days(2024y/09/23). That
seems surprising, and I think 2024-09-22 is what most users would
expect.
This change attempts to provide a hybrid rounding heuristic where we use
chrono::round for the general case, but when the result has a period
that is one of minutes, hours, days, weeks, or years then we truncate
towards negative infinity using chrono::floor. This means that we
truncate "2024-09-22 18:34:56" to the start of the current
minute/hour/day/week/year, instead of rounding up to 2024-09-23, or to
18:35, or 17:00. For a period of months chrono::round is used, because
the months duration is defined as a twelfth of a year, which is not
actually the length of any calendar month. We don't want to truncate to
a whole number of "months" if that can actually go from e.g. 2023-03-01
to 2023-01-31, because February is shorter than chrono::months(1).
libstdc++-v3/ChangeLog:
* include/bits/chrono_io.h (__detail::__use_floor): New
function.
(__detail::__round): New function.
(from_stream): Use __detail::__round.
* testsuite/std/time/clock/file/io.cc: Check for expected
rounding in parse.
* testsuite/std/time/clock/gps/io.cc: Likewise.
-rw-r--r-- | libstdc++-v3/include/bits/chrono_io.h | 64 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/std/time/clock/file/io.cc | 21 | ||||
-rw-r--r-- | libstdc++-v3/testsuite/std/time/clock/gps/io.cc | 20 |
3 files changed, 97 insertions, 8 deletions
diff --git a/libstdc++-v3/include/bits/chrono_io.h b/libstdc++-v3/include/bits/chrono_io.h index 1e34c82..362bb5a 100644 --- a/libstdc++-v3/include/bits/chrono_io.h +++ b/libstdc++-v3/include/bits/chrono_io.h @@ -2407,6 +2407,56 @@ namespace __detail template<typename _Duration> using _Parser_t = _Parser<common_type_t<_Duration, seconds>>; + template<typename _Duration> + consteval bool + __use_floor() + { + if constexpr (_Duration::period::den == 1) + { + switch (_Duration::period::num) + { + case minutes::period::num: + case hours::period::num: + case days::period::num: + case weeks::period::num: + case years::period::num: + return true; + } + } + return false; + } + + // A "do the right thing" rounding function for duration and time_point + // values extracted by from_stream. When treat_as_floating_point is true + // we don't want to do anything, just a straightforward conversion. + // When the destination type has a period of minutes, hours, days, weeks, + // or years, we use chrono::floor to truncate towards negative infinity. + // This ensures that an extracted timestamp such as 2024-09-05 13:00:00 + // will produce 2024-09-05 when rounded to days, rather than rounding up + // to 2024-09-06 (a different day). + // Otherwise, use chrono::round to get the nearest value representable + // in the destination type. + template<typename _ToDur, typename _Tp> + constexpr auto + __round(const _Tp& __t) + { + if constexpr (__is_duration_v<_Tp>) + { + if constexpr (treat_as_floating_point_v<typename _Tp::rep>) + return chrono::duration_cast<_ToDur>(__t); + else if constexpr (__detail::__use_floor<_ToDur>()) + return chrono::floor<_ToDur>(__t); + else + return chrono::round<_ToDur>(__t); + } + else + { + static_assert(__is_time_point_v<_Tp>); + using _Tpt = time_point<typename _Tp::clock, _ToDur>; + return _Tpt(__detail::__round<_ToDur>(__t.time_since_epoch())); + } + } + } // namespace __detail /// @endcond @@ -2421,7 +2471,7 @@ namespace __detail auto __need = __format::_ChronoParts::_TimeOfDay; __detail::_Parser_t<duration<_Rep, _Period>> __p(__need); if (__p(__is, __fmt, __abbrev, __offset)) - __d = chrono::duration_cast<duration<_Rep, _Period>>(__p._M_time); + __d = __detail::__round<duration<_Rep, _Period>>(__p._M_time); return __is; } @@ -2882,7 +2932,7 @@ namespace __detail else { auto __st = __p._M_sys_days + __p._M_time - *__offset; - __tp = chrono::time_point_cast<_Duration>(__st); + __tp = __detail::__round<_Duration>(__st); } } return __is; @@ -2918,7 +2968,7 @@ namespace __detail // "23:59:60" to correctly produce a time within a leap second. auto __ut = utc_clock::from_sys(__p._M_sys_days) + __p._M_time - *__offset; - __tp = chrono::time_point_cast<_Duration>(__ut); + __tp = __detail::__round<_Duration>(__ut); } return __is; } @@ -2956,7 +3006,7 @@ namespace __detail constexpr sys_days __epoch(-days(4383)); // 1958y/1/1 auto __d = __p._M_sys_days - __epoch + __p._M_time - *__offset; tai_time<common_type_t<_Duration, seconds>> __tt(__d); - __tp = chrono::time_point_cast<_Duration>(__tt); + __tp = __detail::__round<_Duration>(__tt); } } return __is; @@ -2995,7 +3045,7 @@ namespace __detail constexpr sys_days __epoch(days(3657)); // 1980y/1/Sunday[1] auto __d = __p._M_sys_days - __epoch + __p._M_time - *__offset; gps_time<common_type_t<_Duration, seconds>> __gt(__d); - __tp = chrono::time_point_cast<_Duration>(__gt); + __tp = __detail::__round<_Duration>(__gt); } } return __is; @@ -3020,7 +3070,7 @@ namespace __detail { sys_time<_Duration> __st; if (chrono::from_stream(__is, __fmt, __st, __abbrev, __offset)) - __tp = chrono::time_point_cast<_Duration>(file_clock::from_sys(__st)); + __tp = __detail::__round<_Duration>(file_clock::from_sys(__st)); return __is; } @@ -3049,7 +3099,7 @@ namespace __detail { days __d = __p._M_sys_days.time_since_epoch(); auto __t = local_days(__d) + __p._M_time; // ignore offset - __tp = chrono::time_point_cast<_Duration>(__t); + __tp = __detail::__round<_Duration>(__t); } return __is; } diff --git a/libstdc++-v3/testsuite/std/time/clock/file/io.cc b/libstdc++-v3/testsuite/std/time/clock/file/io.cc index 9da5019..27b4507 100644 --- a/libstdc++-v3/testsuite/std/time/clock/file/io.cc +++ b/libstdc++-v3/testsuite/std/time/clock/file/io.cc @@ -67,8 +67,27 @@ test_parse() VERIFY( abbrev == "456" ); VERIFY( offset == (1h + 23min) ); - ss.str(""); + // Test rounding ss.clear(); + ss.str("2224-09-06 23"); + file_time<days> d; + ss >> parse("%F %H", d); // Should be truncated to start of day, not rounded. + ss.str(""); + ss << d; + VERIFY( ss.str() == "2224-09-06 00:00:00" ); + ss.str("1969-12-31 23"); + ss >> parse("%F %H", d); // Should be truncated to start of day, not rounded. + ss.str(""); + ss << d; + VERIFY( ss.str() == "1969-12-31 00:00:00" ); + + file_time<duration<long, std::ratio<10>>> ds; // decaseconds + ss.str("2224-09-06 15:07:06"); + ss >> parse("%F %T", ds); // Should be rounded to nearest decasecond. + ss << ds; + VERIFY( ss.str() == "2224-09-06 15:07:10" ); + + ss.str(""); ss << file_time<seconds>{}; VERIFY( ss >> parse("%F %T", tp) ); VERIFY( tp.time_since_epoch() == 0s ); diff --git a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc index c012520..647ed02 100644 --- a/libstdc++-v3/testsuite/std/time/clock/gps/io.cc +++ b/libstdc++-v3/testsuite/std/time/clock/gps/io.cc @@ -75,6 +75,26 @@ test_parse() VERIFY( abbrev == "GPS" ); VERIFY( offset == -(12h + 34min) ); + // Test rounding + ss.clear(); + ss.str("2224-09-06 23"); + gps_time<days> d; + ss >> parse("%F %H", d); // Should be truncated to start of day, not rounded. + ss.str(""); + ss << d; + VERIFY( ss.str() == "2224-09-06 00:00:00" ); + ss.str("1969-12-31 23"); + ss >> parse("%F %H", d); // Should be truncated to start of day, not rounded. + ss.str(""); + ss << d; + VERIFY( ss.str() == "1969-12-31 00:00:00" ); + + gps_time<duration<long, std::ratio<10>>> ds; // decaseconds + ss.str("2224-09-06 15:07:06"); + ss >> parse("%F %T", ds); // Should be rounded to nearest decasecond. + ss << ds; + VERIFY( ss.str() == "2224-09-06 15:07:10" ); + ss.str(""); ss << gps_seconds{}; VERIFY( ss >> parse("%F %T", tp) ); |