public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* [RFC] libstdc++: chrono::current_zone() should always respect $TZ
@ 2023-05-07 21:55 James Morris
  2023-05-08  7:41 ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: James Morris @ 2023-05-07 21:55 UTC (permalink / raw)
  To: libstdc++

Hello,

I was playing around with GCC 13 and upgrading some old C++ code to
C++20. I had a small function for getting the current time based on
the <ctime> functions and tried to convert it to use the new <chrono>
functions. I was quite surprised that when using
chrono::current_zone(), my program stopped responding to the TZ
environment variable.

I'd like to propose that chrono::current_zone() always pays attention
to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
already has code for this gated by an #ifdef, so maybe all that needs
to be done is to make that code available to all platforms.

Example of my code with <ctime>:

```
#include <ctime>
#include <iostream>

int main() {
    std::time_t epoch_time = std::time(nullptr);
    std::tm* local_time = std::localtime(&epoch_time);

    char time_string[] = "00:00:00 UTC";
    std::strftime(time_string, sizeof "00:00:00 UTC", "%T %Z", local_time);

    std::cout << time_string << '\n';
}
```

Example of what I tried to do with <chrono>:

```
#include <chrono>
#include <format>
#include <iostream>

namespace chrono = std::chrono;

int main() {
    auto zt = chrono::zoned_time(chrono::current_zone(),
chrono::system_clock::now());
    std::cout << std::format("{:%T %Z}", zt) << '\n';
}
```

Diff of proposed changes:

```
diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc
index a43b4f33eba..c16913dab67 100644
--- a/libstdc++-v3/src/c++20/tzdb.cc
+++ b/libstdc++-v3/src/c++20/tzdb.cc
@@ -1632,6 +1632,20 @@ namespace std::chrono
   {
     // TODO cache this function's result?

+    // AIX stores current zone in $TZ in /etc/environment but the value
+    // is typically a POSIX time zone name, not IANA zone.
+    // https://developer.ibm.com/articles/au-aix-posix/
+    // https://www.ibm.com/support/pages/managing-time-zone-variable-posix
+    //
+    // Other POSIX systems also allow the user to override the
+    // system timezone by setting $TZ.
+    if (const char* env = std::getenv("TZ"))
+      {
+    // This will fail unless TZ contains an IANA time zone name.
+    if (auto tz = do_locate_zone(this->zones, this->links, env))
+      return tz;
+      }
+
 #ifndef _AIX
     error_code ec;
     // This should be a symlink to e.g. /usr/share/zoneinfo/Europe/London
@@ -1685,17 +1699,6 @@ namespace std::chrono
           return tz;
           }
       }
-#else
-    // AIX stores current zone in $TZ in /etc/environment but the value
-    // is typically a POSIX time zone name, not IANA zone.
-    // https://developer.ibm.com/articles/au-aix-posix/
-    // https://www.ibm.com/support/pages/managing-time-zone-variable-posix
-    if (const char* env = std::getenv("TZ"))
-      {
-    // This will fail unless TZ contains an IANA time zone name.
-    if (auto tz = do_locate_zone(this->zones, this->links, env))
-      return tz;
-      }
 #endif

     // Default to UTC.
```

Thanks,
James

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] libstdc++: chrono::current_zone() should always respect $TZ
  2023-05-07 21:55 [RFC] libstdc++: chrono::current_zone() should always respect $TZ James Morris
@ 2023-05-08  7:41 ` Jonathan Wakely
  2023-05-08  8:06   ` Jonathan Wakely
                     ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-05-08  7:41 UTC (permalink / raw)
  To: James Morris; +Cc: libstdc++

