Skip to content
Permalink
Browse files
TemporalPlainTime::toTemporalTimeRecord shouldn't require all propert…
…ies to be provided

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

Reviewed by Yusuke Suzuki and Darin Adler.

Following the spec correction of tc39/proposal-temporal#1862, this patch
fixes our Temporal.PlainTime implementation to require that *one* property be provided, not *all* of them.

* stress/temporal-plaintime.js:
* test262/expectations.yaml: Mark 32 test cases as passing.

* runtime/TemporalDuration.cpp:
(JSC::TemporalDuration::fromDurationLike):
* runtime/TemporalPlainTime.cpp:
(JSC::toTemporalTimeRecord):

Canonical link: https://commits.webkit.org/250543@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294176 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
rkirsling committed May 13, 2022
1 parent a45efcc commit 498784314099791e9fb364c297f4972267ec5446
@@ -1,3 +1,13 @@
2022-05-13 Ross Kirsling <ross.kirsling@sony.com>

TemporalPlainTime::toTemporalTimeRecord shouldn't require all properties to be provided
https://bugs.webkit.org/show_bug.cgi?id=240394

Reviewed by Yusuke Suzuki and Darin Adler.

* stress/temporal-plaintime.js:
* test262/expectations.yaml: Mark 32 test cases as passing.

2022-05-12 Angelos Oikonomopoulos <angelos@igalia.com>

Unskip no longer failing test
@@ -156,23 +156,17 @@ shouldBe(String(Temporal.PlainTime.from('2007-01-09 03:24:30[u-ca=japanese]')),
nanosecond: 205
});
shouldBe(String(time), `19:39:09.068346205`);

// This is spec bug. Currently this throws an error. But possibly this should not throw an error.
// Tracked in https://github.com/tc39/proposal-temporal/issues/1803.
shouldThrow(() => {
Temporal.PlainTime.from({ hour: 19, minute: 39, second: 9 });
}, TypeError);
}
{
// Different overflow modes
shouldBe(String(Temporal.PlainTime.from({ hour: 15, minute: 60, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 }, { overflow: 'constrain' })), `15:59:00`);
shouldBe(String(Temporal.PlainTime.from({ hour: 15, minute: -1, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 }, { overflow: 'constrain' })), `15:00:00`);
shouldBe(String(Temporal.PlainTime.from({ hour: 15, minute: 60 }, { overflow: 'constrain' })), `15:59:00`);
shouldBe(String(Temporal.PlainTime.from({ hour: 15, minute: -1 }, { overflow: 'constrain' })), `15:00:00`);
}
shouldThrow(() => {
Temporal.PlainTime.from({ hour: 15, minute: 60, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 }, { overflow: 'reject' });
Temporal.PlainTime.from({ hour: 15, minute: 60 }, { overflow: 'reject' });
}, RangeError);
shouldThrow(() => {
Temporal.PlainTime.from({ hour: 15, minute: -1, second: 0, millisecond: 0, microsecond: 0, nanosecond: 0 }, { overflow: 'reject' });
Temporal.PlainTime.from({ hour: 15, minute: -1 }, { overflow: 'reject' });
}, RangeError);

