Bug 238050 – [JSC] Change Date.parse to stop returning numbers with fractional part

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Richard Gibson]

JavaScriptCore `Date.parse` can return non-integer non-NaN numbers, in violation of https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-date.parse (which requires the output to be a _time value_—"either a finite **integral Number** representing an instant in time to millisecond precision or NaN representing no specific instant" [cf. https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-time-values-and-time-range ]):
```sh
$ eshost -se '
  [
    "1970-01-01T00:00:00.000500001Z",
    "1969-12-31T23:59:59.999515625Z",
    "1969-12-31T23:59:59.999015625Z",
  ]
  .map(str => {
    const tv = Date.parse(str);
    return `${str} => ${(new Date(str)).toISOString()} (${Object.is(tv, -0) ? "-0" : tv} ms from epoch)`;
  })
  .join("\n")
'
#### ChakraCore, engine262, GraalJS, Hermes, SpiderMonkey, V8
1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
1969-12-31T23:59:59.999515625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)

#### JavaScriptCore
1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0.500001 ms from epoch)
1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (-0.484375 ms from epoch)
1969-12-31T23:59:59.999015625Z => 1970-01-01T00:00:00.000Z (-0.984375 ms from epoch)

#### Moddable XS
1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.001Z (1 ms from epoch)
1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
```

And worse, this results in bizarre round-towards-epoch behavior in new Date construction (as seen above).

Any rounding behavior would correct this bug, but I think it would be best to align with the majority of other implementations in specifically rounding time values down (i.e., towards negative infinity), yielding results equivalent to ignoring all digits beyond millisecond precision.


Also reported to test262 for coverage: https://github.com/tc39/test262/issues/3437

[Yusuke Suzuki]

(In reply to Richard Gibson from comment #0)
> JavaScriptCore `Date.parse` can return non-integer non-NaN numbers, in
> violation of
> https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-date.parse
> (which requires the output to be a _time value_—"either a finite **integral
> Number** representing an instant in time to millisecond precision or NaN
> representing no specific instant" [cf.
> https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-time-values-and-
> time-range ]):
> ```sh
> $ eshost -se '
>   [
>     "1970-01-01T00:00:00.000500001Z",
>     "1969-12-31T23:59:59.999515625Z",
>     "1969-12-31T23:59:59.999015625Z",
>   ]
>   .map(str => {
>     const tv = Date.parse(str);
>     return `${str} => ${(new Date(str)).toISOString()} (${Object.is(tv, -0)
> ? "-0" : tv} ms from epoch)`;
>   })
>   .join("\n")
> '
> #### ChakraCore, engine262, GraalJS, Hermes, SpiderMonkey, V8
> 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
> 1969-12-31T23:59:59.999515625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> 1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)

I think this looks very strange: if we perform TimeClip operation (https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-timeclip),
then all results should be 0 ms since TimeClip is effectively trunc operation.

I think the reasonable behavior is +0 for all three inputs.

> 
> #### JavaScriptCore
> 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0.500001 ms from
> epoch)
> 1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (-0.484375 ms
> from epoch)
> 1969-12-31T23:59:59.999015625Z => 1970-01-01T00:00:00.000Z (-0.984375 ms
> from epoch)
> 
> #### Moddable XS
> 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.001Z (1 ms from epoch)
> 1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
> 1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> ```
> 
> And worse, this results in bizarre round-towards-epoch behavior in new Date
> construction (as seen above).

I think this is the right behavior. TimeClip is trunc.
Let's see

`new Date(-0.484375).valueOf()` => 0 in all engines.

> 
> Any rounding behavior would correct this bug, but I think it would be best
> to align with the majority of other implementations in specifically rounding
> time values down (i.e., towards negative infinity), yielding results
> equivalent to ignoring all digits beyond millisecond precision.
> 
> 
> Also reported to test262 for coverage:
> https://github.com/tc39/test262/issues/3437

[Radar WebKit Bug Importer]

<rdar://problem/90587492>

[Yusuke Suzuki]

Created attachment 455269 [details]
Patch

[EWS]

Committed r291603 (248696@main): <https://commits.webkit.org/248696@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455269 [details].

[Richard Gibson]

