Re-enable tests in DateParse2.js

[xiaoyinl]

In #4067, we've changed the format of Date.toUTCDate for negative years. After this change, some tests in DateParse2.js fail because Date.parse() isn't able to parse negative years in some cases, so these failing tests are disabled in #4067. They should be re-enabled (uncommented) after #4178 is fixed.

Ref: #4067 (comment)

[obastemur]

Really appreciate the effort but appreciate even more if you do not enable / add negative year tests to xplat. (my 2 cents, if it has a runtime cost, please do not even change for Windows too)

Why => xplat negative years time zone info != what we get from Windows. Correcting that, costs lots of if/else/copy. At the moment, toStringAndToUTCStringYearPadding.js is failing on local xplat and I'm planning to disable it there too.

Curious.. why anyone needs negative years?

[obastemur]

See #4325

[dilijev]

Curious.. why anyone needs negative years?

Because it's in the spec.

(Or, more specifically, by my reading of the relevant spec sections, negative years are not out of range and therefore do not cause Date ctor to return NaN and therefore negative years are supported by the spec.)

[obastemur]

Not sure question/answer matches :) Correct me if I'm wrong but, no real app needs negative years, no?

Probably some wouldn't agree but can we consider twice on any extra ops we will add because of making negative years more spec compliant?

[dilijev]

If it slows down the common case, absolutely we should consider this being an issue that is so unimportant that it's not worth fixing. If it only slows down the negative years case, there's no cost to the common case, so I don't see why we couldn't fix it there.

[xiaoyinl]

I will run a benchmark to see if it slows down common cases.

[dilijev]

^ @xiaoyinl: We should be able to tell by analyzing the code: if there's already a branch surrounding the logic for the negative year, adding more logic to that block means that only the negative cases will be affected. If this change would require adding a branch to separate positive and negative cases, then we will need to look at the benchmarks to decide if there's a noticable impact.

[xiaoyinl]

@dilijev There're a few branches added to separate both cases, like L1240-1245, and L1405-1408. But I'm not sure if this cost is negligible or not for common cases.

[dilijev]

Ah yes, those new checks will add non-zero overhead, though without a benchmark it's hard to tell how small the overhead will be (although the branch predictor will probably render it even more negligible for the common case).