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: Revert U+2212 from time zone offset grammar #2856

Merged
merged 3 commits into from
Jul 4, 2024

Conversation

justingrant
Copy link
Collaborator

@justingrant justingrant commented May 23, 2024

This PR is the Temporal counterpart to tc39/ecma262#3334.

See that PR for details.

spec/mainadditions.html Show resolved Hide resolved
@ptomato
Copy link
Collaborator

ptomato commented Jun 13, 2024

This normative change reached consensus at the TC39 meeting of 2024-06-12. It doesn't need to be merged exactly at the same time as tc39/ecma262#3334, however, we should write test262 tests before merging either of them.

Copy link

codecov bot commented Jun 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.39%. Comparing base (82ad54f) to head (ba4365f).
Report is 30 commits behind head on main.

Current head ba4365f differs from pull request most recent head c05b4c5

Please upload reports for the commit c05b4c5 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2856      +/-   ##
==========================================
- Coverage   96.51%   96.39%   -0.12%     
==========================================
  Files          23       23              
  Lines       12432    12105     -327     
  Branches     2258     2208      -50     
==========================================
- Hits        11999    11669     -330     
- Misses        374      377       +3     
  Partials       59       59              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ptomato
Copy link
Collaborator

ptomato commented Jun 27, 2024

Disregard the codecov message - this still needs an implementation. I'll work on the test262 tests and then use that to develop an implementation which I'll tack on to this PR.

@ptomato
Copy link
Collaborator

ptomato commented Jun 28, 2024

Tests are in tc39/test262#4120 and I've added the corresponding implementation to this PR.

justingrant and others added 3 commits July 4, 2024 18:52
This PR is the Temporal counterpart to
tc39/ecma262#3334.

See that PR for details.

This PR is dependent on that PR's approval.
Implementation corresponding to the spec text change in the previous
commit.
@ptomato ptomato merged commit 8875228 into tc39:main Jul 4, 2024
5 checks passed
sosukesuzuki added a commit to sosukesuzuki/WebKit that referenced this pull request Aug 14, 2024
https://bugs.webkit.org/show_bug.cgi?id=278075

Reviewed by NOBODY (OOPS!).

In the latest Temporal spec[1], the U+2212 MINUS SIGN cannot be used to represent negative numbers.
This is consistent with RFC3339[2] and RFC9557[3].

However, the current JSC allows the U+2212 MINUS SIGN within ISO8601 strings.

This patch changes to reject the U+2212 MINUS SIGN as the symbol for negative numbers in ISO8601
strings.

[1]: tc39/proposal-temporal#2856
[2]: https://www.rfc-editor.org/rfc/rfc3339
[3]: https://www.rfc-editor.org/rfc/rfc9557

* JSTests/test262/expectations.yaml:
* JSTests/stress/temporal-duration.js:
(shouldThrow.Temporal.Duration.from):
* JSTests/stress/temporal-instant.js:
* JSTests/stress/temporal-plaindate.js:
* JSTests/stress/temporal-plaindatetime.js:
* JSTests/stress/temporal-plaintime.js:
* JSTests/stress/temporal-timezone.js:
* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::parseDuration):
(JSC::ISO8601::parseTimeZoneNumericUTCOffset):
(JSC::ISO8601::parseUTCOffsetInMinutes):
(JSC::ISO8601::canBeTimeZone):
(JSC::ISO8601::parseTimeZoneBracketedAnnotation):
(JSC::ISO8601::parseTimeZone):
(JSC::ISO8601::parseDate):
sosukesuzuki added a commit to sosukesuzuki/WebKit that referenced this pull request Aug 14, 2024
https://bugs.webkit.org/show_bug.cgi?id=278075

Reviewed by NOBODY (OOPS!).

In the latest Temporal spec[1], the U+2212 MINUS SIGN cannot be used to represent negative numbers.
This is consistent with RFC3339[2] and RFC9557[3].

However, the current JSC allows the U+2212 MINUS SIGN within ISO8601 strings.

This patch changes to reject the U+2212 MINUS SIGN as the symbol for negative numbers in ISO8601
strings.