(In reply to Yusuke Suzuki from comment #1)
> (In reply to Richard Gibson from comment #0)
> > #### ChakraCore, engine262, GraalJS, Hermes, SpiderMonkey, V8
> > 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
> > 1969-12-31T23:59:59.999515625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> > 1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> 
> I think this looks very strange: if we perform TimeClip operation
> (https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-timeclip),
> then all results should be 0 ms since TimeClip is effectively trunc
> operation.
> 
> I think the reasonable behavior is +0 for all three inputs.

I disagree, because privileging January 1 1970 as an attractor isn't helpful—its status as the epoch is a curiosity of internal representation by computers, and the rest of the world would be justified in expecting that 1970-01-01T00:00:00.000500001Z rounds to milliseconds in the same direction as 1969-12-31T00:00:00.000500001Z.

> > 
> > #### JavaScriptCore
> > 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0.500001 ms from
> > epoch)
> > 1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (-0.484375 ms
> > from epoch)
> > 1969-12-31T23:59:59.999015625Z => 1970-01-01T00:00:00.000Z (-0.984375 ms
> > from epoch)
> > 
> > #### Moddable XS
> > 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.001Z (1 ms from epoch)
> > 1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
> > 1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> > ```
> > 
> > And worse, this results in bizarre round-towards-epoch behavior in new Date
> > construction (as seen above).
> 
> I think this is the right behavior. TimeClip is trunc.
> Let's see
> 
> `new Date(-0.484375).valueOf()` => 0 in all engines.

Date.parse doesn't depend upon TimeClip, and couldn't even use that operation until first parsing a number from input. The above expression bypasses Date.parse, and is instead subject only to the usual treatment of non-integer input in JavaScript where integers are needed. Truncating _numbers_ towards zero is expected, while rounding _datetimes_ towards the start of 1970 is surprising, especially when it results in rollover of coarser unit components due to arithmetic carry (e.g., when input like "1969-…" gets mapped to 1970).

To reiterate, every implementation except JSC and XS round down (i.e., towards negative infinity) rather than truncate (i.e., towards zero). XS is [updating to align with the majority](https://github.com/Moddable-OpenSource/moddable/issues/879), and I would recommend that JSC do the same.

But truncation *is* compliant with the underspecified [Date.parse](https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-date.parse), and JSC is free to break from the herd in this way.

[Yusuke Suzuki]

(In reply to Richard Gibson from comment #5)
> (In reply to Yusuke Suzuki from comment #1)
> > (In reply to Richard Gibson from comment #0)
> > > #### ChakraCore, engine262, GraalJS, Hermes, SpiderMonkey, V8
> > > 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
> > > 1969-12-31T23:59:59.999515625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> > > 1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> > 
> > I think this looks very strange: if we perform TimeClip operation
> > (https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-timeclip),
> > then all results should be 0 ms since TimeClip is effectively trunc
> > operation.
> > 
> > I think the reasonable behavior is +0 for all three inputs.
> 
> I disagree, because privileging January 1 1970 as an attractor isn't
> helpful—its status as the epoch is a curiosity of internal representation by
> computers, and the rest of the world would be justified in expecting that
> 1970-01-01T00:00:00.000500001Z rounds to milliseconds in the same direction
> as 1969-12-31T00:00:00.000500001Z.

I think handling differently in Date.parse is inconsistent. The rest of the spec is consistently using TimeClip, and that is trunc to the epoch.
If it looks strange, then we should change TimeClip first.

> 
> > > 
> > > #### JavaScriptCore
> > > 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.000Z (0.500001 ms from
> > > epoch)
> > > 1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (-0.484375 ms
> > > from epoch)
> > > 1969-12-31T23:59:59.999015625Z => 1970-01-01T00:00:00.000Z (-0.984375 ms
> > > from epoch)
> > > 
> > > #### Moddable XS
> > > 1970-01-01T00:00:00.000500001Z => 1970-01-01T00:00:00.001Z (1 ms from epoch)
> > > 1969-12-31T23:59:59.999515625Z => 1970-01-01T00:00:00.000Z (0 ms from epoch)
> > > 1969-12-31T23:59:59.999015625Z => 1969-12-31T23:59:59.999Z (-1 ms from epoch)
> > > ```
> > > 
> > > And worse, this results in bizarre round-towards-epoch behavior in new Date
> > > construction (as seen above).
> > 
> > I think this is the right behavior. TimeClip is trunc.
> > Let's see
> > 
> > `new Date(-0.484375).valueOf()` => 0 in all engines.
> 
> Date.parse doesn't depend upon TimeClip, and couldn't even use that
> operation until first parsing a number from input. The above expression
> bypasses Date.parse, and is instead subject only to the usual treatment of
> non-integer input in JavaScript where integers are needed. Truncating
> _numbers_ towards zero is expected, while rounding _datetimes_ towards the
> start of 1970 is surprising, especially when it results in rollover of
> coarser unit components due to arithmetic carry (e.g., when input like
> "1969-…" gets mapped to 1970).

I don't think it is surprising because of how TimeClip is implemented.
TimeClip is not a number function, it is intended to be used only for time value (as the name is specified).
It is used when the input time is including a fraction, and in this Date.parse case, the parser is parsing sub-milliseconds in JSC.
So, after getting an input, performing TimeClip is reasonable as it is used in the other places which take double input.
I think whether we should parse and recognize sub-milliseconds in Date.parse is different question and I think using TimeClip is just correct behavior.

> 
> To reiterate, every implementation except JSC and XS round down (i.e.,
> towards negative infinity) rather than truncate (i.e., towards zero). XS is
> [updating to align with the
> majority](https://github.com/Moddable-OpenSource/moddable/issues/879), and I
> would recommend that JSC do the same.
> 
> But truncation *is* compliant with the underspecified
> [Date.parse](https://tc39.es/ecma262/multipage/numbers-and-dates.html#sec-
> date.parse), and JSC is free to break from the herd in this way.

[Richard Gibson]

Look at *how* TimeClip is used...
* In the Date constructor with a single argument, where the value to be clipped is either (step 4.b) the time value from an existing Date instance, or (step 4.c.ii) a time value output from the Date.parse algorithm, or (step 4.c.iii) the result of interpreting the argument as a number (and note that the first two cases are _already_ fraction-free time values).
* In the Date constructor with more than one argument (step 5), where the value to be clipped is derived from MakeDate's arithmetic combination of MakeDay output with MakeTime output (both of which are fraction-free, and theirselves derived from truncating the _numeric_ time component arguments).
* In Date.UTC and Date.prototype.set{,UTC}{Date,FullYear,Hours,Milliseconds,Minutes,Month,Seconds} and Date.prototype.setYear, all of which use a MakeDate(…) pattern analogous to the multi-argument Date constructor case.
* In Date.setTime, where the value to be clipped is the result of interpreting the argument as a number.

The only cases where TimeClip input can have a nonempty fractional component are precisely the two in which a date function treats a single argument as a number (`Date(nonDateNumberable)` and `Date.setTime(numberable)`), which are expected to experience truncation towards zero per the usual ToNumber pattern of deriving integers from inputs. In all other cases, the TimeClip input is **already** fraction-free and its primary purpose is replacing high-magnitude Number values (specifically those outside the valid range of time values) with NaN.

[Richard Gibson]

Correction: "Date.setTime" → "Date.prototype.setTime"

[Yusuke Suzuki]

(In reply to Richard Gibson from comment #7)
> The only cases where TimeClip input can have a nonempty fractional component
> are precisely the two in which a date function treats a single argument as a
> number (`Date(nonDateNumberable)` and `Date.setTime(numberable)`), which are
> expected to experience truncation towards zero per the usual ToNumber
> pattern of deriving integers from inputs. In all other cases, the TimeClip
> input is **already** fraction-free and its primary purpose is replacing
> high-magnitude Number values (specifically those outside the valid range of
> time values) with NaN.

This means that,

1. In the other places, it is fraction-free, thus there is no problem between trunc / floor.
2. Some places are getting fractions, and they are always using TimeClip.

I don't see any other places using floor to the fractions, thus, if we can get the fraction,
using TimeClip sounds consistent.

Can you show me the example which takes fraction and using floor to get time value?

[Richard Gibson]

(In reply to Yusuke Suzuki from comment #9)
> 1. In the other places, it is fraction-free, thus there is no problem
> between trunc / floor.
> 2. Some places are getting fractions, and they are always using TimeClip.
> 
> I don't see any other places using floor to the fractions, thus, if we can
> get the fraction,
> using TimeClip sounds consistent.
> 
> Can you show me the example which takes fraction and using floor to get time
> value?

I'm sorry, I don't understand this. Can you elaborate and/or rephrase?

What I'm saying is that there are precisely two code paths in which TimeClip input is not guaranteed to be fraction-free, and both of those involve a code-accessible function interpreting its input as an integer. There is an overwhelming convention in ECMAScript for such interpretation to use ToIntegerOrInfinity (which coerces with ToNumber, replaces NaN and -0 with 0, preserves infinities, and truncates fractional values towards 0), but there is no corresponding convention for how to parse strings into time values—and in fact there would be no normative consequences if ToIntegerOrInfinity were entirely removed from TimeClip an instead moved to `Date` and `Date.prototype.setTime`:
```patch
@@ -32241,5 +32241,5 @@ TimeClip (
           1. If _time_ is not finite, return *NaN*.
           1. If abs(ℝ(_time_)) &gt; 8.64 &times; 10<sup>15</sup>, return *NaN*.
-          1. Return 𝔽(! ToIntegerOrInfinity(_time_)).
+          1. Return _time_.
         </emu-alg>
       </emu-clause>
@@ -32444,5 +32444,5 @@ <h1>Date ( ..._values_ )</h1>
                 1. Let _tv_ be the result of parsing _v_ as a date, in exactly the same manner as for the `parse` method (<emu-xref href="#sec-date.parse"></emu-xref>).
               1. Else,
-                1. Let _tv_ be ? ToNumber(_v_).
+                1. Let _tv_ be ? ToIntegerOrInfinity(_v_).
             1. Let _dv_ be TimeClip(_tv_).
           1. Else,
@@ -32875,5 +32875,5 @@ <h1>Date.prototype.setTime ( _time_ )</h1>
         <emu-alg>
           1. Perform ? thisTimeValue(*this* value).
-          1. Let _t_ be ? ToNumber(_time_).
+          1. Let _t_ be ? ToIntegerOrInfinity(_time_).
           1. Let _v_ be TimeClip(_t_).
           1. Set the [[DateValue]] internal slot of this Date object to _v_.
```

But although there is no convention in the spec for how to parse strings into time values, there *is* a majority convention across implementations—and that is to round excess precision towards negative infinity.

[Yusuke Suzuki]

(In reply to Richard Gibson from comment #10)
> (In reply to Yusuke Suzuki from comment #9)
> > 1. In the other places, it is fraction-free, thus there is no problem
> > between trunc / floor.
> > 2. Some places are getting fractions, and they are always using TimeClip.
> > 
> > I don't see any other places using floor to the fractions, thus, if we can
> > get the fraction,
> > using TimeClip sounds consistent.
> > 
> > Can you show me the example which takes fraction and using floor to get time
> > value?
> 
> I'm sorry, I don't understand this. Can you elaborate and/or rephrase?
> 
> What I'm saying is that there are precisely two code paths in which TimeClip
> input is not guaranteed to be fraction-free, and both of those involve a
> code-accessible function interpreting its input as an integer. There is an
> overwhelming convention in ECMAScript for such interpretation to use
> ToIntegerOrInfinity (which coerces with ToNumber, replaces NaN and -0 with
> 0, preserves infinities, and truncates fractional values towards 0), but
> there is no corresponding convention for how to parse strings into time
> values—and in fact there would be no normative consequences if
> ToIntegerOrInfinity were entirely removed from TimeClip an instead moved to
> `Date` and `Date.prototype.setTime`:
> ```patch
> @@ -32241,5 +32241,5 @@ TimeClip (
>            1. If _time_ is not finite, return *NaN*.
>            1. If abs(ℝ(_time_)) &gt; 8.64 &times; 10<sup>15</sup>, return
> *NaN*.
> -          1. Return 𝔽(! ToIntegerOrInfinity(_time_)).
> +          1. Return _time_.
>          </emu-alg>
>        </emu-clause>
> @@ -32444,5 +32444,5 @@ <h1>Date ( ..._values_ )</h1>
>                  1. Let _tv_ be the result of parsing _v_ as a date, in
> exactly the same manner as for the `parse` method (<emu-xref
> href="#sec-date.parse"></emu-xref>).
>                1. Else,
> -                1. Let _tv_ be ? ToNumber(_v_).
> +                1. Let _tv_ be ? ToIntegerOrInfinity(_v_).
>              1. Let _dv_ be TimeClip(_tv_).
>            1. Else,
> @@ -32875,5 +32875,5 @@ <h1>Date.prototype.setTime ( _time_ )</h1>
>          <emu-alg>
>            1. Perform ? thisTimeValue(*this* value).
> -          1. Let _t_ be ? ToNumber(_time_).
> +          1. Let _t_ be ? ToIntegerOrInfinity(_time_).
>            1. Let _v_ be TimeClip(_t_).
>            1. Set the [[DateValue]] internal slot of this Date object to _v_.
> ```
> 
> But although there is no convention in the spec for how to parse strings
> into time values, there *is* a majority convention across
> implementations—and that is to round excess precision towards negative
> infinity.

What I'm saying is that, whenever an input can include fraction, ECMAScript spec is using TimeClip to make it time value.
And if we parse sub-milliseconds (JSC is currently doing), then using TimeClip is consistent and natural way.
And as it is said in https://bugs.webkit.org/show_bug.cgi?id=238050#c6, whether we should recognize sub-milliseconds or ignore them in parsing is yet another thing.