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>

1 files changed, 35 insertions, 12 deletions

diff --git a/src/corelib/time/qdatetime.cpp b/src/corelib/time/qdatetime.cpp
index 4f7c3ff761..eb65d428ed 100644
--- a/src/corelib/time/qdatetime.cpp
+++ b/src/corelib/time/qdatetime.cpp
@@ -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)) {