Part 4: Refactor implementation of TemporalPlainDate/prototype/add by ptomato · Pull Request #49776 · WebKit/WebKit

ptomato · 2025-08-22T17:20:31Z

`707926f`

Part 4: Refactor implementation of TemporalPlainDate/prototype/add
https://bugs.webkit.org/show_bug.cgi?id=223166

Reviewed by Sosuke Suzuki.

Add a new operation from the spec, TemporalCalendar::addDurationToDate(),
and use it in the implementation of `add`.

This is a refactoring and shouldn't change anything.

(For reference, modifications by Philip Chimento to address review
comments:)

* Remove ISO8601::isValidISODate() and ISO8601::createISODateRecord()
  since they are not used yet in this PR.
* Expose JSC::checkedCastDoubleToInt128 in ISO8601.h header.
* Replace C-style double-to-Int128 casts with checked casts.
* Assert that TemporalDuration::timeDurationFromComponents and
  add24HourDaysToTimeDuration do not overflow. (The limits on individual
  Duration components should ensure that.)

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

0bd083b

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

webkit-early-warning-system · 2025-08-22T17:21:16Z

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

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
🧪 bindings	✅ 🛠 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
🧪 services	✅ 🛠 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

ptomato · 2025-08-22T17:24:02Z

@catamorphism is out today, so here's my attempt to move the Temporal implementation forward on his behalf - note that he wrote all the code here! The only change I made is to fix up the cast style as per #45603 (comment).

sosukesuzuki · 2025-08-26T00:25:53Z

Can you make the commits and PRs follow the WebKit style? There should be one commit per PR.

ptomato · 2025-08-26T01:05:18Z

Can you make the commits and PRs follow the WebKit style? There should be one commit per PR.

Sure. I initially left it separate so it was clear what changes I made to Tim's code.

webkit-early-warning-system · 2025-08-26T01:37:30Z

EWS run on previous version of this PR (hash a791524)

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

webkit-early-warning-system · 2025-08-27T18:01:46Z

EWS run on previous version of this PR (hash 432b163)

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

webkit-early-warning-system · 2025-08-29T15:08:19Z

EWS run on previous version of this PR (hash 193ec95)

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

ptomato · 2025-08-30T01:19:32Z

@sosukesuzuki I think I've addressed all the comments and the tests are now green. (the jsc-armv7-tests is an unrelated failure and I understand it is being looked into)

webkit-early-warning-system · 2025-09-01T17:12:42Z

EWS run on previous version of this PR (hash 3a58783)

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

sosukesuzuki

r=me

Ahmad-S792 · 2025-09-02T06:58:02Z

@ptomato - can you raise bugzilla bug and add it to commit message, it makes it easy in case, if we ned to revert and track (caused by) bugs?

webkit-early-warning-system · 2025-09-02T15:12:18Z

EWS run on current version of this PR (hash 0bd083b)

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

ptomato · 2025-09-02T15:13:03Z

Done.

https://bugs.webkit.org/show_bug.cgi?id=223166 Reviewed by Sosuke Suzuki. Add a new operation from the spec, TemporalCalendar::addDurationToDate(), and use it in the implementation of `add`. This is a refactoring and shouldn't change anything. (For reference, modifications by Philip Chimento to address review comments:) * Remove ISO8601::isValidISODate() and ISO8601::createISODateRecord() since they are not used yet in this PR. * Expose JSC::checkedCastDoubleToInt128 in ISO8601.h header. * Replace C-style double-to-Int128 casts with checked casts. * Assert that TemporalDuration::timeDurationFromComponents and add24HourDaysToTimeDuration do not overflow. (The limits on individual Duration components should ensure that.) Canonical link: https://commits.webkit.org/299436@main

webkit-commit-queue · 2025-09-02T16:19:36Z

Committed 299436@main (707926f): https://commits.webkit.org/299436@main

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

ptomato requested a review from a team as a code owner August 22, 2025 17:20

ptomato self-assigned this Aug 22, 2025

ptomato changed the title ~~fixup: Use static_cast instead of C-style casts~~ Refactor implementation of TemporalPlainDate/prototype/add Aug 22, 2025

sosukesuzuki reviewed Aug 26, 2025

View reviewed changes

Comment thread Source/JavaScriptCore/runtime/TemporalDuration.cpp Outdated

Comment thread Source/JavaScriptCore/runtime/TemporalDuration.cpp Outdated

Comment thread Source/JavaScriptCore/runtime/ISO8601.h Outdated

ptomato changed the title ~~Refactor implementation of TemporalPlainDate/prototype/add~~ Part 4: Refactor implementation of TemporalPlainDate/prototype/add Aug 26, 2025

ptomato force-pushed the balance-precision-part-4 branch from 5ca9835 to a791524 Compare August 26, 2025 01:37

ptomato requested a review from sosukesuzuki August 26, 2025 01:39

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 26, 2025

ptomato removed the merging-blocked Applied to prevent a change from being merged label Aug 27, 2025

ptomato force-pushed the balance-precision-part-4 branch from a791524 to 432b163 Compare August 27, 2025 18:01

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 28, 2025

ptomato removed the merging-blocked Applied to prevent a change from being merged label Aug 28, 2025

ptomato force-pushed the balance-precision-part-4 branch from 432b163 to 193ec95 Compare August 29, 2025 15:08

ptomato added the request-merge-queue Request a pull request to be added to merge-queue once ready label Aug 30, 2025

sosukesuzuki reviewed Aug 30, 2025

View reviewed changes

Comment thread Source/JavaScriptCore/runtime/TemporalDuration.cpp Outdated

ptomato force-pushed the balance-precision-part-4 branch from 193ec95 to 3a58783 Compare September 1, 2025 17:12

ptomato requested a review from sosukesuzuki September 1, 2025 17:13

webkit-ews-buildbot added merging-blocked Applied to prevent a change from being merged and removed request-merge-queue Request a pull request to be added to merge-queue once ready labels Sep 1, 2025

ptomato added request-merge-queue Request a pull request to be added to merge-queue once ready and removed merging-blocked Applied to prevent a change from being merged labels Sep 2, 2025

sosukesuzuki approved these changes Sep 2, 2025

View reviewed changes

ptomato force-pushed the balance-precision-part-4 branch from 3a58783 to 0bd083b Compare September 2, 2025 15:12

ptomato added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Sep 2, 2025

csaavedra added merge-queue Applied to send a pull request to merge-queue and removed request-merge-queue Request a pull request to be added to merge-queue once ready labels Sep 2, 2025

webkit-commit-queue force-pushed the balance-precision-part-4 branch from 0bd083b to 707926f Compare September 2, 2025 16:19

webkit-commit-queue merged commit 707926f into WebKit:main Sep 2, 2025

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Sep 2, 2025

ptomato deleted the balance-precision-part-4 branch September 2, 2025 16:37

Conversation

ptomato commented Aug 22, 2025 • edited by webkit-early-warning-system Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

707926f

Uh oh!

webkit-early-warning-system commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ptomato commented Aug 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sosukesuzuki commented Aug 26, 2025

Uh oh!

ptomato commented Aug 26, 2025

Uh oh!

webkit-early-warning-system commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

webkit-early-warning-system commented Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ptomato commented Aug 30, 2025

Uh oh!

Uh oh!

webkit-early-warning-system commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sosukesuzuki left a comment

Choose a reason for hiding this comment

Uh oh!

Ahmad-S792 commented Sep 2, 2025

Uh oh!

webkit-early-warning-system commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ptomato commented Sep 2, 2025

Uh oh!

webkit-commit-queue commented Sep 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ptomato commented Aug 22, 2025 •

edited by webkit-early-warning-system

Loading

`707926f`

webkit-early-warning-system commented Aug 22, 2025 •

edited

Loading

webkit-early-warning-system commented Aug 26, 2025 •

edited

Loading

webkit-early-warning-system commented Aug 27, 2025 •

edited

Loading

webkit-early-warning-system commented Aug 29, 2025 •

edited

Loading

webkit-early-warning-system commented Sep 1, 2025 •

edited

Loading

webkit-early-warning-system commented Sep 2, 2025 •

edited

Loading