HowardHinnant / date

A date and time library based on the C++11/14/17 <chrono> header
Other
3.14k stars 675 forks source link

Incorrect value of date::sys_info::save #810

Open apavenis opened 8 months ago

apavenis commented 8 months ago

I specified to date::zoned_time objects (one with DST and another without) and printed following:

Fmi::date_time::LocalDateTime tester
====================================
Running 2 test cases...
Fmi::date_time::LocalDateTime: construction and extraction (1)
2014-Feb-02 12:34:56.789000 EET   base: 02:00:00   save: 00:00:00   sys_info 2013-10-27 01:00:00
2014-03-30 01:00:00
02:00:00
00:00:00
EET

Fmi::date_time::LocalDateTime: construction and extraction (2)
2014-May-05 12:34:56.789000 EEST   base: 03:00:00   save: 00:01:00   sys_info 2014-03-30 01:00:00
2014-10-26 01:00:00
03:00:00
00:01:00
EEST

LocalDateTimeTest.cpp(71): error: in "local_date_time_test_2": check ldt.base_offset() == 2.0 has failed [2.9833333333333334 != 2]
LocalDateTimeTest.cpp(72): error: in "local_date_time_test_2": check ldt.dst_offset() == 1.0 has failed [0 != 1]

*** 2 failures are detected in the test module "Fmi::date_time::LocalDateTime tester"

Sys_info::save value in the second sample (DST on) is clearly incorrect 00:01:00, when it should be 01:00:00

PS: We are trying to use a wrapper around std::chrono and date libraries to preserve earlier used API as much as possible to make conversion of large software project (about 3000000 lines) as easy as possible.

HowardHinnant commented 8 months ago

PS: We are trying to use a wrapper around std::chrono and date libraries to preserve earlier used API as much as possible to make conversion of large software project (about 3000000 lines) as easy as possible.

This is a great plan. Many people don't realize how useful chrono can be as middleware like this.

It appears you are using the time zone database that ships with your OS (USE_OS_TZDB=1), and this is fine. However one drawback of this is that this database strips out the save value and replaces it with a single bit: 1 == (save != 0min) and 0 == (save == 0min). This is most unfortunate. However the offset is still accurate: 3h vs 2h. So you can still get accurate conversions between local_time and sys_time.

My recommendation is to ignore the save value, except to answer the question of "is this DST or not".

Another workaround would be to interpret 1min as 1h. However that approach is incorrect with some timezones. So I don't recommend it unless you know a-priori that you are dealing with a subset of timezones where the delta between standard and dst is always 1h.