Skip to content

[JSC] Refactor Temporal calendar annotation key parsing#43849

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
ptomato:temporal-calendar-keys-part1
Apr 10, 2025
Merged

[JSC] Refactor Temporal calendar annotation key parsing#43849
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
ptomato:temporal-calendar-keys-part1

Conversation

@ptomato

@ptomato ptomato commented Apr 9, 2025 •
edited by webkit-early-warning-system
Loading

Copy link
Copy Markdown
Contributor

fec80ce

[JSC] Refactor Temporal calendar annotation key parsing
https://bugs.webkit.org/show_bug.cgi?id=223166

Reviewed by Justin Michaud and Sosuke Suzuki.

This does not change anything currently, but is groundwork for
implementing a normative change to the Temporal proposal:
tc39/proposal-temporal#2397
It changes parseCalendar() to handle unknown annotation keys and known
annotation keys with the `!` critical flag. (Although those are still
disallowed by canBeCalendar() at this time, I will send a follow-up PR
for that.)

Note this code was already submitted as part of
#36361 which I'm trying to split up
into smaller, more easily reviewable parts.

* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::parseCalendar):

Canonical link: https://commits.webkit.org/293522@main

2e7faa2

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@ptomato ptomato requested a review from a team as a code owner April 9, 2025 18:38
@webkit-early-warning-system

webkit-early-warning-system commented Apr 9, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on previous version of this PR (hash 5fe9bd6)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

justinmichaud
justinmichaud reviewed Apr 9, 2025
View reviewed changes
Comment thread Source/JavaScriptCore/runtime/ISO8601.cpp Outdated
sosukesuzuki
sosukesuzuki approved these changes Apr 10, 2025
View reviewed changes

@sosukesuzuki sosukesuzuki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r=me

Comment thread Source/JavaScriptCore/runtime/ISO8601.cpp Outdated
@ptomato ptomato force-pushed the temporal-calendar-keys-part1 branch from 5fe9bd6 to 2e7faa2 Compare April 10, 2025 01:01
@webkit-early-warning-system

webkit-early-warning-system commented Apr 10, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

EWS run on current version of this PR (hash 2e7faa2)

Details

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ❌ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@justinmichaud justinmichaud added the merge-queue Applied to send a pull request to merge-queue label Apr 10, 2025
@webkit-commit-queue webkit-commit-queue force-pushed the temporal-calendar-keys-part1 branch from 2e7faa2 to 4e5acc7 Compare April 10, 2025 15:41
@catamorphism @justinmichaud
[JSC] Refactor Temporal calendar annotation key parsing
fec80ce 
https://bugs.webkit.org/show_bug.cgi?id=223166

Reviewed by Justin Michaud and Sosuke Suzuki.

This does not change anything currently, but is groundwork for
implementing a normative change to the Temporal proposal:
tc39/proposal-temporal#2397
It changes parseCalendar() to handle unknown annotation keys and known
annotation keys with the `!` critical flag. (Although those are still
disallowed by canBeCalendar() at this time, I will send a follow-up PR
for that.)

Note this code was already submitted as part of
WebKit#36361 which I'm trying to split up
into smaller, more easily reviewable parts.

* Source/JavaScriptCore/runtime/ISO8601.cpp:
(JSC::ISO8601::parseCalendar):

Canonical link: https://commits.webkit.org/293522@main
@webkit-commit-queue webkit-commit-queue force-pushed the temporal-calendar-keys-part1 branch from 4e5acc7 to fec80ce Compare April 10, 2025 15:43
@webkit-commit-queue

webkit-commit-queue commented Apr 10, 2025

Copy link
Copy Markdown
Collaborator

Committed 293522@main (fec80ce): https://commits.webkit.org/293522@main

Reviewed commits have been landed. Closing PR #43849 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit fec80ce into WebKit:main Apr 10, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Apr 10, 2025
@ptomato ptomato deleted the temporal-calendar-keys-part1 branch April 10, 2025 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@justinmichaud justinmichaud justinmichaud approved these changes

@sosukesuzuki sosukesuzuki sosukesuzuki approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ptomato @webkit-early-warning-system @webkit-commit-queue @justinmichaud @sosukesuzuki @catamorphism