Temporal: Reimplement TemporalDuration::total() according to the spec #50468

catamorphism · 2025-09-08T23:12:06Z

`ae59647`

Temporal: Reimplement TemporalDuration::total() according to the spec
https://bugs.webkit.org/show_bug.cgi?id=298561

Reviewed by Yusuke Suzuki.

This enables some of the tests for Temporal/Duration/prototype/total.

This patch was co-authored by Philip Chimento <pchimento@igalia.com>.

* JSTests/stress/temporal-duration.js:
* JSTests/test262/config.yaml:
* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/TemporalDuration.cpp:
(JSC::appendInteger):
(JSC::totalTimeDuration):
(JSC::TemporalDuration::total const):
* Source/JavaScriptCore/runtime/TemporalObject.h:
(JSC::lengthInNanoseconds):

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

5d2b115

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~~
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2025-09-08T23:12:12Z

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

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-09-08T23:14:40Z

EWS run on previous version of this PR (hash 1eae6b0)

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-09-08T23:24:08Z

EWS run on previous version of this PR (hash 67c0a70)

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

Constellation · 2025-09-09T17:40:17Z

Source/JavaScriptCore/runtime/TemporalDuration.cpp

+    StringBuilder result;
+    appendInteger(globalObject, result, static_cast<double>(absInt128(quotient)));
+    RETURN_IF_EXCEPTION(scope, { });
+    result.append('.');
+    result.append(decimalDigits.toString());
+    // NOTE: if result.toString() == 9007199254740992.999,
+    // the result is rounded down to 9007199254740992.
+    // This causes the test262 test
+    // Temporal/Duration/prototype/total/precision-exact-mathematical-values-7.js
+    // to fail when unit=milliseconds, smallerUnit=microseconds, integer=2**53, fraction=1999.
+    return sign * result.toString().toDouble();


This looks very inefficient. Why do we need to do this string and re-parsing?

The code is making up for the lack of float128. The spec calls this out: "The following step cannot be implemented directly using floating-point arithmetic when 𝔽(timeDuration) is not a safe integer."

I followed the polyfill implementation closely, but if you have a better idea, let me know.

Apparently, boa temporal etc. is not doing this inefficient implementation. So, r-.
Translating JS polyfill to C++ is not what we should follow.

Boa fails the test Temporal/Duration/prototype/total/precision-mathematical-values-6.js. (see https://boajs.dev/conformance?version=main&testPath=built-ins%2FTemporal%2FDuration%2Fprototype%2Ftotal ). There is a note to this effect here ("TODO Fix: Arithemtic on floating point numbers is not safe. According to NOTE 2 in the spec").

On the other hand, tests like this are the uncommon case, so I'll try to think about whether there's a way to optimize the common case.

@Constellation I took a look at what SpiderMonkey does; their implementation is conformant (at least for the Duration/prototype/total tests). So I adapted that code instead of using the algorithm from the polyfill. Let me know what you think of this approach.

I've been told it may not be an option to commit non-BSD-licensed code to WebKit, so this approach might not be feasible. In any event, I'll be on vacation until Sept. 29; will reply to any comments when I return.

Constellation · 2025-09-09T20:53:03Z

Source/JavaScriptCore/runtime/TemporalDuration.cpp

+    StringBuilder result;
+    appendInteger(globalObject, result, static_cast<double>(absInt128(quotient)));
+    RETURN_IF_EXCEPTION(scope, { });
+    result.append('.');
+    result.append(decimalDigits.toString());
+    // NOTE: if result.toString() == 9007199254740992.999,
+    // the result is rounded down to 9007199254740992.
+    // This causes the test262 test
+    // Temporal/Duration/prototype/total/precision-exact-mathematical-values-7.js
+    // to fail when unit=milliseconds, smallerUnit=microseconds, integer=2**53, fraction=1999.
+    return sign * result.toString().toDouble();


Apparently, boa temporal etc. is not doing this inefficient implementation. So, r-.
Translating JS polyfill to C++ is not what we should follow.

webkit-early-warning-system · 2025-09-10T21:05:24Z

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

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-09-10T21:08:30Z

EWS run on previous version of this PR (hash 19d0e4c)

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-09-10T23:41:56Z

EWS run on previous version of this PR (hash 2ca4f28)

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-09-30T21:02:44Z

EWS run on previous version of this PR (hash 1cc23b2)

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

catamorphism · 2025-09-30T21:05:17Z

@Constellation I've eliminated the MPLed code and replaced it with an implementation contributed by @ptomato, who reports that this code is twice as fast as the equivalent code in Firefox. Hopefully this addresses the concerns about performance.

webkit-early-warning-system · 2025-10-01T04:06:21Z

EWS run on current version of this PR (hash 5d2b115)

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~~
✅ 🛠 🧪 unsafe-merge	✅ 🛠 tv	✅ 🛠 mac-safer-cpp	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch
	✅ 🛠 watch-sim

catamorphism · 2025-10-01T04:07:32Z

Force-push was to rebase and resolve a conflict in expectations.yaml.

https://bugs.webkit.org/show_bug.cgi?id=298561 Reviewed by Yusuke Suzuki. This enables some of the tests for Temporal/Duration/prototype/total. This patch was co-authored by Philip Chimento <pchimento@igalia.com>. * JSTests/stress/temporal-duration.js: * JSTests/test262/config.yaml: * JSTests/test262/expectations.yaml: * Source/JavaScriptCore/runtime/TemporalDuration.cpp: (JSC::appendInteger): (JSC::totalTimeDuration): (JSC::TemporalDuration::total const): * Source/JavaScriptCore/runtime/TemporalObject.h: (JSC::lengthInNanoseconds): Canonical link: https://commits.webkit.org/300802@main

webkit-commit-queue · 2025-10-01T05:01:16Z

Committed 300802@main (ae59647): https://commits.webkit.org/300802@main

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

catamorphism self-assigned this Sep 8, 2025

catamorphism requested a review from a team as a code owner September 8, 2025 23:12

catamorphism force-pushed the balance-precision-part-7 branch from c4b6238 to 1eae6b0 Compare September 8, 2025 23:14

catamorphism force-pushed the balance-precision-part-7 branch from 1eae6b0 to 67c0a70 Compare September 8, 2025 23:24

Constellation reviewed Sep 9, 2025

View reviewed changes

Constellation requested changes Sep 9, 2025

View reviewed changes

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

catamorphism force-pushed the balance-precision-part-7 branch from 2ca4f28 to 1cc23b2 Compare September 30, 2025 21:02

Constellation approved these changes Oct 1, 2025

View reviewed changes

catamorphism force-pushed the balance-precision-part-7 branch from 1cc23b2 to 5d2b115 Compare October 1, 2025 04:06

catamorphism 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 Oct 1, 2025

Constellation added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Oct 1, 2025

webkit-commit-queue force-pushed the balance-precision-part-7 branch from 5d2b115 to ae59647 Compare October 1, 2025 05:01

webkit-commit-queue merged commit ae59647 into WebKit:main Oct 1, 2025

webkit-commit-queue removed unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing request-merge-queue Request a pull request to be added to merge-queue once ready labels Oct 1, 2025

Temporal: Reimplement TemporalDuration::total() according to the spec #50468

Temporal: Reimplement TemporalDuration::total() according to the spec #50468

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

catamorphism commented Sep 30, 2025

Uh oh!

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

Uh oh!

Uh oh!

catamorphism commented Oct 1, 2025

Uh oh!

webkit-commit-queue commented Oct 1, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

catamorphism commented Sep 8, 2025 •

edited by webkit-early-warning-system

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading

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

edited

Loading