The previous patches for PR 116110 left one case unresolved: a Zone
line with a wall-time UNTIL whose RULES field is a named rule set.
The save value used to convert the wall UNTIL to UTC depends on which
rule of the set was active at the UNTIL instant, but at parse time
the rule records have not all been loaded, so the active-rule lookup
can't yet be performed.
The remaining FIXME in operator>>(istream&, ZoneInfo&) caused zones
like Africa/Algiers (around 1977-10-21) to place their zone-line
boundary one save-period off from the canonical zic interpretation,
producing brief incorrect sys_info windows during DST transitions.
This commit defers the save adjustment to _M_get_sys_info call,
and applies it only for the ZoneInfo surrounding the specified
time. This assumes that applying the save does not change relative
order of ZoneInfo (i.e. the until members are spaced more than save).
The m_expanded member is replaced with four state _M_state setting:
Expanded, and three separte state used for rule based zones:
* SaveKnown - m_save contains save value at time m_until
* SavePending - m_save is defualted to zero, m_until is correct
* UntilPending - as above, but also m_until time need to adjusted
The parser set the UntilPending state when it sees a wall-time
UNTIL on a named-rule line, and SaveKnown if not UNTIL date is
specified.
The fixup in _M_get_sys_info, is performed by calc_save function
on previous/current zone. If necessary (depending on m_state) this
function updates m_save and m_until members, and returns true
if the m_until was shifted in the process. In such case, we redo
the comparision of until() against tp, and move iterator to active
zone info if necessary. The save at boundary for previous/current
ZoneInfo is required to implement proper zone merging, so it is
always computed.
The active-rule lookup either reuses the existing find_active_rule
overload accepting sys_seconds (if UNTIL is not affected by save),
or newly introduced overload accepting local_seconds (local time).
The local_seconds versions, follows the same logic for finding
rule transitions (extracted to find_surrounding_transitions)
surrounding the sys_time. The final active rule, is then determined
after converting the transition times to local time, including the
running save.
The test_apia case in 116110.cc had a hardcoded `+11h` workaround
for the unfixed bug; with this fix in place the workaround is removed
and the value becomes the canonical `+10h`.
libstdc++-v3/ChangeLog:
PR libstdc++/116110
* src/c++20/tzdb.cc (ZoneInfo::calc_save, ZoneInfo::State): Define.
(ZoneInfo::m_expanded): Replaced with m_state.
(ZoneInfo::m_pos): Reduce the bit with to 14.
(ZoneInfo::m_state): Expands m_expanded with four state enum.
(ZoneInfo::ZoneInfo, ZoneInfo::expanded, ZoneInfo::set_abbrev):
Replace m_expanded usage with m_state and State::Expanded.
(Transitions, find_surrounding_transitions): Extracted from
find_active_rule.
(find_active_rule): Define overload for local_seconds. Implement
both in terms of find_surrounding_transitions.
(time_zone::_M_get_sys_info): Use ZoneInfo::calc_save to adjust
until() on surrounding zones, and shift the current zone iterator
(i) accordingly.
(operator>>(istream&, ZoneInfo&)): Set m_state for UntilPending
for UNTIL time using wall time, and SaveKnown for lines without
end date. For clarity, set m_save to 0 if daylight is not used.
* testsuite/std/time/time_zone/116110.cc (test_apia): Use correct
value of the the total offset.
testsuite/std/time/time_zone/pr116110_named.cc: New test.
Co-authored-by: Álvaro Begué <[email protected]>
Signed-off-by: Álvaro Begué <[email protected]>
Signed-off-by: Tomasz Kamiński <[email protected]>
---
v3 rewrites the implementation. The major change is the fact that
save-adjustment is now applied lazily and locally to the neighbooring
zones. This have strong assumtion about zone changes being resonably
spaced (i.e. they remain sorted after save adjustment). After considering
that I believe this is resonable, as we are discussing save, and not
offset (that may jump for 24h).
The original v3 also changed the code to find active rule at time of
transition, to include transition happening exaclty at info begin.
I have dropped it for this version of the patch, as it was not
necessary to pass included test, and caused another failures.
We should revisit the comparision against the zic result once all
fixes are in place.
Testing on x86_64-linux. All *time_zone* test passes.
We should also see the performance impact before any decision to
backport.
libstdc++-v3/src/c++20/tzdb.cc | 221 +++++++++++++-----
.../testsuite/std/time/time_zone/116110.cc | 4 +-
.../std/time/time_zone/pr116110_named.cc | 74 ++++++
3 files changed, 244 insertions(+), 55 deletions(-)
create mode 100644 libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc
diff --git a/libstdc++-v3/src/c++20/tzdb.cc b/libstdc++-v3/src/c++20/tzdb.cc
index 998a363151d..09c2127b242 100644
--- a/libstdc++-v3/src/c++20/tzdb.cc
+++ b/libstdc++-v3/src/c++20/tzdb.cc
@@ -483,6 +483,8 @@ namespace std::chrono
select_std_or_dst_abbrev(info.abbrev, info.save);
}
+ struct Rule;
+
// A time zone information record.
// Zone NAME STDOFF RULES FORMAT [UNTIL]
// Zone Asia/Amman 2:00 Jordan EE%sT 2017 Oct 27 01:00
@@ -492,12 +494,12 @@ namespace std::chrono
ZoneInfo() = default;
ZoneInfo(sys_info&& info)
- : m_buf(std::move(info.abbrev)), m_expanded(true), m_save(info.save),
+ : m_buf(std::move(info.abbrev)), m_state(Expanded), m_save(info.save),
m_offset(info.offset - seconds(info.save)), m_until(info.end)
{ }
ZoneInfo(const pair<sys_info, string_view>& info)
- : m_expanded(true), m_save(info.first.save),
+ : m_state(Expanded), m_save(info.first.save),
m_offset(info.first.offset - seconds(info.first.save)),
m_until(info.first.end)
{
@@ -533,17 +535,23 @@ namespace std::chrono
sys_seconds
until() const noexcept { return m_until; }
+ // For non-expanded zone (using rules) computes the value
+ // of 'save' at the time of transitions, caches it in m_save.
+ // Returns true if until value was changed in the process.
+ bool
+ calc_save(span<const Rule> all_rules) noexcept;
+
friend istream& operator>>(istream&, ZoneInfo&);
bool
expanded() const noexcept
- { return m_expanded; }
+ { return m_state == Expanded; }
// For an expanded ZoneInfo this returns the LETTERS that apply to the
// **next** sys_info after this one.
string_view
next_letters() const noexcept
- { return m_expanded ? rules() : string_view{}; }
+ { return (m_state == Expanded) ? rules() : string_view{}; }
bool
@@ -551,7 +559,7 @@ namespace std::chrono
{
// If this object references a named Rule then we can't populate
// a sys_info yet.
- if (!m_expanded)
+ if (m_state != Expanded)
return false;
info.end = until();
@@ -565,12 +573,16 @@ namespace std::chrono
private:
friend class time_zone;
+ enum class State : uint_least16_t
+ { Expanded, SaveKnown, SavePending, UntilPending };
+ using enum State;
+
void
set_abbrev(string abbrev)
{
m_buf = std::move(abbrev);
m_pos = 0;
- m_expanded = true;
+ m_state = Expanded;
}
void
@@ -586,8 +598,8 @@ namespace std::chrono
}
string m_buf; // rules() + ' ' + format() OR letters + ' ' + format()
- uint_least16_t m_pos : 15 = 0; // offset of format() in m_buf
- uint_least16_t m_expanded : 1 = 0;
+ uint_least16_t m_pos : 14 = 0; // offset of format() in m_buf
+ State m_state : 2 = SavePending;
duration<int_least16_t, ratio<60>> m_save{};
sec32_t m_offset{};
sys_seconds m_until{};
@@ -695,14 +707,27 @@ namespace std::chrono
#endif
};
- const Rule*
- find_active_rule(span<const Rule> rules, sys_seconds t, seconds std_offset)
+ struct Transitions
{
- struct Transition {
+ struct Entry
+ {
const Rule* rule;
sys_seconds when;
};
+ Entry prev{nullptr, sys_seconds::min()};
+ Entry curr{nullptr, sys_seconds::min()};
+ Entry next{nullptr, sys_seconds::max()};
+ };
+
+ // Collect transitions of rules surrounding specified time t:
+ // * .curr - rule active directly before or at t,
+ // * .prev - rule transition before trans.curr
+ // * .next - rule transition directly after t
+ Transitions
+ find_surrounding_transitions(span<const Rule> rules, sys_seconds t,
+ seconds std_offset)
+ {
const year_month_day date(chrono::floor<days>(t));
// Expand the search window for a time near the end
// of the year which can be pushed to next/previous
@@ -716,16 +741,7 @@ namespace std::chrono
if (date.month() == January && date.day() == day(1))
--first_year;
- // Rule specifying start time as Wall time, should apply
- // running 'save' accumulated by earlier rules. To handle
- // that we firstly collect transitions surrounding specified
- // time t, ignoring the 'save':
- // * curr_tran - rule active directly before or at t,
- // * prev_tran - rule transition before curr_tran
- // * next_tran - rule transition directly after t
- Transition prev_tran{nullptr, sys_seconds::min()};
- Transition curr_tran{nullptr, sys_seconds::min()};
- Transition next_tran{nullptr, sys_seconds::max()};
+ Transitions trans;
for (const auto& rule : rules)
{
if (last_year < rule.from) // Rule doesn't apply yet at time t.
@@ -761,7 +777,7 @@ namespace std::chrono
if (first_year > rule.to)
// Rule no longer applies at time t, record last transition
- start_before = rule.start_time(rule.to, offset);
+ start_before = rule.start_time(rule.to, offset);
else if (first_year == last_year)
for_year(first_year);
else
@@ -773,24 +789,35 @@ namespace std::chrono
for_year(last_year);
}
- if (curr_tran.when < start_before)
+ if (trans.curr.when < start_before)
{
- prev_tran = curr_tran;
- curr_tran = {&rule, start_before};
+ trans.prev = trans.curr;
+ trans.curr = {&rule, start_before};
}
- else if (prev_tran.when < start_before)
- prev_tran = {&rule, start_before};
+ else if (trans.prev.when < start_before)
+ trans.prev = {&rule, start_before};
- if (start_after < next_tran.when)
- next_tran = {&rule, start_after};
+ if (start_after < trans.next.when)
+ trans.next = {&rule, start_after};
}
+ return trans;
+ }
+
+ const Rule*
+ find_active_rule(span<const Rule> rules, sys_seconds t, seconds std_offset)
+ {
+ // Rule specifying start time as Wall time, should apply
+ // running 'save' accumulated by earlier rules. To handle
+ // that we firstly collect transitions surrounding specified
+ // time t:
+ auto trans = find_surrounding_transitions(rules, t, std_offset);
// No rule was active at the time of t, running 'save'
// cannot change this output, as we have no save to apply.
- if (!curr_tran.rule)
+ if (!trans.curr.rule)
return nullptr;
- auto cascade_save = [](const Rule* from, Transition& to)
+ auto cascade_save = [](const Rule* from, Transitions::Entry& to)
{
if (!from || from->save == seconds(0))
return false;
@@ -800,19 +827,49 @@ namespace std::chrono
return true;
};
- if (cascade_save(curr_tran.rule, next_tran))
- // Running save moved what we considered next_tran to time
- // before or at t, in that case next_tran is active rule.
- if (next_tran.when <= t)
- return next_tran.rule;
+ if (cascade_save(trans.curr.rule, trans.next))
+ // Running save moved what we considered trans.next to time
+ // before or at t, in that case trans.next is active rule.
+ if (trans.next.when <= t)
+ return trans.next.rule;
- if (cascade_save(prev_tran.rule, curr_tran))
- // Running save moved what we consider curr_tran to
- // time after t, in that case prev_tran is active rule.
- if (curr_tran.when > t)
- return prev_tran.rule;
+ if (cascade_save(trans.prev.rule, trans.curr))
+ // Running save moved what we consider trans.curr to
+ // time after t, in that case trans.prev is active rule.
+ if (trans.curr.when > t)
+ return trans.prev.rule;
- return curr_tran.rule;
+ return trans.curr.rule;
+ }
+
+ const Rule*
+ find_active_rule(span<const Rule> rules, local_seconds t, seconds
std_offset)
+ {
+ // UNTIL or START time in local time should take a 'save' from
+ // active rule. To handle approximate system time at, and
+ // collection transitions surrounding it:
+ const sys_seconds at(t.time_since_epoch() - std_offset);
+ const auto trans = find_surrounding_transitions(rules, at, std_offset);
+
+ auto to_local = [&](const Transitions::Entry& tran, const Rule* before)
+ {
+ local_seconds ls(tran.when.time_since_epoch() + std_offset);
+ if (!before || !tran.rule)
+ return ls;
+ if (tran.rule->when.indicator == at_time::Wall)
+ // 'when' was converted from local time without considering
+ // running 'save' in first place
+ return ls;
+ return ls + before->save;
+ };
+
+ // Translate the transitions time to local time, taking 'save'
+ // into consideration, and recheck active rule.
+ if (to_local(trans.next, trans.curr.rule) <= t)
+ return trans.next.rule;
+ if (to_local(trans.curr, trans.prev.rule) > t)
+ return trans.prev.rule;
+ return trans.curr.rule;
}
const Rule*
@@ -835,6 +892,43 @@ namespace std::chrono
}
return first;
}
+
+ bool
+ ZoneInfo::calc_save(span<const Rule> all_rules) noexcept
+ {
+ // Expanded rule, or the save value was already computed.
+ if (m_state < SavePending)
+ return false;
+
+ // Find the rules named by rules()
+ span<const Rule> sel_rules
+ = ranges::equal_range(all_rules, rules(), ranges::less{}, &Rule::name);
+ if (sel_rules.empty())
+ __throw_runtime_error("std::chrono::time_zone::get_info: invalid data");
+
+ if (m_state == UntilPending)
+ {
+ // UNTIL was specified in Wall time, so it is affected by 'save'.
+ // Convert it back to local time, and find active rule using that.
+ const local_seconds lu(m_until.time_since_epoch() + m_offset);
+ if (const Rule* rule = find_active_rule(sel_rules, lu - seconds(1),
m_offset))
+ if (rule->save.count() != 0)
+ {
+ m_state = SaveKnown;
+ m_save = duration_cast<minutes>(rule->save);
+ m_until -= rule->save;
+ return true;
+ }
+ }
+ else
+ // UNTIL was either specified in Universal time, or Standard
+ // time (known total offset). m_until is corresponding sys_time point.
+ if (const Rule* rule = find_active_rule(sel_rules, m_until -
seconds(1), m_offset))
+ m_save = duration_cast<minutes>(rule->save);
+
+ m_state = SaveKnown;
+ return false;
+ }
} // namespace
#endif // TZDB_DISABLED
@@ -966,19 +1060,35 @@ namespace std::chrono
// Find the transition info for the time point.
auto i = ranges::upper_bound(infos, tp, ranges::less{}, &ZoneInfo::until);
+ // Perform the comparision on save adjusted until values (if needed)
+ // Assumme that applying the save will not change relative order of
+ // ZoneInfo objects.
+ if (i != infos.begin() && i[-1].calc_save(node->rules) && (i[-1].until() >
tp))
+ --i;
+ else if (i != infos.end() && i->calc_save(node->rules) && (i->until() <=
tp))
+ ++i;
+
if (i == infos.end())
{
if (infos.empty())
__throw_runtime_error("std::chrono::time_zone::get_info: invalid
data");
- tp = (--i)->until();
- }
+ (--i)->calc_save(node->rules);
+ tp = i->until();
+ }
+ else // Guarantee that i->until() is correct
+ i->calc_save(node->rules);
+
sys_info info;
if (i == infos.begin())
info.begin = sys_days(year::min()/January/1);
else
- info.begin = i[-1].until();
+ {
+ ZoneInfo& prev = i[-1];
+ prev.calc_save(node->rules);
+ info.begin = prev.until();
+ }
if (i->to(info)) // We already know a sys_info for this time.
return info;
@@ -990,10 +1100,10 @@ namespace std::chrono
const ZoneInfo& ri = *i;
// Find the rules named by ri.rules()
- auto rules = ranges::equal_range(node->rules, ri.rules(),
- ranges::less{}, &Rule::name);
+ span<const Rule> rules = ranges::equal_range(node->rules, ri.rules(),
+ ranges::less{}, &Rule::name);
- if (ranges::empty(rules))
+ if (rules.empty())
__throw_runtime_error("std::chrono::time_zone::get_info: invalid data");
vector<pair<sys_info, string_view>> new_infos;
@@ -1012,8 +1122,6 @@ namespace std::chrono
// rule changes always occur between periods of standard time.
info.offset = ri.offset();
info.save = 0min;
- // XXX The ri.until() time point should be
- // "interpreted using the rules in effect just before the transition"
info.end = ri.until();
info.abbrev = ri.format();
@@ -2612,6 +2720,7 @@ constinit tzdb_list::_Node::NumLeapSeconds
tzdb_list::_Node::num_leap_seconds;
if (rules == "-")
{
// Standard time always applies, no DST.
+ inf.m_save = minutes(0);
}
else
{
@@ -2645,14 +2754,20 @@ constinit tzdb_list::_Node::NumLeapSeconds
tzdb_list::_Node::num_leap_seconds;
inf.m_until -= seconds(inf.m_offset);
if (t.indicator != at_time::Standard)
{
- if (inf.m_expanded) // Not a named Rule, SAVE is known now.
+ if (inf.expanded()) // Not a named Rule, SAVE is known now.
inf.m_until -= inf.m_save;
- // else Named Rule, SAVE is unknown. FIXME: PR 116110
+ else // else Named Rule, SAVE is unknown, mark as pending
+ inf.m_state = ZoneInfo::UntilPending;
}
}
}
else
- inf.m_until = sys_days(year::max()/December/31);
+ {
+ inf.m_until = sys_days(year::max()/December/31);
+ // The line does not define UNTIL, so it is not affected by save
+ if (!inf.expanded())
+ inf.m_state = ZoneInfo::SaveKnown;
+ }
in.clear(in.rdstate() & ios::eofbit);
in.exceptions(ex);
diff --git a/libstdc++-v3/testsuite/std/time/time_zone/116110.cc
b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc
index 26b9ba33c31..1ec01d35955 100644
--- a/libstdc++-v3/testsuite/std/time/time_zone/116110.cc
+++ b/libstdc++-v3/testsuite/std/time/time_zone/116110.cc
@@ -65,8 +65,8 @@ test_apia()
auto* tz = locate_zone("Pacific/Apia");
local_seconds t = local_days(2011y/December/29) + 24h;
- // FIXME: this should be + 10h but we do not account for DST yet, so + 11h.
- sys_seconds ut(t.time_since_epoch() + 11h );
+ // The wall UNTIL is interpreted in the prior offset (-11h + 1h)
+ sys_seconds ut(t.time_since_epoch() + 10h);
sys_info info;
info = tz->get_info(ut - 1s);
VERIFY( info.offset == (-11h + info.save) );
diff --git a/libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc
b/libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc
new file mode 100644
index 00000000000..b3bf4eb1aa9
--- /dev/null
+++ b/libstdc++-v3/testsuite/std/time/time_zone/pr116110_named.cc
@@ -0,0 +1,74 @@
+// { dg-do run { target c++20 } }
+// { dg-require-effective-target tzdb }
+// { dg-require-effective-target cxx11_abi }
+// { dg-xfail-run-if "no weak override on AIX" { powerpc-ibm-aix* } }
+
+// Africa/Algiers 1977-10-21: a Zone line whose RULES references a
+// named Rule and whose UNTIL is a wall-time expression. The wall
+// UNTIL is interpreted using the SAVE value in force just before the
+// boundary (the May-6 rule's save=1, not the Oct-21 rule's save=0
+// even though the Oct-21 rule fires at the same wall instant).
+//
+// Rule d 1977 May 6 0:00 wall save=1
+// Rule d 1977 Oct 21 0:00 wall save=0
+// Z A 0 d WE%sT 1977 O 21
+// 1 d CE%sT
+
+#include <chrono>
+#include <fstream>
+#include <testsuite_hooks.h>
+
+static bool override_used = false;
+
+namespace __gnu_cxx
+{
+ const char* zoneinfo_dir_override() {
+ override_used = true;
+ return "./";
+ }
+}
+
+int
+main()
+{
+ using namespace std::chrono;
+
+ std::ofstream("tzdata.zi") << R"(# version test_pr116110_named
+R d 1977 o - May 6 0 1 S
+R d 1977 o - O 21 0 0 -
+Z Test/Algiers 0 d WE%sT 1977 O 21
+ 1 d CE%sT
+)";
+
+ const auto& db = reload_tzdb();
+ VERIFY( override_used );
+ VERIFY( db.version == "test_pr116110_named" );
+
+ auto* tz = locate_zone("Test/Algiers");
+
+ // Just before the boundary: still in the first Zone line under
+ // the May-6 rule (save=1, WEST, total +1).
+ auto pre = tz->get_info(sys_days{1977y/October/20} + 22h);
+ VERIFY( pre.offset == 1h );
+ VERIFY( pre.save == 1h );
+ VERIFY( pre.abbrev == "WEST" );
+
+ // The boundary is Oct 20 23:00 UTC (= wall 00:00 - stdoff(0) - save(1)).
+ // At and after the boundary we are in the second line (CET).
+ auto at = tz->get_info(sys_days{1977y/October/20} + 23h);
+ VERIFY( at.offset == 1h ); // stdoff 1 + save 0 (CET, second line)
+ VERIFY( at.save == 0min );
+ VERIFY( at.abbrev == "CET" );
+
+ // A second query inside the second line, well clear of the boundary.
+ auto after = tz->get_info(sys_days{1977y/October/21} + 12h);
+ VERIFY( after.offset == 1h );
+ VERIFY( after.save == 0min );
+ VERIFY( after.abbrev == "CET" );
+
+ // A query inside the [Oct 20 23:00, Oct 21 00:00] UTC window must be
+ // in the second line, not in a leftover stretch from the first line.
+ auto window = tz->get_info(sys_days{1977y/October/20} + 23h + 30min);
+ VERIFY( window.offset == 1h );
+ VERIFY( window.abbrev == "CET" );
+}
--
2.55.0
