[Development] QtCS - QDateTime changes

[Knoll Lars]

On 26.07.13 22:26, "John Layt" <jlayt at kde.org> wrote:

>Hi,
>
>As was unable to be discussed at QtCS due to running out of time, I've
>gotten back on to my QTimeZone changes, but taking a slightly
>different approach to getting them integrated for 5.2.  The QTimeZone
>class itself works fine, the issue I hit before 5.1 was getting
>QDateTime to properly deal with the daylight saving transitions, a
>problem common with the existing Qt::LocalTime implementation.  Rather
>than try deal with the two issues in parallel I've decided to fix
>QDateTime first, then integrate QTimeZone.  I've also decided to start
>pushing smaller commits to make it easier for people to review what
>I'm doing :-)  The result should be that the QTimeZone integration
>step requires fewer changes to QDateTime itself.
>
>My plan looks something like:
>* Push existing OffsetFromUTC and date formatter fixes
>* Make the required changes to fix Qt::LocalTime at the daylight time
>transitions
>* Integrate the existing QTimeZone implementation
>
>The big problems come from the required changes to fix Qt::LocalTime
>at the transitions.  There are two issues here:
>* Transition from standard to daylight leaves a 1 hour 'hole', i.e.
>going forward from 2am to 3am means 2:30am doesn't exist
>* Transition from daylight to standard repeats a 1 hour period, i.e.
>going back from 3am to 2am means 2:30am occurs twice
>
>Currently Qt::LocalTime ignores both problems and pretends they don't
>exist.  The missing hour is treated as a valid time, and the second
>occurrence can only be set or read using the actual MSecsSinceEpoch
>value.  We need to change to the correct behaviour for both these
>situations.
>
>Current QDateTime behaviour:
>* QDateTime uses a lazy initialization that accepts any date, time and
>spec, validity is only checked when used
>* When called QDateTime::isValid() does not take the time zone into
>account, it only checks if QDate and QTime are individually valid,
>i.e. it doesn't check if the time falls in the 'hole'
>* Date-only math functions (add day/month/year) are passed straight to
>QDate, i.e. validity check and maths applied is on date only and
>doesn¹t consider time and time zone, i.e. if the result falls into the
>hole or is first or second occurrence.
>* Time math functions work correctly by checking QDateTime::isValid()
>first and then converting to UTC to calculate
>
>To fix the 'hole' problem we have to ensure validity is checked
>properly every time:
>* QDateTime::isValid() must check if valid in tz, i.e. call mktime the
>first time called then cache result in QDateTimePrivate::Spec
>* All date-only maths needs to be converted to UTC first then
>converted back, same as for time maths, but this will be a behaviour
>change
>
>To fix the occurrence problem will require new api to allow it to be
>set and read, which was previously designed for QTimeZone.  Issues
>with mktime having different behaviour on different platforms will
>make the implementation rather tricky.  Linux assumes an ambiguous
>time is the first occurrence, Windows assumes it is the second
>occurrence, and Mac assumes first occurrence for about the first 40
>minutes and second occurrence for the last 20 minutes (probably a
>bug).

Please make sure we get one behavior here for all platforms. Anything else
would be impossible to document.
>
>It has been discussed to re-write QDateTime to internally store as an
>absolute msecs since epoch which would inherently fix these issues and
>greatly simplify the maths and conversion code, but this would
>radically change the behaviour of QDateTime which currently treats the
>ymd/hms values as ³fixed² and the time spec is used to interpret that
>value. This would mostly affect the default Qt::LocalTime spec where
>the system time zone can change underneath QDateTime causing the
>absolute UTC value to change. There is also the behaviour that you can
>store an invalid date but valid time in QDateTime, and later fix the
>date to be valid.  Considerable special case code could be required to
>keep consistent with the current behaviour.

If we'd write this class now, storing the date time as ms since epoch is
what I'd prefer. It would be nice if we can change to that. Maybe it could
work if you in addition have two boolean flags indicating invalid dates
and times.

I don't think the corner cases where the time zone changes while the Qt
app is running is a problem. I would actually expect that having QDT
stored as ms since epoch would fix bugs here in 90% of the cases.

>Other likely effects would be:
>* Creation would be slower as has to validate and convert first
>* Accessing date() and time(), probably the most commonly used
>functions e.g. to get day() or hour(), would be slower, but we want to
>deprecate these anyway and use QCalendarSystem instead which would
>probably work faster with it

I don't think this will be an issue. Many other operations will be faster
and better defined, and converting to a date or time is not that slow.

>* Maths and conversion functions would be faster and simpler and more
>accurate
>* Would only need a qint64 instead of a qint64 and a qint32 so saving
>memory
>* Would reduce the historic supported date range, but QDateTime is
>only promised to be useful from 1970 onwards anyway

Why? The int64 should be plenty to store all historic dates.
>
>For now I will stick with cleaning up the current QDateTime code and
>only consider the re-write scenario if that doesn't work.
>
>With regards to the ICU situation, QTimeZone already has an ICU
>backend, but also system backends for Mac, Win32, and the standard tz
>file.  Currently it only uses ICU if QT_USE_ICU is set and it is not
>on UNIX, but this can change as I'm considering a QT_USE_ICU_TZ
>option.  In fact it provides the model for how the planned new locale
>classes will work.

Cheers,
Lars