[Bug libstdc++/114260] New: std::formatter<std::chrono::utc_time<std::chrono::days>> formats as the previous day

[Bug libstdc++/114260] New: std::formatter<std::chrono::utc_time<std::chrono::days>> formats as the previous day

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114260

            Bug ID: 114260
           Summary: std::formatter<std::chrono::utc_time<std::chrono::days
                    >> formats as the previous day
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: redi at gcc dot gnu.org
  Target Milestone: ---

We give surprising output for std::formatter<std::chrono::utc_time<days>>:

#include <chrono>
#include <iostream>
#include <sstream>

using namespace std::chrono;

int main(){
  auto sdays = std::chrono::sys_days(2024y/March/5);
  auto udays = std::chrono::utc_clock::from_sys(sdays);
  std::cout << udays << '\n';
  std::cout << round<days>(udays) << '\n';
}

This prints:

2024-03-05 00:00:00
2024-03-04 23:59:33

This happens because formatter<chrono::utc_time<days>> subtracts leap seconds
to get a sys_time (and checks to see whether we need to format the seconds as
"60") and then formats that result using formatter<chrono::sys_seconds>. The
result has a higher precision than utc_time<days> and is no longer the
"correct" day.

I think we want to use chrono::round<D> after subtracting leap seconds, to get
back to the original resolution. Otherwise we're formatting a sys_time that
differs from the supplied utc_time by less than that time's minimum tick.

So:

--- a/libstdc++-v3/include/bits/chrono_io.h
+++ b/libstdc++-v3/include/bits/chrono_io.h
@@ -2067,7 +2067,7 @@ namespace __format
          const auto __li = chrono::get_leap_second_info(__t);
          sys_time<_CDur> __s{__t.time_since_epoch() - __li.elapsed};
          if (!__li.is_leap_second) [[likely]]
-           return _M_f._M_format(__s, __fc);
+           return _M_f._M_format(chrono::round<_Duration>(__s), __fc);
          else
            return _M_f._M_format(__utc_leap_second(__s), __fc);
        }



Or maybe not even subtract leap seconds at all when the the sum of elapsed leap
seconds is less than Duration{1}? If the time being formatted can't represent
the number of elapsed leap seconds, is it meaningful to say the time falls
within a leap second? For the first ever leap second, yes it is:

clock_cast<utc_clock>(sys_days{July/1/1972} - 500ms) + 500ms
-> 1972-06-30 23:59:60.000
round<minutes>(clock_cast<utc_clock>(sys_days{July/1/1972} - 500ms) + 500ms)
-> 1972-06-30 23:59:60

But for every leap second after that (and all future ones, unless the sum of
positive and negative leap seconds becomes a multiple of 60 again) rounding a
sys_time to utc_time<minutes> cannot fall within a leap second and so doesn't
need to print "60" for the seconds:

clock_cast<utc_clock>(sys_days{January/1/1973} - 500ms) + 500ms
-> 1972-12-31 23:59:60.000
round<minutes>(clock_cast<utc_clock>(sys_days{January/1/1973} - 500ms) + 500ms)
-> 1972-12-31 23:59:59 (with current GCC trunk, so not rounded to minutes)
-> 1973-01-01 00:00:00 (with the patch above to round to minutes)

The 23:59:59 result is not useful, it's neither a leap second like 23:59:60,
nor a round number of minutes like 00:00:00. I think we should format it as
00:00:00, which we could do by not subtracting the leap seconds at all.

Maybe we could do:

if (auto li = get_leap_second_info(ut); !li.is_leap_second && li.elapsed <
Duration{1})
  _M_format(sys_time<Duration>(ut.time_since_epoch()), fc);
else if (!li.is_leap_second)
  _M_format(round<Duration>(sys_time<CDur>(ut.time_since_epoch()) -
li.elapsed), fc);
else
  // ...

But I don't think that's necessary, just round<Duration> should give the
desired result. Avoiding the subtraction doesn't seem like a useful
optimization (especially as we'd still have done the much slower
get_leap_second_info lookup anyway).

CC Howard to check I'm not talking nonsense.

[Bug libstdc++/114260] std::formatter<std::chrono::utc_time<std::chrono::days>> formats as the previous day

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114260

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |howard.hinnant at gmail dot com

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #0)
> CC Howard to check I'm not talking nonsense.

It would help if I remembered to actually CC him though.

[Bug libstdc++/114260] std::formatter<std::chrono::utc_time<std::chrono::days>> formats as the previous day

[howard.hinnant at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114260

--- Comment #2 from Howard Hinnant <howard.hinnant at gmail dot com> ---
This:

2024-03-05 00:00:00
2024-03-04 23:59:33

looks like correct output to me.

sys_time and utc_time map to the same civil calendar date/time (except during a
leap second).  That is 2024-03-05 00:00:00 sys_time, converted to utc_time,
also should print out as 2024-03-05 00:00:00.

The only difference between sys_time and utc_time is that utc_time counts the
leap seconds since 1970 (27 at this point).  This means if you look at the
difference in .time_since_epoch(), utc_time will be 27 seconds longer, even
though it prints out as the same date and time.

Consequently, 2024-03-05 00:00:00 in utc_time is *not* a multiple of 86400s,
but rather 27s greater than a multiple of 86400s.  And all round<days>(udays)
does is round the .time_since_epoch() to the nearest multiple of 86400s.  Which
in utc_time is 27s earlier, or 2024-03-04 23:59:33.

So in summary, if you make a change, and *don't* get this output, then you've
introduced a bug.

[Bug libstdc++/114260] std::formatter<std::chrono::utc_time<std::chrono::days>> formats as the previous day

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114260

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yup, I understand why that's the output. I just thought it seemed off that
something rounded to days didn't land at 00:00:00 and convinced myself that it
should be "corrected". But of course utc_time<days> is not a year_month_day,
it's an exact time point, and it happens to not line up with 00:00:00 for the
reason you gave:

(In reply to Howard Hinnant from comment #2)
> Consequently, 2024-03-05 00:00:00 in utc_time is *not* a multiple of 86400s,
> but rather 27s greater than a multiple of 86400s.  And all
> round<days>(udays) does is round the .time_since_epoch() to the nearest
> multiple of 86400s.  Which in utc_time is 27s earlier, or 2024-03-04
> 23:59:33.

So not a bug then. Thanks for talking me out of introducing a bug here!