On Fri, 21 Mar 2025 at 12:24, Tomasz Kaminski <tkami...@redhat.com> wrote: > > > > On Thu, Mar 20, 2025 at 10:39 AM Jonathan Wakely <jwak...@redhat.com> wrote: >> >> The previous commit fixed most cases of %c formatting, but it >> incorrectly prints times using the system's local time zone. This only >> matters if the locale's %c format includes %Z, but some do. >> >> To print a correct value for %Z we can set tm.tm_zone to either "UTC" or >> the abbreviation passed to the formatter in the local-time-format-t >> structure. >> >> For local times with no info and for systems that don't support tm_zone >> (which is new in POSIX.1-2024) we just set tm_isdst = -1 so that no zone >> name is printed. >> >> In theory, a locale's %c format could use %z which should print a +hhmm >> offset from UTC. I'm unsure how to control that though. The new >> tm_gmtoff field in combination with tm_isdst != -1 seems like it should >> work, but using that without also setting tm_zone causes the system zone >> to be used for %Z again. That means local_time_format(lt, nullptr, &off) >> might work for a locale that uses %z but prints the wrong thing for %Z. >> This commit doesn't set tm_gmtoff even if _M_offset_sec is provided for >> a local-time-format-t value. >> >> libstdc++-v3/ChangeLog: >> >> PR libstdc++/117214 >> * configure.ac: Use AC_STRUCT_TIMEZONE. >> * config.h.in: Regenerate. >> * configure: Regenerate. >> * include/bits/chrono_io.h (__formatter_chrono::_M_c): Set >> tm_isdst and tm_zone. >> * testsuite/std/time/format/pr117214.cc: Check %c formatting of >> zoned_time and local time. >> --- >> >> Tested x86_64-linux. > > The c++ code parts LGTM, however the changes in config.h.in and > configure went over my head: they seem to check for new members, > but I cannot confirm if they are correct and according to best practices.
It's all generated by autoconf. The only change is adding AC_STRUCT_TIMEZONE to configure.ac which is documented at https://www.gnu.org/software/autoconf/manual/autoconf-2.64/html_node/Particular-Structures.html#Particular-Structures The rest is just what autoconf generates from that, and so doesn't need to be reviewed. >> diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac >> index a3b257fe652..fe0cdde1f7a 100644 >> --- a/libstdc++-v3/configure.ac >> +++ b/libstdc++-v3/configure.ac >> @@ -584,6 +584,8 @@ GLIBCXX_CHECK_FILEBUF_NATIVE_HANDLES >> # For std::text_encoding >> GLIBCXX_CHECK_TEXT_ENCODING >> >> +AC_STRUCT_TIMEZONE >> + >> # Define documentation rules conditionally. >> >> # See if makeinfo has been installed and is modern enough >> diff --git a/libstdc++-v3/include/bits/chrono_io.h >> b/libstdc++-v3/include/bits/chrono_io.h >> index 86338d48d18..08969166d2f 100644 >> --- a/libstdc++-v3/include/bits/chrono_io.h >> +++ b/libstdc++-v3/include/bits/chrono_io.h >> @@ -894,13 +894,42 @@ namespace __format >> // %Ec Locale's alternate date and time representation. >> >> using namespace chrono; >> + using ::std::chrono::__detail::__utc_leap_second; >> + using ::std::chrono::__detail::__local_time_fmt; >> + >> + struct tm __tm{}; >> + >> + // Some locales use %Z in their %c format but we don't want >> strftime >> + // to use the system's local time zone (from /etc/localtime or $TZ) >> + // as the output for %Z. Setting tm_isdst to -1 says there is no >> + // time zone info available for the time in __tm. >> + __tm.tm_isdst = -1; >> + >> +#ifdef _GLIBCXX_HAVE_STRUCT_TM_TM_ZONE >> + // POSIX.1-2024 adds tm.tm_zone which will be used for %Z. >> + if constexpr (__is_time_point_v<_Tp>) >> + { >> + // One of sys_time, utc_time, or local_time. >> + if constexpr (!is_same_v<typename _Tp::clock, local_t>) >> + __tm.tm_zone = "UTC"; > > I have checked that other clocks, like TAI and GPS that user different zone > name, > are formatted using __local_time_fmt. Thanks! >> >> + } >> + else if constexpr (__is_specialization_of<_Tp, __local_time_fmt>) >> + { >> + // local-time-format-t is used to provide time zone info for >> + // one of zoned_time, tai_time, gps_time, or local_time. >> + if (__t._M_abbrev) >> + __tm.tm_zone = __t._M_abbrev->c_str(); >> + } >> + else >> + __tm.tm_zone = "UTC"; >> +#endif >> + >> auto __d = _S_days(__t); // Either sys_days or local_days. >> using _TDays = decltype(__d); >> 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)__ymd.month() - 1; >> @@ -909,6 +938,7 @@ namespace __format >> __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'); >> } >> diff --git a/libstdc++-v3/testsuite/std/time/format/pr117214.cc >> b/libstdc++-v3/testsuite/std/time/format/pr117214.cc >> index e7831832206..87c703dd255 100644 >> --- a/libstdc++-v3/testsuite/std/time/format/pr117214.cc >> +++ b/libstdc++-v3/testsuite/std/time/format/pr117214.cc >> @@ -28,7 +28,72 @@ test_c() >> } >> } >> >> +#include <stdlib.h> >> +#include <time.h> >> + >> +extern "C++" int tzset(int); >> + >> +template<typename T = void> >> +void maybe_set_tz() >> +{ >> +#ifdef _GLIBCXX_HAVE_SETENV >> + // True if a function 'T tzset()' has been declared. >> + if constexpr (requires (T (*p)()) { p = &tzset; }) >> + { >> + setenv("TZ", "GMT0", 1); >> + tzset(); // Set C library's global time zone to GMT0 >> + } >> +#endif >> +} >> + >> +void >> +test_c_zoned() >> +{ >> +#if _GLIBCXX_USE_CXX11_ABI >> + maybe_set_tz(); >> + >> + using namespace std::chrono; >> + auto time = sys_days(2025y/March/19) + 15h; >> + zoned_time z("America/New_York", time); >> + auto s = std::format(std::locale("aa_DJ.UTF-8"), "{:L%c}", z); >> + VERIFY( s.find("11:00") != s.npos ); >> + >> + // We should not print an incorrect time zone. >> + VERIFY( s.find("GMT") == s.npos ); >> + VERIFY( s.find("UTC") == s.npos ); >> + >> +#ifdef _GLIBCXX_HAVE_STRUCT_TM_TM_ZONE >> + // When tm.tm_zone is supported, we do print the correct time zone. >> + VERIFY( s.find(z.get_info().abbrev) != s.npos ); >> +#endif >> +#endif >> +} >> + >> +void >> +test_c_local() >> +{ >> + maybe_set_tz(); >> + >> + std::locale("aa_DJ.UTF-8"); >> + using namespace std::chrono; >> + auto time = local_days(2025y/March/19) + 11h; >> + auto s = std::format(std::locale("aa_DJ.UTF-8"), "{:L%c}", time); >> + // __builtin_printf("%s\n", s.c_str()); return; >> + VERIFY( s.find("11:00") != s.npos ); >> + >> + // We should not print any time zone. >> + VERIFY( s.find("GMT") == s.npos ); >> + VERIFY( s.find("UTC") == s.npos ); >> + >> +#if _GLIBCXX_USE_CXX11_ABI >> + zoned_time z(system_clock::now()); >> + VERIFY( s.find(z.get_info().abbrev) == s.npos ); >> +#endif >> +} >> + >> int main() >> { >> test_c(); >> + test_c_zoned(); >> + test_c_local(); >> } >> -- >> 2.48.1 >>
