Skip to content
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: Clarify validity of negative expanded year 0. #2550

Merged
merged 1 commit into from
Aug 3, 2022

Conversation

EricIO
Copy link
Contributor

@EricIO EricIO commented Oct 10, 2021

Following a discussion in
tc39/proposal-temporal#1753 there is some
reason to clarify the parsing of a negative expanded year 0 (-000000).

There is currently some discrepancy in how different implementors
parse this, some rejecting and some accepting a negative expanded year
0.

@ljharb
Copy link
Member

ljharb commented Oct 10, 2021

Can you provide more details about current web reality here?

A normative change requires consensus, which includes the implementers who’d need to change being willing to do so, so we need to be clear about who those are and what needs changing.

@ljharb ljharb added needs consensus This needs committee consensus before it can be eligible to be merged. needs data This PR needs more information; such as web compatibility data, “web reality” (what all engines do)… normative change Affects behavior required to correctly evaluate some ECMAScript source text labels Oct 10, 2021
@EricIO
Copy link
Contributor Author

EricIO commented Oct 10, 2021

Date.parse("-000000-01-01T00:00:00.000Z")

SpiderMonkey: -62167219200000
JSC: -62167219200000
V8: NaN

@ljharb
Copy link
Member

ljharb commented Oct 11, 2021

It’d be great to run https://npmjs.com/eshost and get data for all the engines :-) but that’s a great start

@arhadthedev
Copy link
Contributor

@ljharb, here are the results for eshost -se 'Date.parse("-000000-01-01T00:00:00.000Z")':

#### ChakraCore, engine262, V8
NaN

#### GraalJS, JavaScriptCore, Moddable XS, QuickJS, SpiderMonkey
-62167219200000

Details on engines for reproducibility:

┌────────────────┬───────────┬────────────────────────────────────────────┬──────┬────────────────────────────────────────────────┐
│ name           │ type      │ path                                       │ args │ tags                                           │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ ChakraCore     │ ch        │ /home/runner/.esvu/bin/chakra              │      │ 1_11_24,web                                    │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ GraalJS        │ graaljs   │ /home/runner/.esvu/bin/graaljs             │      │ 21.2.0                                         │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ JavaScriptCore │ jsc       │ /home/runner/.esvu/bin/jsc                 │      │ 283881,web                                     │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ Moddable XS    │ xs        │ /home/runner/.esvu/bin/xs                  │      │ 10.5.0,embedded                                │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ QuickJS        │ qjs       │ /home/runner/.esvu/bin/quickjs-run-test262 │      │ 2021-03-27,embedded                            │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ SpiderMonkey   │ jsshell   │ /home/runner/.esvu/bin/sm                  │      │ 95.0a1#20211010214947,web                      │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ V8             │ d8        │ /home/runner/.esvu/bin/v8                  │      │ 9.7.3,web                                      │
├────────────────┼───────────┼────────────────────────────────────────────┼──────┼────────────────────────────────────────────────┤
│ engine262      │ engine262 │ /home/runner/.esvu/bin/engine262           │      │ 0.0.1-f47d7da0653c7b2f0417395794a5259c2287cc44 │
└────────────────┴───────────┴────────────────────────────────────────────┴──────┴────────────────────────────────────────────────┘

It’d be great to run https://npmjs.com/eshost

Unfortunately, a newcomer will install a library and will stuck, so I'd rather refer to https://npmjs.com/eshost-cli (plus https://npmjs.com/esvu to properly install engines and feed them to eshost).

@ljharb ljharb added needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 web reality and removed needs data This PR needs more information; such as web compatibility data, “web reality” (what all engines do)… labels Oct 11, 2021
@ljharb
Copy link
Member

ljharb commented Oct 11, 2021

Thanks, I've added this to the next meeting's agenda.

@gibson042
Copy link
Contributor

ISO 8601-1:2019 §3.2.4 documents use of ± for "a plus sign [“+”] to represent a positive value or zero (the plus sign shall not be omitted), or a minus sign [“-”] otherwise" and employs that in describing expanded representations as in "[±][year(i)…". ISO 8601-2:2019 §4.4.1.2 makes it even more clear, extending part 1 to allow "negative values for years prior to year zero (0)". Further, ECMA-262 itself already defines "The year 0 is considered positive and hence prefixed with a + sign." Unless there's specific reason to support "-000000", I don't think it should be allowed.

