Does this rounding heuristic seem reasonable? I have discussed it with
Howard and he agreed that rounding "2024-09-22 18:34:56" up to the next
day, 2024-09-23, could be surprising. But he convinced me that using
chrono::round was correct in general (certainly better than what we have
now using chrono::time_point_cast). A period like ratio<1,60> (e.g.
a 60Hz refresh rate) can't be represented exactly with decimal digits,
so we want to round from the parsed value to the closest representation
in the result type. Truncating is strictly inferior when we're dealing
with such periods.
The hybrid rounding approach here tries to Do The Right Thing.
MSVC seems to avoid this problem by just refusing to parse the date time
"2024-09-22 18:34:56" into a result type of sys_days, presumably because
the parsed value cannot be represented in the result type. I can see
some justification for that, but I think rounding is more useful than
failing to parse.
Tested x86_64-linux.
-- >8 --
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.
---
libstdc++-v3/include/bits/chrono_io.h | 64 +++++++++++++++++--
.../testsuite/std/time/clock/file/io.cc | 21 +++++-
.../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 1e34c82b532..362bb5aa9e9 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 9da5019ab78..27b4507236e 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 c012520080a..647ed021b24 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) );
--
2.46.1