[-- Attachment #1: Type: text/plain, Size: 4449 bytes --]

On Sun, 7 May 2023, 22:55 James Morris via Libstdc++, <libstdc++@gcc.gnu.org>
wrote:

> Hello,
>
> I was playing around with GCC 13 and upgrading some old C++ code to
> C++20. I had a small function for getting the current time based on
> the <ctime> functions and tried to convert it to use the new <chrono>
> functions. I was quite surprised that when using
> chrono::current_zone(), my program stopped responding to the TZ
> environment variable.
>
> I'd like to propose that chrono::current_zone() always pays attention
> to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
> already has code for this gated by an #ifdef, so maybe all that needs
> to be done is to make that code available to all platforms.
>

No, this is not a good idea. Libstdc++ cannot inspect the environment
safely because another thread could be changing it concurrently, which
would lead to undefined behaviour. The commented out code was an attempt to
support <chrono> on AIX which is unconventional and has no /etc/localtime
symlink.

In any case, the C++ standard requires that current_zone() refers to the
computer's zone, not just the current process' TZ setting:

"A pointer to the time zone which the computer has set as its local time
zone."

<chrono> is not <ctime>. The chrono lib is not restricted to only working
with a global time zone defined in the standard library, but instead
everything works equally well with any chrono::time_zone object, whether
that refers to the system zone or not. If you want to use a per-process
zone specified by $TZ then you can easily do so, using something like:

const chrono::time_zone* tz;
if (auto env = getenv("TZ"))
  tz = chrono::locate_zone(env);
if (!tz)
  tz = chrono::current_zone();

This makes it your problem if you modify the environment in parallel. You
can choose to do this in main() before starting other threads, but that's
not something which libstdc++ can control, as current_zone() must work from
anywhere in the program.


> Example of my code with <ctime>:
>
> ```
> #include <ctime>
> #include <iostream>
>
> int main() {
>     std::time_t epoch_time = std::time(nullptr);
>     std::tm* local_time = std::localtime(&epoch_time);
>
>     char time_string[] = "00:00:00 UTC";
>     std::strftime(time_string, sizeof "00:00:00 UTC", "%T %Z", local_time);
>
>     std::cout << time_string << '\n';
> }
> ```
>
> Example of what I tried to do with <chrono>:
>
> ```
> #include <chrono>
> #include <format>
> #include <iostream>
>
> namespace chrono = std::chrono;
>
> int main() {
>     auto zt = chrono::zoned_time(chrono::current_zone(),
> chrono::system_clock::now());
>     std::cout << std::format("{:%T %Z}", zt) << '\n';
> }
> ```
>
> Diff of proposed changes:
>
> ```
> diff --git a/libstdc++-v3/src/c++20/tzdb.cc
> b/libstdc++-v3/src/c++20/tzdb.cc
> index a43b4f33eba..c16913dab67 100644
> --- a/libstdc++-v3/src/c++20/tzdb.cc
> +++ b/libstdc++-v3/src/c++20/tzdb.cc
> @@ -1632,6 +1632,20 @@ namespace std::chrono
>    {
>      // TODO cache this function's result?
>
> +    // AIX stores current zone in $TZ in /etc/environment but the value
> +    // is typically a POSIX time zone name, not IANA zone.
> +    // https://developer.ibm.com/articles/au-aix-posix/
> +    //
> https://www.ibm.com/support/pages/managing-time-zone-variable-posix
> +    //
> +    // Other POSIX systems also allow the user to override the
> +    // system timezone by setting $TZ.
> +    if (const char* env = std::getenv("TZ"))
> +      {
> +    // This will fail unless TZ contains an IANA time zone name.
> +    if (auto tz = do_locate_zone(this->zones, this->links, env))
> +      return tz;
> +      }
> +
>  #ifndef _AIX
>      error_code ec;
>      // This should be a symlink to e.g. /usr/share/zoneinfo/Europe/London
> @@ -1685,17 +1699,6 @@ namespace std::chrono
>            return tz;
>            }
>        }
> -#else
> -    // AIX stores current zone in $TZ in /etc/environment but the value
> -    // is typically a POSIX time zone name, not IANA zone.
> -    // https://developer.ibm.com/articles/au-aix-posix/
> -    //
> https://www.ibm.com/support/pages/managing-time-zone-variable-posix
> -    if (const char* env = std::getenv("TZ"))
> -      {
> -    // This will fail unless TZ contains an IANA time zone name.
> -    if (auto tz = do_locate_zone(this->zones, this->links, env))
> -      return tz;
> -      }
>  #endif
>
>      // Default to UTC.
> ```
>
> Thanks,
> James
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] libstdc++: chrono::current_zone() should always respect $TZ
  2023-05-08  7:41 ` Jonathan Wakely
@ 2023-05-08  8:06   ` Jonathan Wakely
  2023-05-08  8:07   ` Jonathan Wakely
  2023-05-09  8:54   ` Florian Weimer
  2 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-05-08  8:06 UTC (permalink / raw)
  To: James Morris; +Cc: libstdc++

