-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Normative: Use Number arithmetic for constructing dates and times #1564
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome! These changes make the spec way more clear.
spec.html
Outdated
<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 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. Note that <emu-eqn>MonthFromTime(0) = 0</emu-eqn>, corresponding to Thursday, 01 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use <emu-note>
? If so, then change the "note" in the Week Day
definition to the <emu-note>
format, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this renders better as-is, but will change to <emu-note>
if there are strong feelings about it.
e5c9eb0
to
d6b339c
Compare
spec.html
Outdated
1. If _wholeYears_ is not finite, return *NaN*. | ||
1. Let _monthWithinYear_ be _m_ modulo 12. | ||
1. Let _inLeapYear_ be InLeapYear(TimeFromYear(_wholeYears_)). | ||
1. Let _day_ be DayFromYear(_wholeYears_) `+` DaysWithinYearBeforeMonth(_monthWithinYear_, _inLeapYear_) `+` _d_ `-` 1, performing the arithmetic according to IEEE 754-2008 rules (that is, as if using the ECMAScript operators `+` and `-`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you use the subscript 𝔽
, i don't think you need the "performing …" prose, since that makes them Numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't even need the subscript, since it's optional when the operands are Number values, which they presumably are, according to 5.2.5. And the operators don't need backticks either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
even better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool; I'll update that. Should multiplication of Number values use *
or ×
?
a754929
to
c5b118b
Compare
Differentiate time values representing instants in (absolute) time from those representing (civil) date and time of day.
…in [[DateValue]] UTC/local time calculations near the [[DateValue]] limits can yield integers that exceed them.
…uplicating its logic
…lip immediately after it
c5b118b
to
88c606e
Compare
@gibson042 Since there's a normative component to this change it needs consensus from the committee. I don't believe it's been discussed yet. Are you interested in presenting the normative component? I can try to do so it if not, though I'm not confident I can give a full and correct accounting of the change. |
<p>Local time zone offset values may be positive <i>or</i> negative.</p> | ||
</emu-note> | ||
<p>When _isUTC_ is *true*, <dfn>LocalTZA( _t_, *true* )</dfn> interprets _t_ as a time value representing an instant in time. It returns the offset of the local time zone from UTC in milliseconds at that instant. When the result is added to _t_, it should yield the time value for which UTC date and time of day are equal to the local date and time of day at time value _t_.</p> | ||
<p>When _isUTC_ is *false*, <dfn>LocalTZA( _t_, *false* )</dfn> interprets _t_ as a time value representing a date and time of day. It returns the offset of the local time zone from UTC in milliseconds at the smallest (closest to negative infinity) time value _earliestMatch_ for which either the local date and time of day at _earliestMatch_ is equal to the UTC date and time of day at _t_ or the local date and time of day at _earliestMatch_ + 1 is greater than the UTC date and time of day at _t_. When the result is subtracted from _t_, it should yield the smallest time value for which the local date and time of day are equal to the UTC date and time of day at _t_. If no such time value exists because of local times skipped by positive time zone transitions (e.g. when the daylight saving time starts or the time zone adjustment is increased due to a time zone rule change), then subtracting the result from _t_ should instead yield the time value for which local date and time of day in the UTC offset applicable before the transition are equal to the UTC date and time of day at _t_.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be given in pseudo code?
Let localtime
be _t_
.
Find smallest integer t
for which localtime < t + max(LocalTZA(t - 1, true), LocalTZA(t, true))
Return localtime - LocalTZA(t - 1, true)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure at a glance if that expression is correct, and even if it is, the lack of an explanation makes it even harder to follow in my opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gibson042
It is the same expression as you are using:
Let _earliestMatch_
be the smallest integer for which LocalTime(_earliestMatch_) === _t_ || LocalTime(_earliestMatch_ + 1) > _t_
.
Return _t_ - (LocalTime(_earliestMatch_) - _earliestMatch_)
.
where LocalTime
is the abstract operation mentioned in the spec.
I think, it can be used together with your explanation, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'd rather not introduce cyclic references between the text of isUTC and LocalTime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gibson042 , LocalTime calls LocalTZA(t, true), it would be even better to move what LocalTZA(t, false) does into the "UTC".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#2120 actually replaces part of this PR, but I think there are still normative changes in MakeDay and MakeDate that will be observable via e.g. Date.UTC( 1970, 0, 1 - 1e8, -BIG, BIG * 60 + 1, 1e8 * 86400 * 2 - 61, 61000 )
(where BIG
is at or near Number.MAX_SAFE_INTEGER
).
<p>Local time zone offset values may be positive <i>or</i> negative.</p> | ||
</emu-note> | ||
<p>When _isUTC_ is *true*, <dfn>LocalTZA( _t_, *true* )</dfn> interprets _t_ as a time value representing an instant in time. It returns the offset of the local time zone from UTC in milliseconds at that instant. When the result is added to _t_, it should yield the time value for which UTC date and time of day are equal to the local date and time of day at time value _t_.</p> | ||
<p>When _isUTC_ is *false*, <dfn>LocalTZA( _t_, *false* )</dfn> interprets _t_ as a time value representing a date and time of day. It returns the offset of the local time zone from UTC in milliseconds at the smallest (closest to negative infinity) time value _earliestMatch_ for which either the local date and time of day at _earliestMatch_ is equal to the UTC date and time of day at _t_ or the local date and time of day at _earliestMatch_ + 1 is greater than the UTC date and time of day at _t_. When the result is subtracted from _t_, it should yield the smallest time value for which the local date and time of day are equal to the UTC date and time of day at _t_. If no such time value exists because of local times skipped by positive time zone transitions (e.g. when the daylight saving time starts or the time zone adjustment is increased due to a time zone rule change), then subtracting the result from _t_ should instead yield the time value for which local date and time of day in the UTC offset applicable before the transition are equal to the UTC date and time of day at _t_.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure at a glance if that expression is correct, and even if it is, the lack of an explanation makes it even harder to follow in my opinion.
At the last meeting I asked for (and achieved) consensus that all arithmetic on Date objects was to be done with Number semantics, particularly calling out the large number cases, so I think the normative parts are good to go unless they entail changes beyond that. I was planning to make that particular change as a part of #2007 (which makes it unambiguous when arithmetic is being done as if on reals vs as if on Numbers). Unfortunately that PR is going to have a bunch of conflicts with this one; when we were working on it I'd forgotten you already made this PR. I'll need to rebase one of them. |
3d0c24c
to
7a79833
Compare
@gibson042 Is this PR still applicable after #2007? |
No, I don't think so. Simplify MonthFromTime and Dedicate a subsection to time value range and Refactor InLeapYear(t) as LeapDaysInYear(y) are editorial changes that could be added independently if deemed desirable, and everything else has already been superseded. |
Fixes #1087
Also differentiates the range for time value vs. [[DateValue]], cleans up the text for date and time operations (particularly LocalTZA), and consolidates them where possible.
Can be reviewed commit-by-commit if the merged view is overwhelming.