Editorial: Add "local-offset time value" for time values with local time zone offset applied #3464

anba · 2024-10-29T09:32:24Z

Commit ed3cab3:

Multiple operations define their inputs as time values, but actually allow time values adjusted with a local time zone offset.

Rename time value into epoch time value to emphasise that it's a number of milliseconds relative to the epoch. Then add local time value which is an epoch time value adjusted with a time zone offset. Time zone offsets are restricted to the exclusive range ±86'400'000 milliseconds.

Finally update all references to time value to use either epoch time value or local time value.

Two drive-by fixes:

13be0f4: Ensure too large inputs are properly handled.
e547c58: Restrict time zone offsets in other operations (fixes Restrict GetNamedTimeZoneOffsetNanoseconds return value to less than 24 hours #3101).

anba · 2024-10-29T09:32:39Z

cc @ptomato

gibson042

I love it!

spec.html

bakkot · 2024-10-30T21:59:58Z

This is a great change. Editors would like to submit for consideration "time value" and "local-offset time value" as the names; @syg found "epoch time value" and "local time value" confusing because "local time value" is also based on the time since the epoch.

…ime zone offset applied Multiple operations define their inputs as "time values", but actually allow time values adjusted with a local time zone offset. Add "local-offset time value" which is a "time value" adjusted with a time zone offset. Time zone offsets are restricted to the exclusive range ±86'400'000 milliseconds.

Return `NaN` if `t` exceeds the "local time value" range, because operations called within `UTC` expect their arguments to be "local time values" and not arbitrarily large integers.

…ess than 24 hours Fixes tc39#3101

…akeDate

anba · 2024-10-31T09:05:46Z

Editors would like to submit for consideration "time value" and "local-offset time value" as the names;

Sure. I've updated b5b1f0a to use the suggested names.

Furthermore I've added e4aec51 and 6d7452a to narrow some input resp. output types. And added 03ddef5 per the above review suggestion.

michaelficarra · 2024-10-31T16:19:07Z

spec.html

@@ -32610,7 +32616,7 @@ <h1>Time-related Constants</h1>
      <emu-clause id="sec-day" type="abstract operation" oldids="eqn-Day,sec-day-number-and-time-within-day">
        <h1>
          Day (
-            _t_: a finite time value,
+            _t_: a finite local-offset time value,


All local-offset time values are finite. We can drop the qualifier at all these use sites. It's already implied by the definition, but we can also be more explicit about it if we want.

michaelficarra · 2024-10-31T16:20:34Z

spec.html

@@ -32589,6 +32589,12 @@ <h1>Time Values and Time Range</h1>
        <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>The exact moment of midnight at the beginning of 1 January 1970 UTC is represented by the time value *+0*<sub>𝔽</sub>.</p>
+        <p>An ECMAScript <dfn variants="local-offset time values">local-offset time value</dfn> is a time value adjusted by a time zone offset that is an integer number of milliseconds in the interval from -86,400,000 (exclusive) to 86,400,000 (exclusive). A local-offset time value represents a local date and time of day, specifically the UTC date and time of day corresponding an identical time value, ignoring range limits. For example, a local-offset time value of *60000*<sub>𝔽</sub> represents time of day 00:01 on 1 January 1970, and could equivalently come from any of the following:</p>


We talked in editor call about how technically a local-offset time value isn't a time value and that might be confusing to readers. But I don't really mind explaining it like this. @tc39/ecma262-editors Thoughts?

michaelficarra · 2024-10-31T16:22:00Z

spec.html

+          Input _t_ is nominally a local-offset time value but may be any Number value.
          The algorithm must not limit _t_ to the time value range, so that inputs corresponding with a boundary of the time value range can be supported regardless of local UTC offset.
          For example, the maximum time value is 8.64 × 10<sup>15</sup>, corresponding with *"+275760-09-13T00:00:00Z"*.
          In an environment where the local time zone offset is ahead of UTC by 1 hour at that instant, it is represented by the larger input of 8.64 × 10<sup>15</sup> + 3.6 × 10<sup>6</sup>, corresponding with *"+275760-09-13T01:00:00+01:00"*.


I suggest we redefine local-offset time value to account for this additional range, then type this AO properly, taking a local-offset time value.

Alternatively, I don't think it'd be a problem to just expand the range of time value either, right?

UTC is called with unbounded integral Number values, so we can't restrict the input to time values.

Okay. I still think we should expand the range supported by local-offset time values.

Also, it's unrelated to this PR, but: t isn't nominally a time value, it's nominally a Number. It's "ostensibly" or "figuratively" a time value. I would update that term as well.

I don't understand to which range local-offset time values should be extended? Any integral Number, including NaN (and possibly Infinity, too)? Because that's the range which can be produced by MakeDate. (And the output of MakeDate is passed as the input to UTC.)

I understand we won't be able to narrow the parameter to UTC. Now I'm just talking about changing the definition of local-offset time value.

ISO 8601 timezone offsets permit ±24 hours (less 1 nanosecond). Time values are integers within the range -8,640,000,000,000,000 to 8,640,000,000,000,000 (100 million days on either side of the epoch). I'm suggesting that local time values have an extended range of ±8,640,000,086,400,000 (100,000,001 days on either side of the epoch).

±8,640,000,086,400,000 suggests it's valid for a time zone offset to be ±24 hours, but just as you said, the maximum valid time zone offset is ±23:59:59.999.999.999 hours. Ignoring micro- and nanoseconds, we get ±23:59:59.999, which is ±86399999 = ±(86400000 - 1) milliseconds, so exactly the range already used within this PR.

Can you point to where a local-offset time value is defined in that way in this PR? All I see is that it's restricted to the same range as (non-offset) time values: ±8,640,000,000,000,000.

Ah, I think I understand our misunderstanding.

The first commit has:

An ECMAScript local-offset time value is a time value adjusted by a time zone offset that is an integer number of milliseconds in the interval from -86,400,000 (exclusive) to 86,400,000 (exclusive).

I guess "time value adjusted by a time zone offset" can be read in two different ways:

Take a time value ±(8.64 * 10^15) and adjust it by ±86399999 to get ±8,640,000,086,399,999 as the final range.

"local-offset time value" is still a "time value", so restricted to ±(8.64 * 10^15), but additionally has some time zone offset applied.

Yes, I read it the second way. I would prefer to explicitly state the final computed range in its definition.

michaelficarra · 2024-10-31T16:25:01Z

spec.html

@@ -33069,7 +33075,7 @@ <h1>SystemTimeZoneIdentifier ( ): a String</h1>
        <h1>
          LocalTime (
            _t_: a finite time value,
-          ): an integral Number
+          ): a local-offset time value


If, for some reason, we don't make all local-offset time values finite, we should add the finite qualifier here.

michaelficarra · 2024-10-31T16:26:27Z

spec.html

@@ -32966,7 +32972,7 @@ <h1>
          GetNamedTimeZoneOffsetNanoseconds (
            _timeZoneIdentifier_: a String,
            _epochNanoseconds_: a BigInt,
-          ): an integer
+          ): an integer in the exclusive interval from -8.64 × 10<sup>13</sup> to 8.64 × 10<sup>13</sup>


