On Wed, Mar 18, 2026 at 8:41 AM Tomasz Kaminski <[email protected]> wrote:
> > > On Tue, Mar 17, 2026 at 6:16 PM Jonathan Wakely <[email protected]> > wrote: > >> On Tue, 17 Mar 2026 at 13:18, Tomasz Kaminski <[email protected]> >> wrote: >> > >> > >> > >> > On Mon, Mar 16, 2026 at 11:55 PM Jonathan Wakely <[email protected]> >> wrote: >> >> >> >> The Australia/Broken_Hill example in PR libstdc++/116110 demonstrates a >> >> bug in the time zone code. The current code gives incorrect results >> when >> >> a zone changes from one named Rule to another during DST, e.g. the >> >> Broken_Hill time zone uses: >> >> >> >> Zone Australia/Broken_Hill 9:25:48 - LMT 1895 Feb >> >> 10 - AEST 1896 Aug 23 >> >> 9 - ACST 1899 May >> >> 9:30 AU AC%sT 1971 >> >> 9:30 AN AC%sT 2000 >> >> 9:30 AS AC%sT >> >> >> >> So the AN Rules take effect on 2000 Jan 1 which is during DST (which >> >> runs from October to March). >> > >> > Now, I understand the problem: the end of AS 2000 Jan 1, which according >> > to the application is: >> > R AS 1987 2007 - O lastSu 2s 1 D >> > However, we seem to be searching the wrong rule list because we are >> using >> > the "AN" rules to find previous value of DST, not the AS. This still >> gives us >> > expected save value, as I assume the rule switch is not meant to cause >> abrupt >> > changes to the actual wall time, and the changes in wall clock happen >> accordingly >> > to DST. >> >> Right. >> >> > >> > I.e. we make assumption that at the time of the rule change, the save >> value >> > is same according to new and old rules. >> > >> > So I think the code is technically incorrect, but practically correct. >> To be exact, >> > we would need look use the rule set from previous iterator in that case. >> >> I think we're handling this case wrong, but with the patch we give the >> right result for this time zone. >> >> The zic(8) man page says: >> >> A zone or continuation line L with a named rule set starts with >> standard >> time by default: that is, any of L's timestamps preceding L's >> earliest >> rule use the rule in effect after L's first transition into >> standard >> time. >> > I think that describes how the rule is interpreted for dates before its > first > entry, i.e. 1971 for both AN and AS: > R AN 1971 1985 - O lastSu 2s 1 D > R AN 1972 o - F 27 2s 0 S > R AN 1973 1981 - Mar Su>=1 2s 0 S > R AS 1971 1985 - O lastSu 2s 1 D > R AS 1986 o - O 19 2s 1 D > R AS 1987 2007 - O lastSu 2s 1 D > R AS 1972 o - F 27 2s 0 S > R AS 1973 1985 - Mar Su>=1 2s 0 S > > >> I think that means that we should not actually change from the AN rule >> to the AS rule on Jan 1st 2001, we should continue using the AN rule >> until the first transition to standard time of the AS rule. >> > Given that the entries may also switch the offset, I think they meant to > apply > until the end date, for example, the Kirimiti zone that switched between > sides > o International Day Line: > Z Pacific/Kiritimati -10:29:20 - LMT 1901 > -10:40 - %z 1979 O > -10 - %z 1994 D 31 > 14 - %z > Switch happened on 23:59:59 21th December of 1994. > Switch happened on 23:59:59 30th December of 1994. > > >> I'll see if that's reasonable to implement, because there could be >> cases where what this patch does is wrong e.g. if the AN and AS rules >> used different formats, say AC%sT and AC%sT2, then we should not >> change from ACDT to ACDT2 on Jan 1, but should continue using ACDT >> until the end of DST, then change to ACT, then change to ACDT2 in >> October 2001. It doesn't matter in this case because both rules use >> the same offset and the same format, so it's not detectable that we >> change to a new rule on Jan 1. >> >> >> >> >> >> >> >> >> The fix for this is to update info.offset and info.save when we find an >> >> active rule, instead of only updating the 'letters' variable. >> >> >> >> libstdc++-v3/ChangeLog: >> >> >> >> PR libstdc++/116110 >> >> * src/c++20/tzdb.cc (time_zone::_M_get_sys_info): Update >> >> info.offset and info.save to values from the active rule. >> >> * testsuite/std/time/time_zone/116110.cc: New test. >> >> --- >> >> >> >> Tested x86_64-linux. >> > >> > Add the explanation of the assumption that save value at rule change is >> same >> > as with previous rules, and we are OK. The comment before ri.offset() >> would >> > benefit from update. >> >> >> >> >> >> libstdc++-v3/src/c++20/tzdb.cc | 6 ++++- >> >> .../testsuite/std/time/time_zone/116110.cc | 22 +++++++++++++++++++ >> >> 2 files changed, 27 insertions(+), 1 deletion(-) >> >> create mode 100644 libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> >> >> >> diff --git a/libstdc++-v3/src/c++20/tzdb.cc >> b/libstdc++-v3/src/c++20/tzdb.cc >> >> index 53441880ae6e..e26a9ee38072 100644 >> >> --- a/libstdc++-v3/src/c++20/tzdb.cc >> >> +++ b/libstdc++-v3/src/c++20/tzdb.cc >> >> @@ -917,7 +917,11 @@ namespace std::chrono >> > >> > >> > >> > >> > This change is inside if and can be completely omitted: >> > string_view letters; >> > if (i != infos.begin() && i[-1].expanded()) >> > letters = i[-1].next_letters(); >> > >> > if (letters.empty()) >> > { >> > And I wondered if caching the next_letters is causing any trouble >> > here (i.e. we should have also handle saving in this situation), however >> > The code below ensures that list of expanded zones always ends with the >> > save == 0, so it is correct. >> > >> > >> > >> >> >> >> } >> >> >> >> if (active_rule) >> >> - letters = active_rule->letters; >> >> + { >> >> + info.offset = ri.offset() + active_rule->save; >> >> + info.save = >> chrono::duration_cast<minutes>(active_rule->save); >> >> + letters = active_rule->letters; >> >> + } >> >> else if (first_std) >> >> letters = first_std->letters; >> >> } >> >> diff --git a/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> >> new file mode 100644 >> >> index 000000000000..b52e83b293cc >> >> --- /dev/null >> >> +++ b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc >> >> @@ -0,0 +1,22 @@ >> >> +// { dg-do run { target c++20 } } >> >> +// { dg-require-effective-target tzdb } >> >> +// { dg-require-effective-target cxx11_abi } >> >> + >> >> +#include <chrono> >> >> +#include <testsuite_hooks.h> >> >> + >> >> +void >> >> +test_broken_hill() >> >> +{ >> >> + using namespace std::chrono; >> >> + auto* tz = locate_zone("Australia/Broken_Hill"); >> >> >> >> + auto info = tz->get_info(sys_days(2000y/February/29d) + 23h + 23min >> + 23s); >> >> + VERIFY( info.offset == 630min ); >> >> + VERIFY( info.save == 60min ); >> >> + VERIFY( info.abbrev == "ACDT" ); >> >> +} >> >> + >> >> +int main() >> >> +{ >> >> + test_broken_hill(); >> >> +} >> >> -- >> >> 2.53.0 >> >> >> >>