@EricIO
Copy link
Contributor Author

EricIO commented Oct 25, 2021

I'm happy with both outcomes, I think the main issue is that whatever we choose should be clarified in the text given the current divergence/confusion amongst the different implementations (given that this all started with a patch to firefox I submitted to disallow "-000000" it would indeed be nice to not "waste" that work :) )

@ljharb
Copy link
Member

ljharb commented Oct 25, 2021

For Temporal, the committee provided consensus today to disallow negative year zeros.

For Date.parse, let's update this PR to clarify the prose that it is disallowed, and then hopefully we can hear from folks from the relevant engines on the thread affirming that the change is acceptable; and we can seek consensus in plenary for that final change once ready.

@EricIO
Copy link
Contributor Author

EricIO commented Nov 7, 2021

Sorry for the silence. Here is an updated version that clarifies the text in the expanded year section to disallow -000000.

@ljharb ljharb added the needs data This PR needs more information; such as web compatibility data, “web reality” (what all engines do)… label Nov 7, 2021
spec.html Outdated
@@ -32067,7 +32067,7 @@ <h1>Date Time String Format</h1>

<emu-clause id="sec-expanded-years" oldids="sec-extended-years">
<h1>Expanded Years</h1>
<p>Covering the full time value range of approximately 273,790 years forward or backward from 1 January 1970 (<emu-xref href="#sec-time-values-and-time-range"></emu-xref>) requires representing years before 0 or after 9999. ISO 8601 permits expansion of the year representation, but only by mutual agreement of the partners in information interchange. In the simplified ECMAScript format, such an expanded year representation shall have 6 digits and is always prefixed with a + or - sign. The year 0 is considered positive and hence prefixed with a + sign. Strings matching the <emu-xref href="#sec-date-time-string-format">Date Time String Format</emu-xref> with expanded years representing instants in time outside the range of a time value are treated as unrecognizable by <emu-xref href="#sec-date.parse">`Date.parse`</emu-xref> and cause that function to return *NaN* without falling back to implementation-specific behaviour or heuristics.</p>
<p>Covering the full time value range of approximately 273,790 years forward or backward from 1 January 1970 (<emu-xref href="#sec-time-values-and-time-range"></emu-xref>) requires representing years before 0 or after 9999. ISO 8601 permits expansion of the year representation, but only by mutual agreement of the partners in information interchange. In the simplified ECMAScript format, such an expanded year representation shall have 6 digits and is always prefixed with a + or - sign. The year 0 is considered positive and must be prefixed with a + sign, 0 prefixed with a - sign must be rejected. Strings matching the <emu-xref href="#sec-date-time-string-format">Date Time String Format</emu-xref> with expanded years representing instants in time outside the range of a time value are treated as unrecognizable by <emu-xref href="#sec-date.parse">`Date.parse`</emu-xref> and cause that function to return *NaN* without falling back to implementation-specific behaviour or heuristics.</p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<p>Covering the full time value range of approximately 273,790 years forward or backward from 1 January 1970 (<emu-xref href="#sec-time-values-and-time-range"></emu-xref>) requires representing years before 0 or after 9999. ISO 8601 permits expansion of the year representation, but only by mutual agreement of the partners in information interchange. In the simplified ECMAScript format, such an expanded year representation shall have 6 digits and is always prefixed with a + or - sign. The year 0 is considered positive and must be prefixed with a + sign, 0 prefixed with a - sign must be rejected. Strings matching the <emu-xref href="#sec-date-time-string-format">Date Time String Format</emu-xref> with expanded years representing instants in time outside the range of a time value are treated as unrecognizable by <emu-xref href="#sec-date.parse">`Date.parse`</emu-xref> and cause that function to return *NaN* without falling back to implementation-specific behaviour or heuristics.</p>
<p>Covering the full time value range of approximately 273,790 years forward or backward from 1 January 1970 (<emu-xref href="#sec-time-values-and-time-range"></emu-xref>) requires representing years before 0 or after 9999. ISO 8601 permits expansion of the year representation, but only by mutual agreement of the partners in information interchange. In the simplified ECMAScript format, such an expanded year representation shall have 6 digits and is always prefixed with a + or - sign. The year 0 is considered positive and must be prefixed with a + sign. The representation of the year 0 as -000000 is invalid. Strings matching the <emu-xref href="#sec-date-time-string-format">Date Time String Format</emu-xref> with expanded years representing instants in time outside the range of a time value are treated as unrecognizable by <emu-xref href="#sec-date.parse">`Date.parse`</emu-xref> and cause that function to return *NaN* without falling back to implementation-specific behaviour or heuristics.</p>