[-- Attachment #1: Type: text/plain, Size: 4925 bytes --]

On Mon, 8 May 2023, 08:41 Jonathan Wakely, <jwakely.gcc@gmail.com> wrote:

>
>
> On Sun, 7 May 2023, 22:55 James Morris via Libstdc++, <
> libstdc++@gcc.gnu.org> wrote:
>
>> Hello,
>>
>> I was playing around with GCC 13 and upgrading some old C++ code to
>> C++20. I had a small function for getting the current time based on
>> the <ctime> functions and tried to convert it to use the new <chrono>
>> functions. I was quite surprised that when using
>> chrono::current_zone(), my program stopped responding to the TZ
>> environment variable.
>>
>> I'd like to propose that chrono::current_zone() always pays attention
>> to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
>> already has code for this gated by an #ifdef, so maybe all that needs
>> to be done is to make that code available to all platforms.
>>
>
> No, this is not a good idea. Libstdc++ cannot inspect the environment
> safely because another thread could be changing it concurrently, which
> would lead to undefined behaviour. The commented out code was an
>

Sorry, I mean #ifdef'd code, not commented out.

In any case, that code is a last resort because AIX leaves us no other
option except for parsing /etc/environment to find TZ (but I don't know if
that's reliable/correct either).


attempt to support <chrono> on AIX which is unconventional and has no
> /etc/localtime symlink.
>
> In any case, the C++ standard requires that current_zone() refers to the
> computer's zone, not just the current process' TZ setting:
>
> "A pointer to the time zone which the computer has set as its local time
> zone."
>
> <chrono> is not <ctime>. The chrono lib is not restricted to only working
> with a global time zone defined in the standard library, but instead
> everything works equally well with any chrono::time_zone object, whether
> that refers to the system zone or not. If you want to use a per-process
> zone specified by $TZ then you can easily do so, using something like:
>
> const chrono::time_zone* tz;
> if (auto env = getenv("TZ"))
>   tz = chrono::locate_zone(env);
> if (!tz)
>   tz = chrono::current_zone();
>
> This makes it your problem if you modify the environment in parallel. You
> can choose to do this in main() before starting other threads, but that's
> not something which libstdc++ can control, as current_zone() must work from
> anywhere in the program.
>
>
>> Example of my code with <ctime>:
>>
>> ```
>> #include <ctime>
>> #include <iostream>
>>
>> int main() {
>>     std::time_t epoch_time = std::time(nullptr);
>>     std::tm* local_time = std::localtime(&epoch_time);
>>
>>     char time_string[] = "00:00:00 UTC";
>>     std::strftime(time_string, sizeof "00:00:00 UTC", "%T %Z",
>> local_time);
>>
>>     std::cout << time_string << '\n';
>> }
>> ```
>>
>> Example of what I tried to do with <chrono>:
>>
>> ```
>> #include <chrono>
>> #include <format>
>> #include <iostream>
>>
>> namespace chrono = std::chrono;
>>
>> int main() {
>>     auto zt = chrono::zoned_time(chrono::current_zone(),
>> chrono::system_clock::now());
>>     std::cout << std::format("{:%T %Z}", zt) << '\n';
>> }
>> ```
>>
>> Diff of proposed changes:
>>
>> ```
>> diff --git a/libstdc++-v3/src/c++20/tzdb.cc
>> b/libstdc++-v3/src/c++20/tzdb.cc
>> index a43b4f33eba..c16913dab67 100644
>> --- a/libstdc++-v3/src/c++20/tzdb.cc
>> +++ b/libstdc++-v3/src/c++20/tzdb.cc
>> @@ -1632,6 +1632,20 @@ namespace std::chrono
>>    {
>>      // TODO cache this function's result?
>>
>> +    // AIX stores current zone in $TZ in /etc/environment but the value
>> +    // is typically a POSIX time zone name, not IANA zone.
>> +    // https://developer.ibm.com/articles/au-aix-posix/
>> +    //
>> https://www.ibm.com/support/pages/managing-time-zone-variable-posix
>> +    //
>> +    // Other POSIX systems also allow the user to override the
>> +    // system timezone by setting $TZ.
>> +    if (const char* env = std::getenv("TZ"))
>> +      {
>> +    // This will fail unless TZ contains an IANA time zone name.
>> +    if (auto tz = do_locate_zone(this->zones, this->links, env))
>> +      return tz;
>> +      }
>> +
>>  #ifndef _AIX
>>      error_code ec;
>>      // This should be a symlink to e.g. /usr/share/zoneinfo/Europe/London
>> @@ -1685,17 +1699,6 @@ namespace std::chrono
>>            return tz;
>>            }
>>        }
>> -#else
>> -    // AIX stores current zone in $TZ in /etc/environment but the value
>> -    // is typically a POSIX time zone name, not IANA zone.
>> -    // https://developer.ibm.com/articles/au-aix-posix/
>> -    //
>> https://www.ibm.com/support/pages/managing-time-zone-variable-posix
>> -    if (const char* env = std::getenv("TZ"))
>> -      {
>> -    // This will fail unless TZ contains an IANA time zone name.
>> -    if (auto tz = do_locate_zone(this->zones, this->links, env))
>> -      return tz;
>> -      }
>>  #endif
>>
>>      // Default to UTC.
>> ```
>>
>> Thanks,
>> James
>>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] libstdc++: chrono::current_zone() should always respect $TZ
  2023-05-08  7:41 ` Jonathan Wakely
  2023-05-08  8:06   ` Jonathan Wakely
@ 2023-05-08  8:07   ` Jonathan Wakely
  2023-05-09  8:54   ` Florian Weimer
  2 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-05-08  8:07 UTC (permalink / raw)
  To: James Morris; +Cc: libstdc++

[-- Attachment #1: Type: text/plain, Size: 4794 bytes --]

On Mon, 8 May 2023, 08:41 Jonathan Wakely, <jwakely.gcc@gmail.com> wrote:

>
>
> On Sun, 7 May 2023, 22:55 James Morris via Libstdc++, <
> libstdc++@gcc.gnu.org> wrote:
>
>> Hello,
>>
>> I was playing around with GCC 13 and upgrading some old C++ code to
>> C++20. I had a small function for getting the current time based on
>> the <ctime> functions and tried to convert it to use the new <chrono>
>> functions. I was quite surprised that when using
>> chrono::current_zone(), my program stopped responding to the TZ
>> environment variable.
>>
>> I'd like to propose that chrono::current_zone() always pays attention
>> to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
>> already has code for this gated by an #ifdef, so maybe all that needs
>> to be done is to make that code available to all platforms.
>>
>
> No, this is not a good idea. Libstdc++ cannot inspect the environment
> safely because another thread could be changing it concurrently, which
> would lead to undefined behaviour. The commented out code was an attempt to
> support <chrono> on AIX which is unconventional and has no /etc/localtime
> symlink.
>
> In any case, the C++ standard requires that current_zone() refers to the
> computer's zone, not just the current process' TZ setting:
>
> "A pointer to the time zone which the computer has set as its local time
> zone."
>
> <chrono> is not <ctime>. The chrono lib is not restricted to only working
> with a global time zone defined in the standard library, but instead
> everything works equally well with any chrono::time_zone object, whether
> that refers to the system zone or not. If you want to use a per-process
> zone specified by $TZ then you can easily do so, using something like:
>
> const chrono::time_zone* tz;
>

Oops, this needs to be initialized to null. I shouldn't reply before
breakfast and a cup of tea.


if (auto env = getenv("TZ"))
>   tz = chrono::locate_zone(env);
> if (!tz)
>   tz = chrono::current_zone();
>
> This makes it your problem if you modify the environment in parallel. You
> can choose to do this in main() before starting other threads, but that's
> not something which libstdc++ can control, as current_zone() must work from
> anywhere in the program.
>
>
>> Example of my code with <ctime>:
>>
>> ```
>> #include <ctime>
>> #include <iostream>
>>
>> int main() {
>>     std::time_t epoch_time = std::time(nullptr);
>>     std::tm* local_time = std::localtime(&epoch_time);
>>
>>     char time_string[] = "00:00:00 UTC";
>>     std::strftime(time_string, sizeof "00:00:00 UTC", "%T %Z",
>> local_time);
>>
>>     std::cout << time_string << '\n';
>> }
>> ```
>>
>> Example of what I tried to do with <chrono>:
>>
>> ```
>> #include <chrono>
>> #include <format>
>> #include <iostream>
>>
>> namespace chrono = std::chrono;
>>
>> int main() {
>>     auto zt = chrono::zoned_time(chrono::current_zone(),
>> chrono::system_clock::now());
>>     std::cout << std::format("{:%T %Z}", zt) << '\n';
>> }
>> ```
>>
>> Diff of proposed changes:
>>
>> ```
>> diff --git a/libstdc++-v3/src/c++20/tzdb.cc
>> b/libstdc++-v3/src/c++20/tzdb.cc
>> index a43b4f33eba..c16913dab67 100644
>> --- a/libstdc++-v3/src/c++20/tzdb.cc
>> +++ b/libstdc++-v3/src/c++20/tzdb.cc
>> @@ -1632,6 +1632,20 @@ namespace std::chrono
>>    {
>>      // TODO cache this function's result?
>>
>> +    // AIX stores current zone in $TZ in /etc/environment but the value
>> +    // is typically a POSIX time zone name, not IANA zone.
>> +    // https://developer.ibm.com/articles/au-aix-posix/
>> +    //
>> https://www.ibm.com/support/pages/managing-time-zone-variable-posix
>> +    //
>> +    // Other POSIX systems also allow the user to override the
>> +    // system timezone by setting $TZ.
>> +    if (const char* env = std::getenv("TZ"))
>> +      {
>> +    // This will fail unless TZ contains an IANA time zone name.
>> +    if (auto tz = do_locate_zone(this->zones, this->links, env))
>> +      return tz;
>> +      }
>> +
>>  #ifndef _AIX
>>      error_code ec;
>>      // This should be a symlink to e.g. /usr/share/zoneinfo/Europe/London
>> @@ -1685,17 +1699,6 @@ namespace std::chrono
>>            return tz;
>>            }
>>        }
>> -#else
>> -    // AIX stores current zone in $TZ in /etc/environment but the value
>> -    // is typically a POSIX time zone name, not IANA zone.
>> -    // https://developer.ibm.com/articles/au-aix-posix/
>> -    //
>> https://www.ibm.com/support/pages/managing-time-zone-variable-posix
>> -    if (const char* env = std::getenv("TZ"))
>> -      {
>> -    // This will fail unless TZ contains an IANA time zone name.
>> -    if (auto tz = do_locate_zone(this->zones, this->links, env))
>> -      return tz;
>> -      }
>>  #endif
>>
>>      // Default to UTC.
>> ```
>>
>> Thanks,
>> James
>>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] libstdc++: chrono::current_zone() should always respect $TZ
  2023-05-08  7:41 ` Jonathan Wakely
  2023-05-08  8:06   ` Jonathan Wakely
  2023-05-08  8:07   ` Jonathan Wakely
@ 2023-05-09  8:54   ` Florian Weimer
  2023-05-09  9:15     ` Jonathan Wakely
  2 siblings, 1 reply; 7+ messages in thread
From: Florian Weimer @ 2023-05-09  8:54 UTC (permalink / raw)
  To: Jonathan Wakely via Libstdc++; +Cc: James Morris, Jonathan Wakely

* Jonathan Wakely via Libstdc:

> On Sun, 7 May 2023, 22:55 James Morris via Libstdc++, <libstdc++@gcc.gnu.org>
> wrote:
>
>> Hello,
>>
>> I was playing around with GCC 13 and upgrading some old C++ code to
>> C++20. I had a small function for getting the current time based on
>> the <ctime> functions and tried to convert it to use the new <chrono>
>> functions. I was quite surprised that when using
>> chrono::current_zone(), my program stopped responding to the TZ
>> environment variable.
>>
>> I'd like to propose that chrono::current_zone() always pays attention
>> to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
>> already has code for this gated by an #ifdef, so maybe all that needs
>> to be done is to make that code available to all platforms.
>>
>
> No, this is not a good idea. Libstdc++ cannot inspect the environment
> safely because another thread could be changing it concurrently, which
> would lead to undefined behaviour. The commented out code was an attempt to
> support <chrono> on AIX which is unconventional and has no /etc/localtime
> symlink.

We can make getenv thread-safe in glibc.  Would that help?

It's still going to be quite slow, so maybe we need a new interface
after all.

> In any case, the C++ standard requires that current_zone() refers to the
> computer's zone, not just the current process' TZ setting:
>
> "A pointer to the time zone which the computer has set as its local time
> zone."

The notion that a computer has a single local time zone is strange.
Surely C++ allows me to set the time zone even if the computer is
located in a country far away, and the machine owner has different
preferences?!

Thanks,
Florian


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] libstdc++: chrono::current_zone() should always respect $TZ
  2023-05-09  8:54   ` Florian Weimer
@ 2023-05-09  9:15     ` Jonathan Wakely
  2023-05-09  9:20       ` Jonathan Wakely
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Wakely @ 2023-05-09  9:15 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Jonathan Wakely via Libstdc++, James Morris, Jonathan Wakely

[-- Attachment #1: Type: text/plain, Size: 4224 bytes --]

On Tue, 9 May 2023 at 09:54, Florian Weimer via Libstdc++ <
libstdc++@gcc.gnu.org> wrote:

> * Jonathan Wakely via Libstdc:
>
> > On Sun, 7 May 2023, 22:55 James Morris via Libstdc++, <
> libstdc++@gcc.gnu.org>
> > wrote:
> >
> >> Hello,
> >>
> >> I was playing around with GCC 13 and upgrading some old C++ code to
> >> C++20. I had a small function for getting the current time based on
> >> the <ctime> functions and tried to convert it to use the new <chrono>
> >> functions. I was quite surprised that when using
> >> chrono::current_zone(), my program stopped responding to the TZ
> >> environment variable.
> >>
> >> I'd like to propose that chrono::current_zone() always pays attention
> >> to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
> >> already has code for this gated by an #ifdef, so maybe all that needs
> >> to be done is to make that code available to all platforms.
> >>
> >
> > No, this is not a good idea. Libstdc++ cannot inspect the environment
> > safely because another thread could be changing it concurrently, which
> > would lead to undefined behaviour. The commented out code was an attempt
> to
> > support <chrono> on AIX which is unconventional and has no /etc/localtime
> > symlink.
>
> We can make getenv thread-safe in glibc.  Would that help?
>

In general, yes, but not here. That would be a glibc-specific feature, so
not portable to other targets libstdc++ supports. And in any case, I don't
think we want to depend on $TZ. That's not the intended design of the
std::chrono API.


>
> It's still going to be quite slow, so maybe we need a new interface
> after all.
>
> > In any case, the C++ standard requires that current_zone() refers to the
> > computer's zone, not just the current process' TZ setting:
> >
> > "A pointer to the time zone which the computer has set as its local time
> > zone."
>
> The notion that a computer has a single local time zone is strange.
>

Yes, and that's a flaw of <time.h>, but not a problem with <chrono>.


> Surely C++ allows me to set the time zone even if the computer is
> located in a country far away, and the machine owner has different
> preferences?!
>


Yes, as I explained in the email you're replying to :-)

A C++ program can use **any number** of different time zones. There is no
preference given in the API to any particular time zone, unlike the libc
<time.h> which depends on global state for "the time zone" (singular). The
only time zone that has special treatment in the C++ API is UTC, which is
the default time zone used by std::chrono::zoned_time<Dur> iff you don't
construct it with a pointer to a specific time zone. But even that can be
changed via chrono::zoned_traits.

The chrono::current_zone() function is not used implicitly by the API, so
if you choose to use it when it's not the time zone you want, **that's your
fault**.

If you want to use "Europe/London" as your program's default time zone, you
can do that. If you want to use the time zone named by getenv("TZ") as your
program's default time zone, you can do that.

If you want to make that automatic:

// A thin wrapper around a const chrono::time_zone*
struct TZ_ptr
{
  TZ_ptr(const time_zone* z) noexcept : zone(z) { }
  auto operator->() const noexcept { return zone; }
  const std::chrono::time_zone* zone;
};

// Trait class specifying behaviour of a zoned_time
// that uses a TZ_ptr.
template<>
struct std::chrono::zoned_traits<TZ_ptr>
{
  static const time_zone* default_zone()
  {
    if (auto env = std::getenv("TZ"))
      if (auto zone = locate_zone(env))
        return zone;
    return std::chrono::current_zone();
  }
  static const time_zone* locate_zone(string_view name)
  { return std::chrono::locate_zone(name); }
};

Now you can use std::chrono::zoned_time<chrono::seconds, TZ_ptr> and it
will default to the time zone specified by $TZ, or if that's not set or not
valid, it will use chrono::current_zone().

The solution to chrono::current_zone() not returning the time zone you want
is to stop using chrono::current_zone(). It's not to make non-portable
changes to the semantics of chrono::current_zone() that would be specific
to libstdc++ on glibc, and not supported by any other std::chrono::
implementation.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] libstdc++: chrono::current_zone() should always respect $TZ
  2023-05-09  9:15     ` Jonathan Wakely
@ 2023-05-09  9:20       ` Jonathan Wakely
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Wakely @ 2023-05-09  9:20 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Jonathan Wakely via Libstdc++, James Morris, Jonathan Wakely

[-- Attachment #1: Type: text/plain, Size: 3610 bytes --]

On Tue, 9 May 2023 at 10:15, Jonathan Wakely <jwakely@redhat.com> wrote:

>
>
> On Tue, 9 May 2023 at 09:54, Florian Weimer via Libstdc++ <
> libstdc++@gcc.gnu.org> wrote:
>
>> * Jonathan Wakely via Libstdc:
>>
>> > On Sun, 7 May 2023, 22:55 James Morris via Libstdc++, <
>> libstdc++@gcc.gnu.org>
>> > wrote:
>> >
>> >> Hello,
>> >>
>> >> I was playing around with GCC 13 and upgrading some old C++ code to
>> >> C++20. I had a small function for getting the current time based on
>> >> the <ctime> functions and tried to convert it to use the new <chrono>
>> >> functions. I was quite surprised that when using
>> >> chrono::current_zone(), my program stopped responding to the TZ
>> >> environment variable.
>> >>
>> >> I'd like to propose that chrono::current_zone() always pays attention
>> >> to $TZ so as not to surprise users moving from <ctime>. tzdb.cc
>> >> already has code for this gated by an #ifdef, so maybe all that needs
>> >> to be done is to make that code available to all platforms.
>> >>
>> >
>> > No, this is not a good idea. Libstdc++ cannot inspect the environment
>> > safely because another thread could be changing it concurrently, which
>> > would lead to undefined behaviour. The commented out code was an
>> attempt to
>> > support <chrono> on AIX which is unconventional and has no
>> /etc/localtime
>> > symlink.
>>
>> We can make getenv thread-safe in glibc.  Would that help?
>>
>
> In general, yes, but not here. That would be a glibc-specific feature, so
> not portable to other targets libstdc++ supports. And in any case, I don't
> think we want to depend on $TZ. That's not the intended design of the
> std::chrono API.
>
>
>>
>> It's still going to be quite slow, so maybe we need a new interface
>> after all.
>>
>> > In any case, the C++ standard requires that current_zone() refers to the
>> > computer's zone, not just the current process' TZ setting:
>> >
>> > "A pointer to the time zone which the computer has set as its local time
>> > zone."
>>
>> The notion that a computer has a single local time zone is strange.
>>
>
> Yes, and that's a flaw of <time.h>, but not a problem with <chrono>.
>
>
>> Surely C++ allows me to set the time zone even if the computer is
>> located in a country far away, and the machine owner has different
>> preferences?!
>>
>
>
> Yes, as I explained in the email you're replying to :-)
>
> A C++ program can use **any number** of different time zones. There is no
> preference given in the API to any particular time zone, unlike the libc
> <time.h> which depends on global state for "the time zone" (singular). The
> only time zone that has special treatment in the C++ API is UTC, which is
> the default time zone used by std::chrono::zoned_time<Dur> iff you don't
> construct it with a pointer to a specific time zone. But even that can be
> changed via chrono::zoned_traits.
>
> The chrono::current_zone() function is not used implicitly by the API, so
> if you choose to use it when it's not the time zone you want, **that's your
> fault**.
>
> If you want to use "Europe/London" as your program's default time zone,
> you can do that. If you want to use the time zone named by getenv("TZ") as
> your program's default time zone, you can do that.
>

And if you want to use "Europe/London" as the default but use
"Africa/Casablanca" in another part of the program and "Asia/Colombo", you
can also do that easily, without needing to change global state via tzset
etc.

You can do time zone calculations concurrently using all three of those
zones in different threads.

The lack of dependency on $TZ is a feature, not a bug.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-05-09  9:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-07 21:55 [RFC] libstdc++: chrono::current_zone() should always respect $TZ James Morris
2023-05-08  7:41 ` Jonathan Wakely
2023-05-08  8:06   ` Jonathan Wakely
2023-05-08  8:07   ` Jonathan Wakely
2023-05-09  8:54   ` Florian Weimer
2023-05-09  9:15     ` Jonathan Wakely
2023-05-09  9:20       ` Jonathan Wakely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).