shouldThrow(() => { new Temporal.PlainTime(-1); }, RangeError);
@@ -1065,27 +1065,12 @@ test/built-ins/Temporal/Instant/prototype/toString/timezone-string-leap-second.j
test/built-ins/Temporal/Instant/prototype/toString/timezone-string-multiple-offsets.js:
default: 'RangeError: argument needs to be UTC offset string, TimeZone identifier, or temporal Instant string'
strict mode: 'RangeError: argument needs to be UTC offset string, TimeZone identifier, or temporal Instant string'
test/built-ins/Temporal/PlainTime/compare/argument-cast.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/compare/argument-string-time-designator-required-for-disambiguation.js:
default: 'Test262Error: 2021-12 is ambiguous and requires T prefix (first argument) Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix (first argument) Expected a RangeError to be thrown but no exception was thrown at all'
test/built-ins/Temporal/PlainTime/compare/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/compare/leap-second.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/compare/plaintime-propertybag-no-time-units.js:
default: 'TypeError: "hour" field is missing'
strict mode: 'TypeError: "hour" field is missing'
test/built-ins/Temporal/PlainTime/from/argument-object-leap-second.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/argument-object.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/argument-plaindatetime.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.PlainDateTime(2000, 5, 2, 12, 34, 56, 987, 654, 321, calendar)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.PlainDateTime(2000, 5, 2, 12, 34, 56, 987, 654, 321, calendar)')"
@@ -1098,78 +1083,30 @@ test/built-ins/Temporal/PlainTime/from/argument-string-with-calendar.js:
test/built-ins/Temporal/PlainTime/from/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/from/leap-second.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/options-undefined.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/overflow-constrain.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/overflow-reject.js:
default: 'Test262Error: Expected a RangeError but got a TypeError'
strict mode: 'Test262Error: Expected a RangeError but got a TypeError'
test/built-ins/Temporal/PlainTime/from/overflow-undefined.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/overflow-wrong-type.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/from/plaintime-propertybag-no-time-units.js:
default: 'TypeError: "hour" field is missing'
strict mode: 'TypeError: "hour" field is missing'
test/built-ins/Temporal/PlainTime/prototype/equals/argument-cast.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/prototype/equals/argument-string-time-designator-required-for-disambiguation.js:
default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
test/built-ins/Temporal/PlainTime/prototype/equals/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/prototype/equals/leap-second.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/prototype/equals/plaintime-propertybag-no-time-units.js:
default: 'TypeError: "hour" field is missing'
strict mode: 'TypeError: "hour" field is missing'
test/built-ins/Temporal/PlainTime/prototype/since/argument-cast.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/prototype/since/argument-string-time-designator-required-for-disambiguation.js:
default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
test/built-ins/Temporal/PlainTime/prototype/since/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/prototype/since/leap-second.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/prototype/since/plaintime-propertybag-no-time-units.js:
default: 'TypeError: "hour" field is missing'
strict mode: 'TypeError: "hour" field is missing'
test/built-ins/Temporal/PlainTime/prototype/since/roundingmode-ceil.js:
default: 'Test262Error: hours hours result Expected SameValue(«4», «5») to be true'
strict mode: 'Test262Error: hours hours result Expected SameValue(«4», «5») to be true'
test/built-ins/Temporal/PlainTime/prototype/since/roundingmode-floor.js:
default: 'Test262Error: hours hours result Expected SameValue(«5», «4») to be true'
strict mode: 'Test262Error: hours hours result Expected SameValue(«5», «4») to be true'
test/built-ins/Temporal/PlainTime/prototype/until/argument-cast.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/prototype/until/argument-string-time-designator-required-for-disambiguation.js:
default: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
strict mode: 'Test262Error: 2021-12 is ambiguous and requires T prefix Expected a RangeError to be thrown but no exception was thrown at all'
test/built-ins/Temporal/PlainTime/prototype/until/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-not-callable.js:
default: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
strict mode: "TypeError: undefined is not a constructor (evaluating 'new Temporal.ZonedDateTime(1_000_000_000_987_654_321n, timeZone)')"
test/built-ins/Temporal/PlainTime/prototype/until/leap-second.js:
default: 'TypeError: "microsecond" field is missing'
strict mode: 'TypeError: "microsecond" field is missing'
test/built-ins/Temporal/PlainTime/prototype/until/plaintime-propertybag-no-time-units.js:
default: 'TypeError: "hour" field is missing'
strict mode: 'TypeError: "hour" field is missing'
test/built-ins/Temporal/getOwnPropertyNames.js:
default: 'Test262Error: PlainDateTime'
strict mode: 'Test262Error: PlainDateTime'
@@ -1,3 +1,18 @@
2022-05-13 Ross Kirsling <ross.kirsling@sony.com>

TemporalPlainTime::toTemporalTimeRecord shouldn't require all properties to be provided
https://bugs.webkit.org/show_bug.cgi?id=240394

Reviewed by Yusuke Suzuki and Darin Adler.

Following the spec correction of https://github.com/tc39/proposal-temporal/pull/1862, this patch
fixes our Temporal.PlainTime implementation to require that *one* property be provided, not *all* of them.

* runtime/TemporalDuration.cpp:
(JSC::TemporalDuration::fromDurationLike):
* runtime/TemporalPlainTime.cpp:
(JSC::toTemporalTimeRecord):

2022-05-13 Anjali Kumar <anjalik_22@apple.com>

Web Inspector: [Meta] Implement Timelines Film Strip
@@ -94,10 +94,8 @@ ISO8601::Duration TemporalDuration::fromDurationLike(JSGlobalObject* globalObjec
JSValue value = durationLike->get(globalObject, temporalUnitPluralPropertyName(vm, unit));
RETURN_IF_EXCEPTION(scope, { });

if (value.isUndefined()) {
result[unit] = 0;
if (value.isUndefined())
continue;
}

hasRelevantProperty = true;
result[unit] = value.toIntegerWithoutRounding(globalObject);
@@ -301,20 +301,18 @@ static ISO8601::Duration toTemporalTimeRecord(JSGlobalObject* globalObject, JSOb
auto scope = DECLARE_THROW_SCOPE(vm);

ISO8601::Duration duration { };
auto hasRelevantProperty = false;
for (TemporalUnit unit : temporalUnitsInTableOrder) {
if (unit < TemporalUnit::Hour)
continue;
auto name = temporalUnitSingularPropertyName(vm, unit);
JSValue value = temporalTimeLike->get(globalObject, name);
RETURN_IF_EXCEPTION(scope, { });

// We are throwing an error here, but probably this is a spec bug.
// Tracked in https://github.com/tc39/proposal-temporal/issues/1803.
if (value.isUndefined()) {
throwTypeError(globalObject, scope, makeString('"', StringView(name.uid()), "\" field is missing"_s));
return { };
}
if (value.isUndefined())
continue;

hasRelevantProperty = true;
double integer = value.toIntegerOrInfinity(globalObject);
RETURN_IF_EXCEPTION(scope, { });
if (!std::isfinite(integer)) {
@@ -323,6 +321,12 @@ static ISO8601::Duration toTemporalTimeRecord(JSGlobalObject* globalObject, JSOb
}
duration[unit] = integer;
}

if (!hasRelevantProperty) {
throwTypeError(globalObject, scope, "Object must contain at least one Temporal time unit property"_s);
return { };
}

return duration;
}

0 comments on commit 4987843

Please sign in to comment.