]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
libstdc++: Fix rounding in chrono::parse
authorJonathan Wakely <jwakely@redhat.com>
Fri, 6 Sep 2024 20:41:47 +0000 (21:41 +0100)
committerJonathan Wakely <redi@gcc.gnu.org>
Wed, 2 Oct 2024 10:36:17 +0000 (11:36 +0100)
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
libstdc++-v3/testsuite/std/time/clock/file/io.cc
libstdc++-v3/testsuite/std/time/clock/gps/io.cc

index 1e34c82b532d2205f337240e8d629eaf2c8985fa..362bb5aa9e98df8d5fd217daff589cb299eaa1a3 100644 (file)
@@ -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;
     }
index 9da5019ab78bf4d33e996048901b000c1656249a..27b4507236e70bdcd0b0ee7e502185e41d16f2de 100644 (file)
@@ -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 );
index c012520080aed81ca2feba0cdc041bcd802e8933..647ed021b2474c5065bfde0b884d2c6b5b13e9cd 100644 (file)
@@ -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) );