Don't pretend we know what DST to use for an offset date.

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 <jedrzej.nowacki@theqtcompany.com>
bb10
Edward Welbourne 2015-11-04 10:26:37 +01:00
parent 08775e4bd7
commit 81858bf172
2 changed files with 38 additions and 32 deletions

View File

@ -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

View File

@ -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");