Adjust msecs instead of offset for spring-forward resolution times

The resolution selects a point in time outside the gap, which will be
represented by toMSecsSinceEpoch()'s return, despite the QDT object's
isValid() returning false. Previously we retained the
originally-calculated msecs, so as to keep date() and time() matching
what was asked for. However, this required adjusting offset, which was
not remembered for local times within 2^55 milliseconds of the start
of 1970. This lead to an inconsistency between the offset from UTC
reported for the resolution for a local time further from the epoch,
or for a time-zone, and the actual offset from UTC at the time
indicated by the return from toMSecsSinceEpoch().

Instead, retain the actually calculated offset (even if we aren't
going to remember it) and adjust the msecs to the value that ensures
toMSecsSinceEpoch() will get the selected resolution. This
incidentally means that, when toMSecsSinceEpoch() has to re-resolve
(for a local time within 2^55 msecs of the epoch), it avoids
revisiting the complications of hitting the gap.

In passing, change internal stateAtMillis() to take the QTimeZone it
is passed by const reference, to save a copy (noticed during debug).
Also tweak a comment in a test to be explicit about a default value.

[ChangeLog][QtCore][Possibly Significant Behavior Change] When
QDateTime is instantiated for a combination of date and time that was
skipped, by local time or a time-zone, for example during a
spring-forward DST transition, the invalid result's time() - and, in
rare cases, date() - no longer match what was asked for. Instead,
these values and offsetFromUtc() now match the point in time
identified by toMSecsSinceEpoch().

Change-Id: Id61c4274b365750f56442a4a598be5c14cfca689
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
This commit is contained in:
Edward Welbourne 2023-08-18 13:53:58 +02:00
parent 217c607782
commit 4aba97e062
2 changed files with 86 additions and 21 deletions

View File

@ -2812,18 +2812,13 @@ QDateTimePrivate::ZoneState QDateTimePrivate::zoneStateAtMillis(const QTimeZone
if (data.offsetFromUtc == QTimeZonePrivate::invalidSeconds())
return {millis};
Q_ASSERT(zone.d->offsetFromUtc(data.atMSecsSinceEpoch) == data.offsetFromUtc);
ZoneState state(data.atMSecsSinceEpoch + data.offsetFromUtc * MSECS_PER_SEC,
data.offsetFromUtc,
data.daylightTimeOffset ? DaylightTime : StandardTime);
// Revise offset, when stepping out of a spring-forward, to what makes a
// fromMSecsSinceEpoch(toMSecsSinceEpoch()) of the resulting QDT work:
if (millis != state.when)
state.offset += (millis - state.when) / MSECS_PER_SEC;
return state;
return ZoneState(data.atMSecsSinceEpoch + data.offsetFromUtc * MSECS_PER_SEC,
data.offsetFromUtc,
data.daylightTimeOffset ? DaylightTime : StandardTime);
}
#endif // timezone
static inline QDateTimePrivate::ZoneState stateAtMillis(QTimeZone zone, qint64 millis,
static inline QDateTimePrivate::ZoneState stateAtMillis(const QTimeZone &zone, qint64 millis,
QDateTimePrivate::DaylightStatus dst)
{
if (zone.timeSpec() == Qt::LocalTime)
@ -2949,6 +2944,15 @@ static void refreshZonedDateTime(QDateTimeData &d, const QTimeZone &zone)
auto status = getStatus(d);
Q_ASSERT(extractSpec(status) == zone.timeSpec());
int offsetFromUtc = 0;
/* Callers are:
* QDTP::create(), where d is too new to be shared yet
* reviseTimeZone(), which detach()es if not short before calling this
* checkValidDateTime(), always follows a setDateTime() that detach()ed if not short
So we can assume d is not shared. We only need to detach() if we convert
from short to pimpled to accommodate an oversize msecs, which can only be
needed in the unlikely event we revise it.
*/
// If not valid date and time then is invalid
if (!status.testFlags(QDateTimePrivate::ValidDate | QDateTimePrivate::ValidTime)) {
@ -2961,14 +2965,33 @@ static void refreshZonedDateTime(QDateTimeData &d, const QTimeZone &zone)
QDateTimePrivate::ZoneState state = stateAtMillis(zone, msecs,
extractDaylightStatus(status));
// Save the offset to use in offsetFromUtc() &c., even if the next check
// marks invalid; this lets fromMSecsSinceEpoch() give a useful fallback
// marks invalid; this lets toMSecsSinceEpoch() give a useful fallback
// for times in spring-forward gaps.
offsetFromUtc = state.offset;
Q_ASSERT(!state.valid || (state.offset >= -SECS_PER_DAY && state.offset <= SECS_PER_DAY));
if (state.valid && msecs == state.when)
if (Q_LIKELY(state.valid && msecs == state.when)) {
status = mergeDaylightStatus(status | QDateTimePrivate::ValidDateTime, state.dst);
else // msecs changed or failed to convert (e.g. overflow)
} else { // msecs changed: gap, or failed to convert (e.g. overflow)
status.setFlag(QDateTimePrivate::ValidDateTime, false);
if (state.valid) { // gap
/* Make sure our offset and msecs do produce the selected UTC
secs, if queried. When d isn't short, we record offset, so
need msecs to match; when d is short, consistency demands we
also update msecs, which will at least mean we don't hit the
gap again, if we ever recompute offset. */
if (status.testFlag(QDateTimePrivate::ShortData)) {
if (msecsCanBeSmall(state.when)) {
d.data.msecs = qintptr(state.when);
} else {
// Convert to long-form so we can hold the revised msecs:
status.setFlag(QDateTimePrivate::ShortData, false);
d.detach();
}
}
if (!status.testFlag(QDateTimePrivate::ShortData))
d->m_msecs = state.when;
}
}
}
if (status.testFlag(QDateTimePrivate::ShortData)) {

View File

@ -2297,6 +2297,8 @@ void tst_QDateTime::springForward()
});
QVERIFY(off == 1 || off == -1);
report.dismiss();
// adjust is the offset on the other side of the gap:
QCOMPARE(direct.offsetFromUtc(), (adjust + off * 60) * 60);
}
// Repeat, but getting there via .toTimeZone(). Apply adjust to datetime,
@ -2306,10 +2308,12 @@ void tst_QDateTime::springForward()
QCOMPARE(detour.time(), time);
detour = detour.addDays(step);
// Insist on consistency:
if (direct.isValid())
if (direct.isValid()) {
QCOMPARE(detour, direct);
else
QCOMPARE(detour.offsetFromUtc(), direct.offsetFromUtc());
} else {
QVERIFY(!detour.isValid());
}
}
void tst_QDateTime::operator_eqeq_data()
@ -3129,8 +3133,8 @@ void tst_QDateTime::fromStringStringFormat_data()
QTimeZone southBrazil("America/Sao_Paulo");
if (southBrazil.isValid()) {
QTest::newRow("spring-forward-midnight")
// NB: no hour field, so hour takes its default, so that default can
// be over-ridden:
// NB: no hour field, so hour takes its default, 0, so that default
// can be over-ridden:
<< QString("2008-10-19 23:45.678 America/Sao_Paulo")
<< QString("yyyy-MM-dd mm:ss.zzz t")
// That's in the hour skipped - expect the matching time after the
@ -3706,7 +3710,7 @@ void tst_QDateTime::daylightTransitions() const
QDateTime missing(QDate(2012, 3, 25), QTime(2, 0));
QVERIFY(!missing.isValid());
QCOMPARE(missing.date(), QDate(2012, 3, 25));
QCOMPARE(missing.time(), QTime(2, 0));
QCOMPARE(missing.time(), QTime(3, 0));
// datetimeparser relies on toMSecsSinceEpoch to still work:
QCOMPARE(missing.toMSecsSinceEpoch(), spring2012);
@ -3739,11 +3743,11 @@ void tst_QDateTime::daylightTransitions() const
utc.setTimeZone(QTimeZone::LocalTime);
QVERIFY(!utc.isValid());
QCOMPARE(utc.date(), QDate(2012, 3, 25));
QCOMPARE(utc.time(), QTime(2, 0));
QCOMPARE(utc.time(), QTime(3, 0));
utc.setTimeZone(UTC);
QVERIFY(utc.isValid());
QCOMPARE(utc.date(), QDate(2012, 3, 25));
QCOMPARE(utc.time(), QTime(2, 0));
QCOMPARE(utc.time(), QTime(3, 0));
// Test date maths, if result falls in missing hour then becomes next
// hour (or is always invalid; mktime() may reject gap-times).
@ -4212,12 +4216,50 @@ void tst_QDateTime::timeZones() const
QDateTime inGap = QDateTime(QDate(2013, 3, 31), QTime(2, 0), cet);
QVERIFY(!inGap.isValid());
QCOMPARE(inGap.date(), QDate(2013, 3, 31));
QCOMPARE(inGap.time(), QTime(2, 0));
QCOMPARE(inGap.time(), QTime(3, 0));
QCOMPARE(inGap.offsetFromUtc(), 7200);
// - Test transition hole, setting 02:59:59.999 is invalid
inGap = QDateTime(QDate(2013, 3, 31), QTime(2, 59, 59, 999), cet);
QVERIFY(!inGap.isValid());
QCOMPARE(inGap.date(), QDate(2013, 3, 31));
QCOMPARE(inGap.time(), QTime(2, 59, 59, 999));
QCOMPARE(inGap.time(), QTime(3, 59, 59, 999));
QCOMPARE(inGap.offsetFromUtc(), 7200);
// Test similar for local time, if it's CET:
if (zoneIsCET) {
inGap = QDateTime(QDate(2013, 3, 31), QTime(2, 30));
QVERIFY(!inGap.isValid());
QCOMPARE(inGap.date(), QDate(2013, 3, 31));
QCOMPARE(inGap.time(), QTime(3, 30));
}
// Test a gap more than 1'141'707.91-years from 1970, outside ShortData's range,
// The zone version is non-short in any case, but check it anyway.
// However, we can only test this if the underlying OS believes CET continues
// exercising DST indefinitely; Darwin, for example, assumes we'll have all
// kicked the habit by the end of 2100.
constexpr int longYear = 1'143'678;
constexpr qint64 millisInWeek = qint64(7) * 24 * 60 * 60 * 1000;
if (QDateTime(QDate(longYear, 3, 24), QTime(12, 0), cet).msecsTo(
QDateTime(QDate(longYear, 3, 31), QTime(12, 0), cet)) < millisInWeek) {
inGap = QDateTime(QDate(longYear, 3, 27), QTime(2, 30), cet);
QVERIFY(!inGap.isValid());
QCOMPARE(inGap.date(), QDate(longYear, 3, 27));
QCOMPARE(inGap.time(), QTime(3, 30));
QCOMPARE(inGap.offsetFromUtc(), 7200);
} else {
qDebug("Skipping far-future check beyond zoned end of DST");
}
if (zoneIsCET && QDateTime(QDate(longYear, 3, 24), QTime(12, 0)).msecsTo(
QDateTime(QDate(longYear, 3, 31), QTime(12, 0))) < millisInWeek) {
inGap = QDateTime(QDate(longYear, 3, 27), QTime(2, 30));
QVERIFY(!inGap.isValid());
QCOMPARE(inGap.date(), QDate(longYear, 3, 27));
QCOMPARE(inGap.offsetFromUtc(), 7200);
QCOMPARE(inGap.time(), QTime(3, 30));
} else {
qDebug(zoneIsCET ? "Skipping far-future check beyond local end of DST"
: "Skipping CET-specific test");
}
// Standard Time to Daylight Time 2013 on 2013-10-27 is 3:00 local time / 1:00 UTC
const qint64 replayMSecs = 1382835600000;