Shortcut QDateTime comparison when difference is large

We want to avoid caling toMSecsSinceEpoch() since it's expensive for
LocalTime (which is presumed to be the common case). We can do so when
both sides have the same offset from UTC (and this can cheaply be
determined) but that's no help for two local times months apart, one
in DST the other not. However, in this case, the difference in millis
is big enough that no plausible difference in offset can overcome it,
so we can again avoid toMSecsSinceEpoch() and simply compare millis.
This should make some previously-expensive comparisons cheap.

Add test-cases to the QDateTime ordering test that verify this doesn't
lead to mis-comparison at the biggest offset-difference known.

Pick-to: 6.8
Fixes: QTBUG-131491
Change-Id: I1afd5d058c8663c908f898d4c50d0837549b87db
Reviewed-by: Christian Ehrlicher <[email protected]>

src/corelib/time/qdatetime.cpp

@@ -3143,7 +3143,7 @@ static inline Qt::TimeSpec getSpec(const QDateTimeData &d)
 /* True if we *can cheaply determine* that a and b use the same offset.
    If they use different offsets or it would be expensive to find out, false.
    Calls to toMSecsSinceEpoch() are expensive, for these purposes.
-   See QDateTime's comparison operators.
+   See QDateTime's comparison operators and areFarEnoughApart().
 */
 static inline bool usesSameOffset(const QDateTimeData &a, const QDateTimeData &b)
 {
@@ -3170,6 +3170,26 @@ static inline bool usesSameOffset(const QDateTimeData &a, const QDateTimeData &b
     Q_UNREACHABLE_RETURN(false);
 }
 
+/* Even datetimes with different offset can be ordered by their getMSecs()
+   provided the difference is bigger than the largest difference in offset we're
+   prepared to believe in. Technically, it may be possible to construct a zone
+   with an offset outside the range and get wrong results - but the answer to
+   someone doing that is that their contrived timezone and its consequences are
+   their own responsibility.
+
+   If two datetimes' millis lie within the offset range of one another, we can't
+   take any short-cuts, even if they're in the same zone, because there may be a
+   zone transition between them. (The full 32-hour difference would only arise
+   before 1845, for one date-time in The Philippines, the other in Alaska.)
+*/
+bool areFarEnoughApart(qint64 leftMillis, qint64 rightMillis)
+{
+    constexpr qint64 UtcOffsetMillisRange
+        = (QTimeZone::MaxUtcOffsetSecs - QTimeZone::MinUtcOffsetSecs) * MSECS_PER_SEC;
+    qint64 gap = 0;
+    return qSubOverflow(leftMillis, rightMillis, &gap) || qAbs(gap) > UtcOffsetMillisRange;
+}
+
 // Refresh the LocalTime or TimeZone validity and offset
 static void refreshZonedDateTime(QDateTimeData &d, const QTimeZone &zone,
                                  QDateTimePrivate::TransitionOptions resolve)
@@ -5202,8 +5222,10 @@ bool QDateTime::equals(const QDateTime &other) const
     if (!other.isValid())
         return false;
 
-    if (usesSameOffset(d, other.d))
-        return getMSecs(d) == getMSecs(other.d);
+    const qint64 thisMs = getMSecs(d);
+    const qint64 yourMs = getMSecs(other.d);
+    if (usesSameOffset(d, other.d) || areFarEnoughApart(thisMs, yourMs))
+        return thisMs == yourMs;
 
     // Convert to UTC and compare
     return toMSecsSinceEpoch() == other.toMSecsSinceEpoch();
@@ -5249,8 +5271,9 @@ Qt::weak_ordering compareThreeWay(const QDateTime &lhs, const QDateTime &rhs)
     if (!rhs.isValid())
         return Qt::weak_ordering::greater; // we know that lhs is valid here
 
-    if (usesSameOffset(lhs.d, rhs.d))
-        return Qt::compareThreeWay(getMSecs(lhs.d), getMSecs(rhs.d));
+    const qint64 lhms = getMSecs(lhs.d), rhms = getMSecs(rhs.d);
+    if (usesSameOffset(lhs.d, rhs.d) || areFarEnoughApart(lhms, rhms))
+        return Qt::compareThreeWay(lhms, rhms);
 
     // Convert to UTC and compare
     return Qt::compareThreeWay(lhs.toMSecsSinceEpoch(), rhs.toMSecsSinceEpoch());

tests/auto/corelib/time/qdatetime/tst_qdatetime.cpp

@@ -2571,6 +2571,31 @@ void tst_QDateTime::ordering_data()
     generateRow(epochEast1h, epochWest1h, Qt::weak_ordering::equivalent);
     if (epochTimeType == LocalTimeIsUtc)
         generateRow(epoch, local1970, Qt::weak_ordering::equivalent);
+
+#if QT_CONFIG(timezone)
+    // See qtimezone.h's comments on M(ax|in)UtcOffsetSecs:
+    QTimeZone alaska("America/Metlakatla"), phillip("Asia/Manila");
+    if (alaska.isValid() && phillip.isValid()) {
+        // Date Narciso Claveria ordered Manila's transition:
+        QDateTime edict(QDate(1844, 8, 16), QTime(8, 4), phillip); // GMT start of next day
+        // Backends may lack relevant data, so check:
+        const int alaskaOffset = edict.toTimeZone(alaska).offsetFromUtc();
+        const int manilaOffset = edict.toTimeZone(phillip).offsetFromUtc();
+        if (manilaOffset < 0 && alaskaOffset > 0) {
+            qint64 offsetGap = alaskaOffset - manilaOffset; // 31h 9m 42s
+            QDateTime equiv = edict.toTimeZone(alaska); // 15:13:42, on the next day.
+            QTest::newRow("extreme.equivalent")
+                << edict << equiv << Qt::weak_ordering::equivalent;
+            // Despite internal msecs values; edict's is offsetGap * 1000 less than equiv's.
+            // Even the least increase in that gap does imply less:
+            QTest::newRow("extreme.less")
+                << edict << equiv.addMSecs(1) << Qt::weak_ordering::less;
+            // Until offsetGap seconds later, edict's msecs doesn't catch up with equiv's:
+            QTest::newRow("extreme.greater")
+                << edict.addSecs(offsetGap - 1) << equiv << Qt::weak_ordering::greater;
+        }
+    }
+#endif
 }
 
 void tst_QDateTime::ordering()