Date time operations cleanup #1
Conversation
…in [[DateValue]] UTC/local time calculations near the [[DateValue]] limits can yield integers that exceed them.
Also refactor InLeapYear to rely upon DayFromYear rather than duplicating it's logic
There was a problem hiding this comment.
I think it's probably worth spending some time on how we would like to pitch this to the editors; I think we're all three agreed that this is useful, but for people that aren't involved in Temporal I can see how one more PR full of finicky time value calculations might be scary, so we should provide a good motivation along with it.
I'm thinking along the lines of "salvaging useful editorial cleanups from tc39#1564, plus fixing tc39#1087"
| @@ -32018,7 +32018,7 @@ <h1>Time Values and Time Range</h1> | |||
| <p>Time measurement in ECMAScript is analogous to time measurement in POSIX, in particular sharing definition in terms of the proleptic Gregorian calendar, an <dfn id="epoch">epoch</dfn> of midnight at the beginning of 1 January 1970 UTC, and an accounting of every day as comprising exactly 86,400 seconds (each of which is 1000 milliseconds long).</p> | |||
| <p>An ECMAScript <dfn variants="time values">time value</dfn> is a Number, either a finite integral Number representing an instant in time to millisecond precision or *NaN* representing no specific instant. A time value that is a multiple of <emu-eqn>24 × 60 × 60 × 1000 = 86,400,000</emu-eqn> (i.e., is equal to 86,400,000 × _d_ for some integer _d_) represents the instant at the start of the UTC day that follows the epoch by _d_ whole UTC days (preceding the epoch for negative _d_). Every other finite time value _t_ is defined relative to the greatest preceding time value _s_ that is such a multiple, and represents the instant that occurs within the same UTC day as _s_ but follows it by (_t_ - _s_) milliseconds.</p> | |||
| <p>Time values do not account for UTC leap seconds—there are no time values representing instants within positive leap seconds, and there are time values representing instants removed from the UTC timeline by negative leap seconds. However, the definition of time values nonetheless yields piecewise alignment with UTC, with discontinuities only at leap second boundaries and zero difference outside of leap seconds.</p> | |||
| <p>A Number can exactly represent all integers from -9,007,199,254,740,992 to 9,007,199,254,740,992 (<emu-xref href="#sec-number.min_safe_integer"></emu-xref> and <emu-xref href="#sec-number.max_safe_integer"></emu-xref>). A time value supports a slightly smaller range of -8,640,000,000,000,000 to 8,640,000,000,000,000 milliseconds. This yields a supported time value range of exactly -100,000,000 days to 100,000,000 days relative to midnight at the beginning of 1 January 1970 UTC.</p> | |||
| <p>A Number can exactly represent all integers from -9,007,199,254,740,992 to 9,007,199,254,740,992 (<emu-xref href="#sec-number.min_safe_integer"></emu-xref> and <emu-xref href="#sec-number.max_safe_integer"></emu-xref>). A time value used for the [[DateValue]] internal slot of a Date instance supports a slightly smaller range of -8,640,000,000,000,000 to 8,640,000,000,000,000 milliseconds (exactly -100,000,000 days to 100,000,000 days relative to midnight at the beginning of 01 January, 1970 UTC).</p> | |||
| <p>The exact moment of midnight at the beginning of 1 January 1970 UTC is represented by the time value *+0*<sub>𝔽</sub>.</p> | |||
| <emu-note> | |||
| <p>The 400 year cycle of the proleptic Gregorian calendar contains 97 leap years. This yields an average of 365.2425 days per year, which is 31,556,952,000 milliseconds. Therefore, the maximum range a Number could represent exactly with millisecond precision is approximately -285,426 to 285,426 years relative to 1970. The smaller range supported by a time value as specified in this section is approximately -273,790 to 273,790 years relative to 1970.</p> | |||
There was a problem hiding this comment.
I like the distinction made here between "time value" and "time value used for the [[DateValue]] internal slot". Compare this to the types used in LocalTime (finite time value → integral Number) and UTC (Number → time value). This "integral Number" language I think is obfuscating a bit what is actually going on. I think what these are really trying to say is "finite time value → finite time value that can go in a [[DateValue]] internal slot" and "time value from a [[DateValue]] internal slot → time value", respectively. But that rename might be a harder case to make convincingly, because it only matters in the case of a time value near the very edge of the range, with a UTC offset.
There are probably a few more places where we'd have to clarify this distinction, like this line:
| <p>The 400 year cycle of the proleptic Gregorian calendar contains 97 leap years. This yields an average of 365.2425 days per year, which is 31,556,952,000 milliseconds. Therefore, the maximum range a Number could represent exactly with millisecond precision is approximately -285,426 to 285,426 years relative to 1970. The smaller range supported by a time value as specified in this section is approximately -273,790 to 273,790 years relative to 1970.</p> | |
| <p>The 400 year cycle of the proleptic Gregorian calendar contains 97 leap years. This yields an average of 365.2425 days per year, which is 31,556,952,000 milliseconds. Therefore, the maximum range a Number could represent exactly with millisecond precision is approximately -285,426 to 285,426 years relative to 1970. The smaller range supported by a [[DateValue]] as specified in this section is approximately -273,790 to 273,790 years relative to 1970.</p> |
There was a problem hiding this comment.
I wonder if separate terminology is the way to go, like "exact time value" and "local-adjusted time value".
| = *9*<sub>𝔽</sub> if *273*<sub>𝔽</sub> + InLeapYear(_t_) ≤ DayWithinYear(_t_) < *304*<sub>𝔽</sub> + InLeapYear(_t_) | ||
| = *10*<sub>𝔽</sub> if *304*<sub>𝔽</sub> + InLeapYear(_t_) ≤ DayWithinYear(_t_) < *334*<sub>𝔽</sub> + InLeapYear(_t_) | ||
| = *11*<sub>𝔽</sub> if *334*<sub>𝔽</sub> + InLeapYear(_t_) ≤ DayWithinYear(_t_) < *365*<sub>𝔽</sub> + InLeapYear(_t_) | ||
| <p>Months are identified by an integer in the range 0 to 11, inclusive. A month value of 0 specifies January; 1 specifies February; 2 specifies March; 3 specifies April; 4 specifies May; 5 specifies June; 6 specifies July; 7 specifies August; 8 specifies September; 9 specifies October; 10 specifies November; and 11 specifies December.</p> |
There was a problem hiding this comment.
| <p>Months are identified by an integer in the range 0 to 11, inclusive. A month value of 0 specifies January; 1 specifies February; 2 specifies March; 3 specifies April; 4 specifies May; 5 specifies June; 6 specifies July; 7 specifies August; 8 specifies September; 9 specifies October; 10 specifies November; and 11 specifies December.</p> | |
| <p>Months are identified by an integer in the inclusive interval from 0 to 11. A month value of 0 specifies January; 1 specifies February; 2 specifies March; 3 specifies April; 4 specifies May; 5 specifies June; 6 specifies July; 7 specifies August; 8 specifies September; 9 specifies October; 10 specifies November; and 11 specifies December.</p> |
(This "inclusive interval" language is new since tc39#2848, so we should preserve it.)
| @@ -32359,12 +32345,14 @@ <h1> | |||
| 1. If _year_ is not finite or _month_ is not finite or _date_ is not finite, return *NaN*. | |||
| 1. Let _y_ be 𝔽(! ToIntegerOrInfinity(_year_)). | |||
| 1. Let _m_ be 𝔽(! ToIntegerOrInfinity(_month_)). | |||
There was a problem hiding this comment.
You could consider changing m to a mathematical value, since we only take ℝ(m) below anyway. But maybe the current way is more consistent, I'm not sure.
| 1. Let _d_ be 𝔽(! ToIntegerOrInfinity(_date_)). | ||
| 1. Let _wholeYears_ be _y_ + 𝔽(floor(ℝ(_m_) / 12)). | ||
| 1. If _wholeYears_ is not finite, return *NaN*. | ||
| 1. Let _monthWithinYear_ be 𝔽(ℝ(_m_) modulo 12). |
There was a problem hiding this comment.
I think this has to be a mathematical value, since it's passed to DaysWithinYearBeforeMonth which takes a mathematical value as its first argument.
| 1. If _wholeYears_ is not finite, return *NaN*. | ||
| 1. Let _monthWithinYear_ be 𝔽(ℝ(_m_) modulo 12). | ||
| 1. Let _leapDaysInYear_ be LeapDaysInYear(TimeFromYear(_wholeYears_)). | ||
| 1. Let _day_ be DayFromYear(_wholeYears_) + DaysWithinYearBeforeMonth(_monthWithinYear_, _leapDaysInYear_) *1*<sub>𝔽</sub>. |
There was a problem hiding this comment.
| 1. Let _day_ be DayFromYear(_wholeYears_) + DaysWithinYearBeforeMonth(_monthWithinYear_, _leapDaysInYear_) *1*<sub>𝔽</sub>. | |
| 1. Let _day_ be DayFromYear(_wholeYears_) + DaysWithinYearBeforeMonth(_monthWithinYear_, _leapDaysInYear_) - *1*<sub>𝔽</sub>. |
| @@ -32060,7 +32060,23 @@ <h1>Year Number</h1> | |||
|
|
|||
There was a problem hiding this comment.
In how far is this commit ("Normative: Use Number arithmetic for constructing dates and times") still normative? Now that the version of ECMA-262 this PR is based on is already clarified to use Number arithmetic.
I guess my doubt would be about the changes to MakeDay. If we consider that currently "underspecified" then maybe it's not normative. (I haven't gone through the new definition with the various examples given in tc39#1087, though.)
If the MakeDay changes are normative, on the other hand, then I'd recommend splitting them out of this commit into a separate one, so that it's immediately clear to reviewers what they have to pay the most attention to.
| = *1*<sub>𝔽</sub> if DaysInYear(YearFromTime(_t_)) = *366*<sub>𝔽</sub> | ||
| </emu-eqn> | ||
| <p>The leap-year function is *1*<sub>𝔽</sub> for a leap year and otherwise is *+0*<sub>𝔽</sub>:</p> | ||
| <emu-eqn id="eqn-LeapDaysInYear" aoid="LeapDaysInYear" oldids="eqn-InLeapYear">LeapDaysInYear(_y_) = DayFromYear(_y_ + 1) - DayFromYear(_y_) - 365</emu-eqn> |
There was a problem hiding this comment.
Does this run into problems in the year +275760, since DayFromYear is called on an out-of-range year? At first glance the DayFromYear definition seems to handle this OK.
| </emu-eqn> | ||
| <emu-clause id="sec-MonthFromTime" aoid="MonthFromTime" oldids="eqn-MonthFromTime"> | ||
| <h1>MonthFromTime ( _t_ )</h1> | ||
| <p>The abstract operation MonthFromTime maps a time value _t_ to a month number. It performs the following steps:</p> |
There was a problem hiding this comment.
I'd recommend updating this definition to use a structured header with types so that you get the standard prose. (although I hope it's still possible to specify oldids on a structured header?)
| <emu-alg> | ||
| 1. Let _day_ be DayWithinYear(_t_). | ||
| 1. Let _leapDaysInYear_ be LeapDaysInYear(YearFromTime(_t_)). | ||
| 1. Let _month_ be *+0*<sub>𝔽</sub>. |
There was a problem hiding this comment.
month is a Number here but later on it's treated as a mathematical value (MV 1 is added to it and it's passed to DaysWithinYearBeforeMonth which takes a MV). So, it should either be consistently a Number or consistently a MV.
I'm not sure which. MV seemed most in line with the definition of a month number in "Months are identified..." above, but on the other hand the result of MonthFromTime is returned directly to JS in some places, like Date.prototype.getMonth, and so those places would have to be passed through 𝔽. (For precision or overflow purposes it doesn't matter which one we choose, since it's always exactly 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, or 11.)
| <p>where</p> | ||
| <emu-eqn id="eqn-DayWithinYear" aoid="DayWithinYear">DayWithinYear(_t_) = Day(_t_) - DayFromYear(YearFromTime(_t_))</emu-eqn> | ||
| <p>A month value of *+0*<sub>𝔽</sub> specifies January; *1*<sub>𝔽</sub> specifies February; *2*<sub>𝔽</sub> specifies March; *3*<sub>𝔽</sub> specifies April; *4*<sub>𝔽</sub> specifies May; *5*<sub>𝔽</sub> specifies June; *6*<sub>𝔽</sub> specifies July; *7*<sub>𝔽</sub> specifies August; *8*<sub>𝔽</sub> specifies September; *9*<sub>𝔽</sub> specifies October; *10*<sub>𝔽</sub> specifies November; and *11*<sub>𝔽</sub> specifies December. Note that <emu-eqn>MonthFromTime(*+0*<sub>𝔽</sub>) = *+0*<sub>𝔽</sub></emu-eqn>, corresponding to Thursday, 1 January 1970.</p> | ||
| <p>Note that <emu-eqn>MonthFromTime(0) = 0</emu-eqn>, corresponding to Thursday, 01 January, 1970.</p> |
There was a problem hiding this comment.
Double-check the types here — I think this might be MonthFromTime(*+0*<sub>𝔽</sub>) = 0 (also taking into account the above comment about the return type of MonthFromTime)
No description provided.