-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[Temporal] Implement with method for PlainMonthDay
#52251
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
[Temporal] Implement with method for PlainMonthDay
#52251
Conversation
|
EWS run on previous version of this PR (hash 3c6140e) Details |
|
EWS run on previous version of this PR (hash 724fd55) Details |
Constellation
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I'm very concerning about these double year / month / day.
It is very inefficient.
Can you provide what part of the spec makes it double instead of using integers?
@Constellation In the spec, Calendar Fields Records ( https://tc39.es/proposal-temporal/#sec-temporal-calendar-fields-records ) have integer fields; these are mathematical integers and not 32-bit integers. In the code, I can move the validation earlier if absolutely necessary, so that the doubles returned by |
|
EWS run on previous version of this PR (hash 68c64c7) Details |
|
@Constellation In 68c64c7 I changed the functions in questions to take |
68c64c7 to
89e2078
Compare
|
EWS run on previous version of this PR (hash 89e2078) Details |
Constellation
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented. Please check all error cases are annotated as [[unlikely]] and checking whether there is missing error cases.
Source/JavaScriptCore/runtime/TemporalPlainMonthDayPrototype.cpp
Outdated
Show resolved
Hide resolved
Source/JavaScriptCore/runtime/TemporalPlainMonthDayPrototype.cpp
Outdated
Show resolved
Hide resolved
|
EWS run on previous version of this PR (hash 348c785) Details |
|
EWS run on previous version of this PR (hash 3c4d9d5) Details |
3c4d9d5 to
88d508b
Compare
|
EWS run on previous version of this PR (hash 88d508b) Details |
88d508b to
dce8c5b
Compare
|
EWS run on current version of this PR (hash dce8c5b) Details |
|
Squashed and rebased. |
https://bugs.webkit.org/show_bug.cgi?id=300640 Reviewed by Yusuke Suzuki. with() calls TemporalCalendar::monthDayFromFields(), which calls TemporalCalendar::isoDateFromFields() -- hence the changes to the type signature of isoDateFromFields(), which also change the type signatures of various from() methods. * JSTests/stress/temporal-plainmonthday.js: (shouldBe): * JSTests/test262/config.yaml: * JSTests/test262/expectations.yaml: * Source/JavaScriptCore/runtime/ISO8601.cpp: (JSC::ISO8601::parseMonthCode): (JSC::ISO8601::validMonthCode): (JSC::ISO8601::monthFromCode): Deleted. * Source/JavaScriptCore/runtime/ISO8601.h: * Source/JavaScriptCore/runtime/TemporalCalendar.cpp: (JSC::calendarResolveFields): (JSC::TemporalCalendar::isoDateFromFields): (JSC::TemporalCalendar::monthDayFromFields): * Source/JavaScriptCore/runtime/TemporalCalendar.h: * Source/JavaScriptCore/runtime/TemporalCalendarPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/TemporalDuration.cpp: (JSC::getEpochNanosecondsFor): * Source/JavaScriptCore/runtime/TemporalDuration.h: * Source/JavaScriptCore/runtime/TemporalObject.cpp: (JSC::getMonthCode): * Source/JavaScriptCore/runtime/TemporalObject.h: * Source/JavaScriptCore/runtime/TemporalPlainDate.cpp: (JSC::TemporalPlainDate::from): (JSC::TemporalPlainDate::toPartialDate): (JSC::TemporalPlainDate::with): * Source/JavaScriptCore/runtime/TemporalPlainDate.h: * Source/JavaScriptCore/runtime/TemporalPlainDateConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/TemporalPlainDatePrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/TemporalPlainDateTime.cpp: (JSC::TemporalPlainDateTime::from): (JSC::TemporalPlainDateTime::with): * Source/JavaScriptCore/runtime/TemporalPlainDateTime.h: * Source/JavaScriptCore/runtime/TemporalPlainDateTimeConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/TemporalPlainMonthDay.cpp: (JSC::TemporalPlainMonthDay::with): * Source/JavaScriptCore/runtime/TemporalPlainMonthDay.h: * Source/JavaScriptCore/runtime/TemporalPlainMonthDayConstructor.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): * Source/JavaScriptCore/runtime/TemporalPlainMonthDayPrototype.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): (JSC::JSC_DEFINE_CUSTOM_GETTER): Canonical link: https://commits.webkit.org/303360@main
dce8c5b to
f37c4da
Compare
|
Committed 303360@main (f37c4da): https://commits.webkit.org/303360@main Reviewed commits have been landed. Closing PR #52251 and removing active labels. |
f37c4da
dce8c5b
🧪 win-tests🧪 ios-wk2🧪 ios-wk2-wpt🧪 api-mac-debug🧪 mac-AS-debug-wk2🛠 playstation