Two sentences. Also clarified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github won't allow me to commit this suggestion. Perhaps someone with write permission must do it? Seems like a better wording to me so I'd be happy with it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github won't allow me to commit this suggestion.

If you mean via the "Commit suggestion" button, yeah that's never worked.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it doesn't work when the suggestion is left by someone with write access. You can add it on the command line tho.

ljharb pushed a commit to EricIO/ecma262 that referenced this pull request Nov 18, 2021
Following a discussion in
tc39/proposal-temporal#1753 there is some
reason to clarify the parsing of a negative expanded year 0 (-000000).

There is currently some discreprancy in how different implementors
parse this, some rejecting and some accepting a negative expanded year
0.
@ljharb ljharb added has consensus This has committee consensus. and removed needs consensus This needs committee consensus before it can be eligible to be merged. needs data This PR needs more information; such as web compatibility data, “web reality” (what all engines do)… labels Nov 18, 2021
@ljharb
Copy link
Member

ljharb commented Mar 9, 2022

@ptomato ping for getting this on your radar; is someone working on test262 tests for this change, so we can land this normative PR?

@ptomato
Copy link
Contributor

ptomato commented Mar 9, 2022

Test262 tests for the Temporal part of it have already landed: tc39/test262@4596c7e

I assumed someone else was going to do the legacy Date part of it... it should be more straightforward than for Temporal, since IIUC only two entry points have to be covered. I can do it eventually, but I have to concentrate on Temporal stuff until the March plenary.

@ljharb
Copy link
Member

ljharb commented Mar 9, 2022

I believe this PR can't land until those tests are done; it's fine if it needs to wait until after March, it'll just miss the ES2022 train.

@ptomato
Copy link
Contributor

ptomato commented Apr 9, 2022

This now has tests at tc39/test262#3470

@ptomato ptomato removed the needs test262 tests The proposal should specify how to test an implementation. Ideally via github.com/tc39/test262 label Apr 9, 2022
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request May 10, 2022
https://bugs.webkit.org/show_bug.cgi?id=240263

Reviewed by Yusuke Suzuki.

As of the following two PRs, -000000 is officially disallowed as a representation of the year zero in ISO date strings.
tc39/ecma262#2550
tc39/proposal-temporal#1992

This patch implements the change for Temporal and Date alike.

* test262/expectations.yaml:
Mark 24 test cases as passing.

* runtime/ISO8601.cpp:
(JSC::ISO8601::parseDate):

* wtf/DateMath.cpp:
(WTF::parseES5DatePortion):

Canonical link: https://commits.webkit.org/250432@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@293996 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Aug 3, 2022
Following a discussion in
tc39/proposal-temporal#1753 there is some
reason to clarify the parsing of a negative expanded year 0 (-000000).

There is currently some discreprancy in how different implementors
parse this, some rejecting and some accepting a negative expanded year
0.
@ljharb
Copy link
Member

ljharb commented Aug 3, 2022

@EricIO would you mind signing the IPR form? Thanks!

@EricIO
Copy link
Contributor Author

EricIO commented Aug 4, 2022

@EricIO would you mind signing the IPR form? Thanks!

@ljharb Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has consensus This has committee consensus. normative change Affects behavior required to correctly evaluate some ECMAScript source text ready to merge Editors believe this PR needs no further reviews, and is ready to land. web reality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants