On Wed, 19 Mar 2025 at 13:09, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On Sat, 1 Mar 2025 at 05:24, XU Kailiang <xu2k...@outlook.com> wrote:
> >
> > Formatting a time point with %c was implemented by calling
> > std::vprint_to with format string constructed from locale's D_T_FMT
> > string, but in some locales this string does not compliant to
> > chrono-specs. So just use _M_locale_fmt to avoid this problem.
>
> I realised there's a problem with this solution, which is that time
> zone information is ignored by _M_locale_fmt.
>
> For example:
>
> #include <print>
> #include <chrono>
> #include <locale>
>
> int main()
> {
> std::locale::global(std::locale("aa_DJ.UTF-8"));
Ah, we're also missing __os.imbue(__loc) in _M_locale_fmt, which is
necessary to use the formatting locale properly, and not rely on
std::locale::global having been changed (as your testcase does).
> using namespace std::chrono;
> auto time = sys_days(2025y/March/19) + 6h;
> std::chrono::zoned_time z("America/New_York", time);
> std::println("{:L%c}", z);
> }
>
> This prints:
> Arb 19 Cig 2025 2:00:00 saaku GMT
>
> This is very wrong - it's printing the local time in New York, 2am,
> but showing "GMT" (because that's my system's time zone). This is
> because the C library only considers the global zone (set from
> /etc/localtime, or $TZ, or the thread-unsafe tzset function) and
> ignores the time zone present in the zoned_time.
>
> We can convert a zoned_time to UTC by applying the zone's offset, then
> use localtime (or localtime_r if available) to convert that to a
> struct tm with a broken down time in the system's local time zone,
> then _M_locale_fmt can print it.
>
> So something like:
>
> using namespace chrono;
> using ::std::chrono::__detail::__local_time_fmt;
>
> struct tm __tm{};
> struct tm* __tm_ok = nullptr;
>
> if constexpr (__is_specialization_of<_Tp, __local_time_fmt>)
> {
> if (__t._M_offset_sec)
> {
> auto __ut = __t._M_time - *__t._M_offset_sec;
> auto __us = chrono::round<seconds>(__ut);
> time_t __tt = __us.time_since_epoch().count();
> #if _GLIBCXX_HAVE_LOCALTIME_R
> __tm_ok = ::localtime_r(&__tt, &__tm);
> #else
> if (__tm_ok = ::localtime(&__tt))
> __builtin_memcpy(&__tm, __tm_ok, sizeof(struct tm));
> #endif
> }
> }
>
> if (!__tm_ok)
> {
> auto __d = _S_days(__t);
>
> This needs some configure changes to detect localtime_r, so I'll make
> that work as a follow-up patch.
>
>
> >
> > libstdc++-v3/ChangeLog:
> >
> > PR libstdc++/117214
> > * include/bits/chrono_io.h (__formatter_chrono::_M_c): use
> > _M_locale_fmt to format %c time point.
> > * testsuite/std/time/format/pr117214.cc: New test.
> >
> > Signed-off-by: XU Kailiang <xu2k...@outlook.com>
> > ---
> > libstdc++-v3/include/bits/chrono_io.h | 35 ++++++++++---------
> > .../testsuite/std/time/format/pr117214.cc | 32 +++++++++++++++++
> > 2 files changed, 51 insertions(+), 16 deletions(-)
> > create mode 100644 libstdc++-v3/testsuite/std/time/format/pr117214.cc
> >
> > diff --git a/libstdc++-v3/include/bits/chrono_io.h
> > b/libstdc++-v3/include/bits/chrono_io.h
> > index 8c026586d4c..569f20376b1 100644
> > --- a/libstdc++-v3/include/bits/chrono_io.h
> > +++ b/libstdc++-v3/include/bits/chrono_io.h
> > @@ -887,27 +887,30 @@ namespace __format
> >
> > template<typename _Tp, typename _FormatContext>
> > typename _FormatContext::iterator
> > - _M_c(const _Tp& __tt, typename _FormatContext::iterator __out,
> > + _M_c(const _Tp& __t, typename _FormatContext::iterator __out,
> > _FormatContext& __ctx, bool __mod = false) const
> > {
> > // %c Locale's date and time representation.
> > // %Ec Locale's alternate date and time representation.
> >
> > - basic_string<_CharT> __fmt;
> > - auto __t = _S_floor_seconds(__tt);
> > - locale __loc = _M_locale(__ctx);
> > - const auto& __tp = use_facet<__timepunct<_CharT>>(__loc);
> > - const _CharT* __formats[2];
> > - __tp._M_date_time_formats(__formats);
> > - if (*__formats[__mod]) [[likely]]
> > - {
> > - __fmt = _GLIBCXX_WIDEN("{:L}");
> > - __fmt.insert(3u, __formats[__mod]);
> > - }
> > - else
> > - __fmt = _GLIBCXX_WIDEN("{:L%a %b %e %T %Y}");
> > - return std::vformat_to(std::move(__out), __loc, __fmt,
> > -
> > std::make_format_args<_FormatContext>(__t));
> > + using namespace chrono;
> > + auto __d = _S_days(__t);
> > + using _TDays = decltype(__d);
> > + const auto __ymd = _S_date(__d);
>
> There's no need to use _S_date(__d) here, because we know that __d is
> either sys_days or local_days, so this can be just:
>
> const year_month_day __ymd(__d);
>
> > + const auto __y = __ymd.year();
> > + const auto __hms = _S_hms(__t);
> > +
> > + struct tm __tm{};
> > + __tm.tm_year = (int)__y - 1900;
> > + __tm.tm_yday = (__d - _TDays(__y/January/1)).count();
> > + __tm.tm_mon = (unsigned)_S_month(__t) - 1;
>
> This would call _S_date(__t).month() but we can use __ymd.month().
>
> > + __tm.tm_mday = (unsigned)_S_day(__t);
>
> And this would call _S_date(__t).day() but we can use __ymd.day().
>
> > + __tm.tm_wday = _S_weekday(__t).c_encoding();
>
> And this would call weekday(_S_days(__t)) so we can use weekday(__d).
>
> > + __tm.tm_hour = __hms.hours().count();
> > + __tm.tm_min = __hms.minutes().count();
> > + __tm.tm_sec = __hms.seconds().count();
> > + return _M_locale_fmt(std::move(__out), _M_locale(__ctx), __tm,
> > 'c',
> > + __mod ? 'E' : '\0');
> > }
> >
> > template<typename _Tp, typename _FormatContext>
> > diff --git a/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> > b/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> > new file mode 100644
> > index 00000000000..5b36edadfa0
> > --- /dev/null
> > +++ b/libstdc++-v3/testsuite/std/time/format/pr117214.cc
> > @@ -0,0 +1,32 @@
> > +// { dg-do run { target c++20 } }
> > +// { dg-require-namedlocale "aa_DJ.UTF-8" }
> > +// { dg-require-namedlocale "ar_SA.UTF-8" }
> > +// { dg-require-namedlocale "ca_AD.UTF-8" }
> > +// { dg-require-namedlocale "az_IR.UTF-8" }
> > +// { dg-require-namedlocale "my_MM.UTF-8" }
> > +
> > +#include <chrono>
> > +#include <locale>
> > +#include <testsuite_hooks.h>
> > +
> > +void
> > +test_c()
> > +{
> > + const char *test_locales[] = {
> > + "aa_DJ.UTF-8",
> > + "ar_SA.UTF-8",
> > + "ca_AD.UTF-8",
> > + "az_IR.UTF-8",
> > + "my_MM.UTF-8",
> > + };
> > + for (auto locale_name : test_locales)
> > + {
> > + std::locale::global(std::locale(locale_name));
> > + VERIFY( !std::format("{:L%c}", std::chrono::sys_seconds()).empty() );
> > + }
> > +}
> > +
> > +int main()
> > +{
> > + test_c();
> > +}
> > --
> > 2.48.1
> >
