From 81858bf1722081d99fdea72f55e67c04c5bb3d92 Mon Sep 17 00:00:00 2001 From: Edward Welbourne Date: Wed, 4 Nov 2015 10:26:37 +0100 Subject: Don't pretend we know what DST to use for an offset date. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When QDateTime::addDate() and friends sanitize their end-state, they were using the DST status of their start-state (if known) to control it. This lead to misguided results and, in particular, inconsistent results given that a raw-constructed QDateTime comes into being ignorant of its DST, while a .toLocalTime() one knows its DST. Furthermore, the code to do this was triplicated, tricky and poorly explained. So pull it out into a local static function and explain what it's doing, and why, more clearly and only once. Task-number: QTBUG-49008 Change-Id: Ia4bb3c5e9267fff8bb963ea705267998218ed623 Reviewed-by: Jędrzej Nowacki --- src/corelib/tools/qdatetime.cpp | 68 ++++++++++++---------- .../auto/corelib/tools/qdatetime/tst_qdatetime.cpp | 2 - 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/corelib/tools/qdatetime.cpp b/src/corelib/tools/qdatetime.cpp index 241b02df1b..fa4ac2b00f 100644 --- a/src/corelib/tools/qdatetime.cpp +++ b/src/corelib/tools/qdatetime.cpp @@ -3652,6 +3652,40 @@ QString QDateTime::toString(const QString& format) const } #endif //QT_NO_DATESTRING +static void massageAdjustedDateTime(Qt::TimeSpec spec, +#ifndef QT_BOOTSTRAPPED + const QTimeZone &zone, +#endif // QT_BOOTSTRAPPED + QDate *date, + QTime *time) +{ + /* + If we have just adjusted to a day with a DST transition, our given time + may lie in the transition hour (either missing or duplicated). For any + other time, telling mktime (deep in the bowels of localMSecsToEpochMSecs) + we don't know its DST-ness will produce no adjustment (just a decision as + to its DST-ness); but for a time in spring's missing hour it'll adjust the + time while picking a DST-ness. (Handling of autumn is trickier, as either + DST-ness is valid, without adjusting the time. We might want to propagate + d->daylightStatus() in that case, but it's hard to do so without breaking + (far more common) other cases; and it makes little difference, as the two + answers do then differ only in DST-ness.) + */ + if (spec == Qt::LocalTime) { + QDateTimePrivate::DaylightStatus status = QDateTimePrivate::UnknownDaylightTime; + localMSecsToEpochMSecs(timeToMSecs(*date, *time), &status, date, time); +#ifndef QT_BOOTSTRAPPED + } else if (spec == Qt::TimeZone) { + QDateTimePrivate::zoneMSecsToEpochMSecs(timeToMSecs(*date, *time), zone, date, time); +#endif // QT_BOOTSTRAPPED + } +} +#ifdef QT_BOOTSTRAPPED // Avoid duplicate #if-ery in uses. +#define MASSAGEADJUSTEDDATETIME(s, z, d, t) massageAdjustedDateTime(s, d, t) +#else +#define MASSAGEADJUSTEDDATETIME(s, z, d, t) massageAdjustedDateTime(s, z, d, t) +#endif // QT_BOOTSTRAPPED + /*! Returns a QDateTime object containing a datetime \a ndays days later than the datetime of this object (or earlier if \a ndays is @@ -3673,16 +3707,7 @@ QDateTime QDateTime::addDays(qint64 ndays) const QDate &date = p.first; QTime &time = p.second; date = date.addDays(ndays); - // Result might fall into "missing" DaylightTime transition hour, - // so call conversion and use the adjusted returned time - if (d->m_spec == Qt::LocalTime) { - QDateTimePrivate::DaylightStatus status = d->daylightStatus(); - localMSecsToEpochMSecs(timeToMSecs(date, time), &status, &date, &time); -#ifndef QT_BOOTSTRAPPED - } else if (d->m_spec == Qt::TimeZone) { - QDateTimePrivate::zoneMSecsToEpochMSecs(timeToMSecs(date, time), d->m_timeZone, &date, &time); -#endif // QT_BOOTSTRAPPED - } + MASSAGEADJUSTEDDATETIME(d->m_spec, d->m_timeZone, &date, &time); dt.d->setDateTime(date, time); return dt; } @@ -3708,16 +3733,7 @@ QDateTime QDateTime::addMonths(int nmonths) const QDate &date = p.first; QTime &time = p.second; date = date.addMonths(nmonths); - // Result might fall into "missing" DaylightTime transition hour, - // so call conversion and use the adjusted returned time - if (d->m_spec == Qt::LocalTime) { - QDateTimePrivate::DaylightStatus status = d->daylightStatus(); - localMSecsToEpochMSecs(timeToMSecs(date, time), &status, &date, &time); -#ifndef QT_BOOTSTRAPPED - } else if (d->m_spec == Qt::TimeZone) { - QDateTimePrivate::zoneMSecsToEpochMSecs(timeToMSecs(date, time), d->m_timeZone, &date, &time); -#endif // QT_BOOTSTRAPPED - } + MASSAGEADJUSTEDDATETIME(d->m_spec, d->m_timeZone, &date, &time); dt.d->setDateTime(date, time); return dt; } @@ -3743,19 +3759,11 @@ QDateTime QDateTime::addYears(int nyears) const QDate &date = p.first; QTime &time = p.second; date = date.addYears(nyears); - // Result might fall into "missing" DaylightTime transition hour, - // so call conversion and use the adjusted returned time - if (d->m_spec == Qt::LocalTime) { - QDateTimePrivate::DaylightStatus status = d->daylightStatus(); - localMSecsToEpochMSecs(timeToMSecs(date, time), &status, &date, &time); -#ifndef QT_BOOTSTRAPPED - } else if (d->m_spec == Qt::TimeZone) { - QDateTimePrivate::zoneMSecsToEpochMSecs(timeToMSecs(date, time), d->m_timeZone, &date, &time); -#endif // QT_BOOTSTRAPPED - } + MASSAGEADJUSTEDDATETIME(d->m_spec, d->m_timeZone, &date, &time); dt.d->setDateTime(date, time); return dt; } +#undef MASSAGEADJUSTEDDATETIME /*! Returns a QDateTime object containing a datetime \a s seconds diff --git a/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp b/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp index 216c670aee..228ce73c6b 100644 --- a/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp +++ b/tests/auto/corelib/tools/qdatetime/tst_qdatetime.cpp @@ -1616,9 +1616,7 @@ void tst_QDateTime::springForward_data() if (europeanTimeZone) { QTest::newRow("Europe from day before") << QDate(2015, 3, 29) << QTime(2, 30, 0) << 1 << 60; -#if 0 // FIXME: fails QTest::newRow("Europe from day after") << QDate(2015, 3, 29) << QTime(2, 30, 0) << -1 << 120; -#endif // } else if (otherZone) { } else { QSKIP("No spring forward test data for this TZ"); -- cgit v1.2.3