[1]: tc39/proposal-temporal#2856
[2]: https://www.rfc-editor.org/rfc/rfc3339
[3]: https://www.rfc-editor.org/rfc/rfc9557

* JSTests/test262/expectations.yaml:
* JSTests/stress/temporal-duration.js:
(shouldThrow.Temporal.Duration.from):
* JSTests/stress/temporal-instant.js:
* JSTests/stress/temporal-plaindate.js:
* JSTests/stress/temporal-plaindatetime.js:
* JSTests/stress/temporal-plaintime.js:
* JSTests/stress/temporal-timezone.js:
* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::parseDuration):
(JSC::ISO8601::parseTimeZoneNumericUTCOffset):
(JSC::ISO8601::parseUTCOffsetInMinutes):
(JSC::ISO8601::canBeTimeZone):
(JSC::ISO8601::parseTimeZoneBracketedAnnotation):
(JSC::ISO8601::parseTimeZone):
(JSC::ISO8601::parseDate):
webkit-commit-queue pushed a commit to sosukesuzuki/WebKit that referenced this pull request Aug 15, 2024
https://bugs.webkit.org/show_bug.cgi?id=278075

Reviewed by Ross Kirsling.

In the latest Temporal spec[1], the U+2212 MINUS SIGN cannot be used to represent negative numbers.
This is consistent with RFC3339[2] and RFC9557[3].

However, the current JSC allows the U+2212 MINUS SIGN within ISO8601 strings.

This patch changes to reject the U+2212 MINUS SIGN as the symbol for negative numbers in ISO8601
strings.

[1]: tc39/proposal-temporal#2856
[2]: https://www.rfc-editor.org/rfc/rfc3339
[3]: https://www.rfc-editor.org/rfc/rfc9557

* JSTests/test262/expectations.yaml:
* JSTests/stress/temporal-duration.js:
(shouldThrow.Temporal.Duration.from):
* JSTests/stress/temporal-instant.js:
* JSTests/stress/temporal-plaindate.js:
* JSTests/stress/temporal-plaindatetime.js:
* JSTests/stress/temporal-plaintime.js:
* JSTests/stress/temporal-timezone.js:
* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::parseDuration):
(JSC::ISO8601::parseTimeZoneNumericUTCOffset):
(JSC::ISO8601::parseUTCOffsetInMinutes):
(JSC::ISO8601::canBeTimeZone):
(JSC::ISO8601::parseTimeZoneBracketedAnnotation):
(JSC::ISO8601::parseTimeZone):
(JSC::ISO8601::parseDate):

Canonical link: https://commits.webkit.org/282271@main
webkit-commit-queue pushed a commit to sosukesuzuki/WebKit that referenced this pull request Aug 15, 2024
https://bugs.webkit.org/show_bug.cgi?id=278075

Reviewed by Ross Kirsling.

In the latest Temporal spec[1], the U+2212 MINUS SIGN cannot be used to represent negative numbers.
This is consistent with RFC3339[2] and RFC9557[3].

However, the current JSC allows the U+2212 MINUS SIGN within ISO8601 strings.

This patch changes to reject the U+2212 MINUS SIGN as the symbol for negative numbers in ISO8601
strings.

[1]: tc39/proposal-temporal#2856
[2]: https://www.rfc-editor.org/rfc/rfc3339
[3]: https://www.rfc-editor.org/rfc/rfc9557

* JSTests/test262/expectations.yaml:
* JSTests/stress/temporal-duration.js:
(shouldThrow.Temporal.Duration.from):
* JSTests/stress/temporal-instant.js:
* JSTests/stress/temporal-plaindate.js:
* JSTests/stress/temporal-plaindatetime.js:
* JSTests/stress/temporal-plaintime.js:
* JSTests/stress/temporal-timezone.js:
* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::parseDuration):
(JSC::ISO8601::parseTimeZoneNumericUTCOffset):
(JSC::ISO8601::parseUTCOffsetInMinutes):
(JSC::ISO8601::canBeTimeZone):
(JSC::ISO8601::parseTimeZoneBracketedAnnotation):
(JSC::ISO8601::parseTimeZone):
(JSC::ISO8601::parseDate):

Canonical link: https://commits.webkit.org/282272@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants