Skip to content

[Temporal] Fix order of operations in Temporal.PlainTime.from#56210

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
catamorphism:plainmonthday-plainyearmonth-split
Jan 10, 2026
Merged

[Temporal] Fix order of operations in Temporal.PlainTime.from#56210
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
catamorphism:plainmonthday-plainyearmonth-split

Conversation

@catamorphism
Copy link
Contributor

@catamorphism catamorphism commented Jan 7, 2026 •
edited by webkit-early-warning-system
Loading

66872df

[Temporal] Fix order of operations in Temporal.PlainTime.from

https://bugs.webkit.org/show_bug.cgi?id=305061

Reviewed by Yusuke Suzuki.

According to the spec, the overflow option should be read after the fields
when calling `from` on a property bag. This change improves the result for
the test test/built-ins/Temporal/PlainTime/from/order-of-operations.js (it
now fails because ZonedDateTime is not implemented).

* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/TemporalObject.cpp:
(JSC::doubleNumberOption):
* Source/JavaScriptCore/runtime/TemporalPlainDatePrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainDateTimePrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainTime.cpp:
(JSC::TemporalPlainTime::from):
* Source/JavaScriptCore/runtime/TemporalPlainTime.h:
* Source/JavaScriptCore/runtime/TemporalPlainTimeConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainTimePrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):

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

41c604b

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@catamorphism catamorphism force-pushed the plainmonthday-plainyearmonth-split branch from 3eec428 to da521c0 Compare January 7, 2026 19:55
@catamorphism catamorphism requested a review from a team as a code owner January 7, 2026 19:55
@catamorphism catamorphism self-assigned this Jan 7, 2026
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jan 7, 2026 •
edited
Loading

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

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 ❌ 🧪 api-mac-debug   🛠 wpe-cairo-libwebrtc
❌ 🛠 🧪 jsc ❌ 🧪 api-ios ❌ 🧪 mac-wk1   🛠 gtk
❌ 🛠 🧪 jsc-arm64 ❌ 🛠 vision ❌ 🧪 mac-wk2   🧪 gtk-wk2
❌ 🛠 vision-sim ❌ 🧪 mac-AS-debug-wk2   🧪 api-gtk
⏳ 🧪 vision-wk2 ❌ 🧪 mac-wk2-stress   🛠 playstation
❌ 🛠 tv ❌ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
❌ 🛠 tv-sim   🛠 mac-safer-cpp ⏳ 🧪 jsc-armv7-tests
❌ 🛠 watch
  🛠 watch-sim

@catamorphism catamorphism added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Jan 7, 2026
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jan 7, 2026
@catamorphism catamorphism force-pushed the plainmonthday-plainyearmonth-split branch from da521c0 to 588507b Compare January 7, 2026 20:04
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jan 7, 2026 •
edited
Loading

EWS run on previous version of this PR (hash 588507b)

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

Constellation
Constellation requested changes Jan 8, 2026
View reviewed changes
Source/JavaScriptCore/runtime/TemporalPlainTimeConstructor.cpp Outdated
RELEASE_AND_RETURN(scope, JSValue::encode(TemporalPlainTime::create(vm, globalObject->plainTimeStructure(), jsCast<TemporalPlainTime*>(itemValue)->plainTime())));
if (itemValue.inherits<TemporalPlainTime>()) {
// Validate overflow -- see step 2(a)(ii) of ToTemporalTime
toTemporalOverflow(globalObject, options);
Copy link
Member

@Constellation Constellation Jan 8, 2026

Choose a reason for hiding this comment

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

options can be nullptr, right?

Copy link
Contributor Author

@catamorphism catamorphism Jan 8, 2026

Choose a reason for hiding this comment

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

Yes -- added a null check.

@catamorphism catamorphism force-pushed the plainmonthday-plainyearmonth-split branch from 588507b to 5f0488d Compare January 8, 2026 01:07
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jan 8, 2026 •
edited
Loading

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

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
❌ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@catamorphism 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 Jan 8, 2026
@ptomato ptomato 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 Jan 8, 2026
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Jan 8, 2026

Commit message contains (OOPS!), blocking PR #56210. Details: Build #29044

@webkit-commit-queue webkit-commit-queue added merging-blocked Applied to prevent a change from being merged and removed merge-queue Applied to send a pull request to merge-queue labels Jan 8, 2026
@catamorphism catamorphism force-pushed the plainmonthday-plainyearmonth-split branch from 5f0488d to 41c604b Compare January 8, 2026 22:55
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jan 8, 2026 •
edited
Loading

EWS run on current version of this PR (hash 41c604b)

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 ✅ 🧪 api-mac-debug ✅ 🛠 wpe-cairo-libwebrtc
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-wk2-stress   🛠 playstation
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv ✅ 🧪 mac-intel-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🛠 mac-safer-cpp ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@catamorphism 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 Jan 8, 2026
@Constellation Constellation added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed request-merge-queue Request a pull request to be added to merge-queue once ready labels Jan 10, 2026
@catamorphism @Constellation
[Temporal] Fix order of operations in Temporal.PlainTime.from
66872df 
https://bugs.webkit.org/show_bug.cgi?id=305061

Reviewed by Yusuke Suzuki.

According to the spec, the overflow option should be read after the fields
when calling `from` on a property bag. This change improves the result for
the test test/built-ins/Temporal/PlainTime/from/order-of-operations.js (it
now fails because ZonedDateTime is not implemented).

* JSTests/test262/expectations.yaml:
* Source/JavaScriptCore/runtime/TemporalObject.cpp:
(JSC::doubleNumberOption):
* Source/JavaScriptCore/runtime/TemporalPlainDatePrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainDateTimePrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainTime.cpp:
(JSC::TemporalPlainTime::from):
* Source/JavaScriptCore/runtime/TemporalPlainTime.h:
* Source/JavaScriptCore/runtime/TemporalPlainTimeConstructor.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):
* Source/JavaScriptCore/runtime/TemporalPlainTimePrototype.cpp:
(JSC::JSC_DEFINE_HOST_FUNCTION):

Canonical link: https://commits.webkit.org/305405@main
@webkit-commit-queue webkit-commit-queue force-pushed the plainmonthday-plainyearmonth-split branch from 41c604b to 66872df Compare January 10, 2026 09:22
@webkit-commit-queue
Copy link
Collaborator

webkit-commit-queue commented Jan 10, 2026

Committed 305405@main (66872df): https://commits.webkit.org/305405@main

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

@webkit-commit-queue webkit-commit-queue merged commit 66872df into WebKit:main Jan 10, 2026
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jan 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Constellation Constellation Constellation approved these changes

Assignees

@catamorphism catamorphism

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@catamorphism @webkit-early-warning-system @webkit-commit-queue @Constellation @ptomato @webkit-ews-buildbot