I would prefer defining this type up in Time Values and Time Range alongside "local-offset time value" as something like "timezone offset".

We can reuse that definition in Temporal.

michaelficarra · 2024-10-31T16:29:40Z

spec.html

@@ -32966,7 +32972,7 @@ <h1>
          GetNamedTimeZoneOffsetNanoseconds (
            _timeZoneIdentifier_: a String,
            _epochNanoseconds_: a BigInt,
-          ): an integer
+          ): an integer in the exclusive interval from -8.64 × 10<sup>13</sup> to 8.64 × 10<sup>13</sup>


We actually don't define the term "exclusive interval". We should add a dfn for it where we define intervals.

ptomato · 2024-11-01T00:16:21Z

spec.html

@@ -32695,7 +32701,7 @@ <h1>
      <emu-clause id="sec-yearfromtime" type="abstract operation" oldids="eqn-YearFromTime">
        <h1>
          YearFromTime (
-            _t_: a finite time value,
+            _t_: a finite local-offset time value,


Note that YearFromTime can still be called from MakeDay with a Number that is outside of the local-offset time value range, for example in Date.UTC(-271821, 3, 20). Explanation in #1087 (comment). Same for MonthFromTime and DateFromTime. (This is pre-existing, so doesn't need to block the PR.)

ptomato · 2024-11-01T00:18:23Z

Glad to see this!

gibson042 approved these changes Oct 29, 2024

View reviewed changes

spec.html Outdated Show resolved Hide resolved

spec.html Show resolved Hide resolved

bakkot added the editor call to be discussed in the next editor call label Oct 29, 2024

michaelficarra removed the editor call to be discussed in the next editor call label Oct 30, 2024

anba force-pushed the time-value branch from e547c58 to 81aa87e Compare October 31, 2024 08:54

anba added 6 commits October 31, 2024 09:55

Editorial: Handle too large inputs early in UTC

e8051bb

Return `NaN` if `t` exceeds the "local time value" range, because operations called within `UTC` expect their arguments to be "local time values" and not arbitrarily large integers.

Editorial: Restrict GetNamedTimeZoneOffsetNanoseconds return values l…

917cf0c

…ess than 24 hours Fixes tc39#3101

Editorial: Narrow input and output types for MakeDay, MakeTime, and M…

e4aec51

…akeDate

Editorial: Narrow output type for TimeClip

6d7452a

Fixup: Review suggestion from gibson042

03ddef5

anba force-pushed the time-value branch from 81aa87e to 03ddef5 Compare October 31, 2024 08:58

anba changed the title ~~Editorial: Split "time value" into "epoch time value" and "local time value"~~ Editorial: Add "local-offset time value" for time values with local time zone offset applied Oct 31, 2024

michaelficarra reviewed Oct 31, 2024

View reviewed changes

ptomato reviewed Nov 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Editorial: Add "local-offset time value" for time values with local time zone offset applied #3464

Editorial: Add "local-offset time value" for time values with local time zone offset applied #3464

anba commented Oct 29, 2024

anba commented Oct 29, 2024

gibson042 left a comment

bakkot commented Oct 30, 2024

anba commented Oct 31, 2024

michaelficarra Oct 31, 2024

michaelficarra Oct 31, 2024

michaelficarra Oct 31, 2024 •

edited

Loading

michaelficarra Oct 31, 2024

anba Oct 31, 2024

michaelficarra Oct 31, 2024

anba Oct 31, 2024

michaelficarra Oct 31, 2024 •

edited

Loading

anba Oct 31, 2024

michaelficarra Oct 31, 2024 •

edited

Loading

anba Oct 31, 2024

michaelficarra Oct 31, 2024

michaelficarra Oct 31, 2024

michaelficarra Oct 31, 2024

ptomato Nov 1, 2024

michaelficarra Oct 31, 2024 •

edited

Loading

ptomato Nov 1, 2024

ptomato commented Nov 1, 2024

Editorial: Add "local-offset time value" for time values with local time zone offset applied #3464

Are you sure you want to change the base?

Editorial: Add "local-offset time value" for time values with local time zone offset applied #3464

Conversation

anba commented Oct 29, 2024

anba commented Oct 29, 2024

gibson042 left a comment

Choose a reason for hiding this comment

bakkot commented Oct 30, 2024

anba commented Oct 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ptomato commented Nov 1, 2024

michaelficarra Oct 31, 2024 •

edited

Loading

michaelficarra Oct 31, 2024 •

edited

Loading

michaelficarra Oct 31, 2024 •

edited

Loading

michaelficarra Oct 31, 2024 •

edited

Loading