Skip to content

Commit 66872df

Browse files
catamorphismConstellation
catamorphism
authored and
Constellation
committed
[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
1 parent d49e3f1 commit 66872df

File tree

7 files changed

+39
-20
lines changed

7 files changed

+39
-20
lines changed

‎JSTests/test262/expectations.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,8 +356,8 @@ test/built-ins/Temporal/PlainTime/from/options-wrong-type.js:
356356
default: 'Test262Error: Invalid string processed before throwing TypeError Expected a RangeError but got a TypeError'
357357
strict mode: 'Test262Error: Invalid string processed before throwing TypeError Expected a RangeError but got a TypeError'
358358
test/built-ins/Temporal/PlainTime/from/order-of-operations.js:
359-
default: 'Test262Error: Actual [get options.overflow, get options.overflow.toString, call options.overflow.toString, get fields.hour, get fields.hour.valueOf, call fields.hour.valueOf, get fields.microsecond, get fields.microsecond.valueOf, call fields.microsecond.valueOf, get fields.millisecond, get fields.millisecond.valueOf, call fields.millisecond.valueOf, get fields.minute, get fields.minute.valueOf, call fields.minute.valueOf, get fields.nanosecond, get fields.nanosecond.valueOf, call fields.nanosecond.valueOf, get fields.second, get fields.second.valueOf, call fields.second.valueOf] and expected [get fields.hour, get fields.hour.valueOf, call fields.hour.valueOf, get fields.microsecond, get fields.microsecond.valueOf, call fields.microsecond.valueOf, get fields.millisecond, get fields.millisecond.valueOf, call fields.millisecond.valueOf, get fields.minute, get fields.minute.valueOf, call fields.minute.valueOf, get fields.nanosecond, get fields.nanosecond.valueOf, call fields.nanosecond.valueOf, get fields.second, get fields.second.valueOf, call fields.second.valueOf, get options.overflow, get options.overflow.toString, call options.overflow.toString] should have the same contents. order of operations'
360-
strict mode: 'Test262Error: Actual [get options.overflow, get options.overflow.toString, call options.overflow.toString, get fields.hour, get fields.hour.valueOf, call fields.hour.valueOf, get fields.microsecond, get fields.microsecond.valueOf, call fields.microsecond.valueOf, get fields.millisecond, get fields.millisecond.valueOf, call fields.millisecond.valueOf, get fields.minute, get fields.minute.valueOf, call fields.minute.valueOf, get fields.nanosecond, get fields.nanosecond.valueOf, call fields.nanosecond.valueOf, get fields.second, get fields.second.valueOf, call fields.second.valueOf] and expected [get fields.hour, get fields.hour.valueOf, call fields.hour.valueOf, get fields.microsecond, get fields.microsecond.valueOf, call fields.microsecond.valueOf, get fields.millisecond, get fields.millisecond.valueOf, call fields.millisecond.valueOf, get fields.minute, get fields.minute.valueOf, call fields.minute.valueOf, get fields.nanosecond, get fields.nanosecond.valueOf, call fields.nanosecond.valueOf, get fields.second, get fields.second.valueOf, call fields.second.valueOf, get options.overflow, get options.overflow.toString, call options.overflow.toString] should have the same contents. order of operations'
359+
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(0n, \"UTC\")')"
360+
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(0n, \"UTC\")')"
361361
test/built-ins/Temporal/PlainTime/prototype/toString/options-read-before-algorithmic-validation.js:
362362
default: 'Test262Error: Actual [get options.smallestUnit, get options.smallestUnit.toString, call options.smallestUnit.toString] and expected [get options.fractionalSecondDigits, get options.fractionalSecondDigits.toString, call options.fractionalSecondDigits.toString, get options.roundingMode, get options.roundingMode.toString, call options.roundingMode.toString, get options.smallestUnit, get options.smallestUnit.toString, call options.smallestUnit.toString] should have the same contents. all options should be read first'
363363
strict mode: 'Test262Error: Actual [get options.smallestUnit, get options.smallestUnit.toString, call options.smallestUnit.toString] and expected [get options.fractionalSecondDigits, get options.fractionalSecondDigits.toString, call options.fractionalSecondDigits.toString, get options.roundingMode, get options.roundingMode.toString, call options.roundingMode.toString, get options.smallestUnit, get options.smallestUnit.toString, call options.smallestUnit.toString] should have the same contents. all options should be read first'

‎Source/JavaScriptCore/runtime/TemporalPlainDatePrototype.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainDatePrototypeFuncToPlainDateTime, (JSGloba
320320
if (itemValue.isUndefined())
321321
RELEASE_AND_RETURN(scope, JSValue::encode(TemporalPlainDateTime::tryCreateIfValid(globalObject, globalObject->plainDateTimeStructure(), plainDate->plainDate(), { })));
322322

323-
auto* plainTime = TemporalPlainTime::from(globalObject, itemValue, std::nullopt);
323+
auto* plainTime = TemporalPlainTime::from(globalObject, itemValue, nullptr);
324324
RETURN_IF_EXCEPTION(scope, { });
325325

326326
RELEASE_AND_RETURN(scope, JSValue::encode(TemporalPlainDateTime::tryCreateIfValid(globalObject, globalObject->plainDateTimeStructure(), plainDate->plainDate(), plainTime->plainTime())));

‎Source/JavaScriptCore/runtime/TemporalPlainDateTimePrototype.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,7 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainDateTimePrototypeFuncWithPlainTime, (JSGlo
256256
TemporalPlainTime* plainTime = nullptr;
257257
JSValue plainTimeLike = callFrame->argument(0);
258258
if (!plainTimeLike.isUndefined()) {
259-
plainTime = TemporalPlainTime::from(globalObject, plainTimeLike, std::nullopt);
259+
plainTime = TemporalPlainTime::from(globalObject, plainTimeLike, nullptr);
260260
RETURN_IF_EXCEPTION(scope, { });
261261
}
262262

‎Source/JavaScriptCore/runtime/TemporalPlainTime.cpp‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -427,21 +427,32 @@ ISO8601::PlainTime TemporalPlainTime::regulateTime(JSGlobalObject* globalObject,
427427
}
428428

429429
// https://tc39.es/proposal-temporal/#sec-temporal-totemporaltime
430-
TemporalPlainTime* TemporalPlainTime::from(JSGlobalObject* globalObject, JSValue itemValue, std::optional<TemporalOverflow> overflowValue)
430+
TemporalPlainTime* TemporalPlainTime::from(JSGlobalObject* globalObject, JSValue itemValue, JSObject* options)
431431
{
432432
VM& vm = globalObject->vm();
433433
auto scope = DECLARE_THROW_SCOPE(vm);
434434

435-
auto overflow = overflowValue.value_or(TemporalOverflow::Constrain);
436-
437435
if (itemValue.isObject()) {
438436
if (itemValue.inherits<TemporalPlainTime>())
439437
return jsCast<TemporalPlainTime*>(itemValue);
440438

441-
if (itemValue.inherits<TemporalPlainDateTime>())
439+
if (itemValue.inherits<TemporalPlainDateTime>()) {
440+
// Validate overflow -- see step 2(a)(ii) of ToTemporalTime
441+
if (options) {
442+
toTemporalOverflow(globalObject, options);
443+
RETURN_IF_EXCEPTION(scope, { });
444+
}
442445
return TemporalPlainTime::create(vm, globalObject->plainTimeStructure(), jsCast<TemporalPlainDateTime*>(itemValue)->plainTime());
446+
}
443447
auto duration = toTemporalTimeRecord(globalObject, jsCast<JSObject*>(itemValue));
444448
RETURN_IF_EXCEPTION(scope, { });
449+
450+
TemporalOverflow overflow = TemporalOverflow::Constrain;
451+
if (options) {
452+
overflow = toTemporalOverflow(globalObject, options);
453+
RETURN_IF_EXCEPTION(scope, { });
454+
}
455+
445456
auto plainTime = regulateTime(globalObject, WTF::move(duration), overflow);
446457
RETURN_IF_EXCEPTION(scope, { });
447458
return TemporalPlainTime::create(vm, globalObject->plainTimeStructure(), WTF::move(plainTime));
@@ -459,6 +470,11 @@ TemporalPlainTime* TemporalPlainTime::from(JSGlobalObject* globalObject, JSValue
459470

460471
auto string = itemValue.toWTFString(globalObject);
461472
RETURN_IF_EXCEPTION(scope, { });
473+
// Validate overflow -- see step 3(g) of ToTemporalTime
474+
if (options) {
475+
toTemporalOverflow(globalObject, options);
476+
RETURN_IF_EXCEPTION(scope, { });
477+
}
462478

463479
auto time = ISO8601::parseCalendarTime(string);
464480
if (time) {

‎Source/JavaScriptCore/runtime/TemporalPlainTime.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ class TemporalPlainTime final : public JSNonFinalObject {
5454
static ISO8601::PlainTime regulateTime(JSGlobalObject*, ISO8601::Duration&&, TemporalOverflow);
5555
static ISO8601::Duration addTime(const ISO8601::PlainTime&, const ISO8601::Duration&);
5656

57-
static TemporalPlainTime* from(JSGlobalObject*, JSValue, std::optional<TemporalOverflow>);
57+
static TemporalPlainTime* from(JSGlobalObject*, JSValue, JSObject*);
5858
static int32_t compare(const ISO8601::PlainTime&, const ISO8601::PlainTime&);
5959

6060
TemporalCalendar* calendar() { return m_calendar.get(this); }

‎Source/JavaScriptCore/runtime/TemporalPlainTimeConstructor.cpp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,15 +118,18 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainTimeConstructorFuncFrom, (JSGlobalObject*
118118
JSObject* options = intlGetOptionsObject(globalObject, callFrame->argument(1));
119119
RETURN_IF_EXCEPTION(scope, { });
120120

121-
TemporalOverflow overflow = toTemporalOverflow(globalObject, options);
122-
RETURN_IF_EXCEPTION(scope, { });
123-
124121
JSValue itemValue = callFrame->argument(0);
125122

126-
if (itemValue.inherits<TemporalPlainTime>())
127-
RELEASE_AND_RETURN(scope, JSValue::encode(TemporalPlainTime::create(vm, globalObject->plainTimeStructure(), jsCast<TemporalPlainTime*>(itemValue)->plainTime())));
123+
if (itemValue.inherits<TemporalPlainTime>()) {
124+
// Validate overflow -- see step 2(a)(ii) of ToTemporalTime
125+
if (options)
126+
toTemporalOverflow(globalObject, options);
127+
RETURN_IF_EXCEPTION(scope, { });
128+
return JSValue::encode(TemporalPlainTime::create(vm, globalObject->plainTimeStructure(),
129+
jsCast<TemporalPlainTime*>(itemValue)->plainTime()));
130+
}
128131

129-
RELEASE_AND_RETURN(scope, JSValue::encode(TemporalPlainTime::from(globalObject, itemValue, overflow)));
132+
RELEASE_AND_RETURN(scope, JSValue::encode(TemporalPlainTime::from(globalObject, itemValue, options)));
130133
}
131134

132135
// https://tc39.es/proposal-temporal/#sec-temporal.plaintime.compare
@@ -135,10 +138,10 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainTimeConstructorFuncCompare, (JSGlobalObjec
135138
VM& vm = globalObject->vm();
136139
auto scope = DECLARE_THROW_SCOPE(vm);
137140

138-
auto* one = TemporalPlainTime::from(globalObject, callFrame->argument(0), std::nullopt);
141+
auto* one = TemporalPlainTime::from(globalObject, callFrame->argument(0), nullptr);
139142
RETURN_IF_EXCEPTION(scope, { });
140143

141-
auto* two = TemporalPlainTime::from(globalObject, callFrame->argument(1), std::nullopt);
144+
auto* two = TemporalPlainTime::from(globalObject, callFrame->argument(1), nullptr);
142145
RETURN_IF_EXCEPTION(scope, { });
143146

144147
return JSValue::encode(jsNumber(TemporalPlainTime::compare(one->plainTime(), two->plainTime())));

‎Source/JavaScriptCore/runtime/TemporalPlainTimePrototype.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainTimePrototypeFuncUntil, (JSGlobalObject* g
179179
if (!plainTime)
180180
return throwVMTypeError(globalObject, scope, "Temporal.PlainTime.prototype.until called on value that's not a PlainTime"_s);
181181

182-
auto* other = TemporalPlainTime::from(globalObject, callFrame->argument(0), std::nullopt);
182+
auto* other = TemporalPlainTime::from(globalObject, callFrame->argument(0), nullptr);
183183
RETURN_IF_EXCEPTION(scope, { });
184184

185185
auto result = plainTime->until(globalObject, other, callFrame->argument(1));
@@ -198,7 +198,7 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainTimePrototypeFuncSince, (JSGlobalObject* g
198198
if (!plainTime)
199199
return throwVMTypeError(globalObject, scope, "Temporal.PlainTime.prototype.since called on value that's not a PlainTime"_s);
200200

201-
auto* other = TemporalPlainTime::from(globalObject, callFrame->argument(0), std::nullopt);
201+
auto* other = TemporalPlainTime::from(globalObject, callFrame->argument(0), nullptr);
202202
RETURN_IF_EXCEPTION(scope, { });
203203

204204
auto result = plainTime->since(globalObject, other, callFrame->argument(1));
@@ -237,7 +237,7 @@ JSC_DEFINE_HOST_FUNCTION(temporalPlainTimePrototypeFuncEquals, (JSGlobalObject*
237237
if (!plainTime)
238238
return throwVMTypeError(globalObject, scope, "Temporal.PlainTime.prototype.equals called on value that's not a PlainTime"_s);
239239

240-
auto* other = TemporalPlainTime::from(globalObject, callFrame->argument(0), std::nullopt);
240+
auto* other = TemporalPlainTime::from(globalObject, callFrame->argument(0), nullptr);
241241
RETURN_IF_EXCEPTION(scope, { });
242242

243243
return JSValue::encode(jsBoolean(plainTime->plainTime() == other->plainTime()));

0 commit comments

Comments
 (0)