search

HowardHinnant / date

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

Change the type of the data member of class year to int from short #800

Closed gggrace14 closed 9 months ago

gggrace14 commented 9 months ago

Currently class year uses short to represent a year value. https://github.com/HowardHinnant/date/blob/cc4685a21e4a4fdae707ad1233c61bbaff241f93/include/date/date.h#L395-L397

We find this limits the range of year value in our use case and causes overflow, and thus request changing the type to int.

HowardHinnant commented 9 months ago

I'm curious as to the context you are experiencing overflow. Can you show a brief code example? Thanks.

HowardHinnant commented 9 months ago

To be clearer, there are benefits to having year be 2 bytes:

  1. A year_month_day can be held in 4 bytes. Experience with boost::date_time over decades has shown that clients want a 4 byte date.
  2. A 2 byte year can handle the civil calendar years -32767 to 32767 which is a larger range than nearly everyone needs. The civil calendar models the solar system to an accuracy of about 1 day in 4000 years.
  3. The two-byte year is now standardized in C++20. And the limited range has enabled at least one implementation to make performance optimizations in the conversion to and from sys_days that would not be available had the year range been 4 bytes.

If all one needs is thousands or millions of years in the past or future, and not a date on the civil calendar, years (plural) is not a bad choice. It has a 4 byte representation. And it would be trivial to create a duration (with a period of years, kiloyears, or megayears) with other representations, including floating point.

If I knew more about your use case, I could almost certainly recommend a reasonable solution.

HowardHinnant commented 9 months ago

I've briefly scanned your code base linked above and see that you're already aware of the year::min() and year::max() functions being set to -32767 and 32767 respectively. My advice is to restrict your calendrical computations to that range. This range is what has been voted into the C++20 chrono specification: http://eel.is/c++draft/time.cal.year#members-19

If you have sufficient motivation to extend your range beyond these limits, here are the calendrical algorithms this library is based on and can be used with any number of bit-sized integers to achieve ranges far greater than +/- the age of the universe: http://howardhinnant.github.io/date_algorithms.html. The algorithms are in the public domain. Changing your copy of this library to use int (as you've done) is a safe and correct method of extending these public domain algorithms to a range of +/-5.8 million years.

The motivations for staying within the more restricted range are outlined in my previous comment. Closing as not-to-be-fixed.