Skip to content

Commit

Permalink
Temporal: Return early from Duration.compare if internal slots equal
Browse files Browse the repository at this point in the history
  • Loading branch information
ptomato committed Aug 18, 2023
1 parent e83f7a1 commit 8e27d12
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const timeZone = TemporalHelpers.oneShiftTimeZone(new Temporal.Instant(0n), 3600
const relativeTo = new Temporal.ZonedDateTime(0n, timeZone, calendar);

const duration1 = new Temporal.Duration(0, 0, 1);
const duration2 = new Temporal.Duration(0, 0, 1);
const duration2 = new Temporal.Duration(0, 0, 1, 1);
Temporal.Duration.compare(duration1, duration2, { relativeTo });
assert.sameValue(calendar.dateAddCallCount, 4);
// one call in CalculateOffsetShift for each duration argument, plus one in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,43 +6,47 @@ esid: sec-temporal.duration.compare
description: relativeTo argument needed if days = 0 but years/months/weeks non-zero
features: [Temporal]
---*/
const duration1 = new Temporal.Duration(1);
const duration2 = new Temporal.Duration(0, 12);
const duration3 = new Temporal.Duration(0, 0, 5);
const duration4 = new Temporal.Duration(0, 0, 0, 42);
const duration1a = new Temporal.Duration(1);
const duration1b = new Temporal.Duration(1, 0, 0, 0, 0, 0, 0, 0, 0, 1);
const duration2a = new Temporal.Duration(0, 12);
const duration2b = new Temporal.Duration(0, 12, 0, 0, 0, 0, 0, 0, 0, 1);
const duration3a = new Temporal.Duration(0, 0, 5);
const duration3b = new Temporal.Duration(0, 0, 5, 0, 0, 0, 0, 0, 0, 1);
const duration4a = new Temporal.Duration(0, 0, 0, 42);
const duration4b = new Temporal.Duration(0, 0, 0, 42, 0, 0, 0, 0, 0, 1);
const relativeTo = new Temporal.PlainDate(2021, 12, 15);
assert.throws(
RangeError,
() => { Temporal.Duration.compare(duration1, duration1); },
() => { Temporal.Duration.compare(duration1a, duration1b); },
"cannot compare Duration values without relativeTo if year is non-zero"
);
assert.sameValue(0,
Temporal.Duration.compare(duration1, duration1, { relativeTo }),
assert.sameValue(-1,
Temporal.Duration.compare(duration1a, duration1b, { relativeTo }),
"compare succeeds for year-only Duration provided relativeTo is supplied");
assert.throws(
RangeError,
() => { Temporal.Duration.compare(duration2, duration2); },
() => { Temporal.Duration.compare(duration2a, duration2b); },
"cannot compare Duration values without relativeTo if month is non-zero"
);
assert.sameValue(0,
Temporal.Duration.compare(duration2, duration2, { relativeTo }),
assert.sameValue(-1,
Temporal.Duration.compare(duration2a, duration2b, { relativeTo }),
"compare succeeds for year-and-month Duration provided relativeTo is supplied");
assert.throws(
RangeError,
() => { Temporal.Duration.compare(duration3, duration3); },
() => { Temporal.Duration.compare(duration3a, duration3b); },
"cannot compare Duration values without relativeTo if week is non-zero"
);
assert.sameValue(0,
Temporal.Duration.compare(duration3, duration3, { relativeTo }),
assert.sameValue(-1,
Temporal.Duration.compare(duration3a, duration3b, { relativeTo }),
"compare succeeds for year-and-month-and-week Duration provided relativeTo is supplied"
);

assert.sameValue(0,
Temporal.Duration.compare(duration4, duration4),
assert.sameValue(-1,
Temporal.Duration.compare(duration4a, duration4b),
"compare succeeds for zero year-month-week non-zero day Duration even without relativeTo");

// Double-check that the comparison also works with a relative-to argument
assert.sameValue(0,
Temporal.Duration.compare(duration4, duration4, { relativeTo }),
assert.sameValue(-1,
Temporal.Duration.compare(duration4a, duration4b, { relativeTo }),
"compare succeeds for zero year-month-week non-zero day Duration with relativeTo"
);
44 changes: 44 additions & 0 deletions test/built-ins/Temporal/Duration/compare/instances-identical.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Copyright (C) 2023 Igalia, S.L. All rights reserved.
// This code is governed by the BSD license found in the LICENSE file.

/*---
esid: sec-temporal.duration.compare
description: >
Shortcut return with no observable user code calls when two Temporal.Duration
have identical internal slots
includes: [temporalHelpers.js]
features: [Temporal]
---*/

const duration1 = new Temporal.Duration(0, 0, 0, 5, 5, 5, 5, 5, 5, 5);
const duration2 = new Temporal.Duration(0, 0, 0, 5, 5, 5, 5, 5, 5, 5);
assert.sameValue(Temporal.Duration.compare(duration1, duration2), 0, "identical Duration instances should be equal");

const dateDuration1 = new Temporal.Duration(5, 5, 5, 5, 5, 5, 5, 5, 5, 5);
const dateDuration2 = new Temporal.Duration(5, 5, 5, 5, 5, 5, 5, 5, 5, 5);
assert.sameValue(
Temporal.Duration.compare(dateDuration1, dateDuration2),
0,
"relativeTo is not required if two distinct Duration instances are identical"
);

const calendar = TemporalHelpers.calendarThrowEverything();
const relativeTo = new Temporal.PlainDate(2000, 1, 1, calendar);
assert.sameValue(
Temporal.Duration.compare(dateDuration1, dateDuration2, { relativeTo }),
0,
"no calendar methods are called if two distinct Duration instances are identical"
);

const dateDuration3 = new Temporal.Duration(5, 5, 5, 5, 4, 65, 5, 5, 5, 5);
assert.throws(
RangeError,
() => Temporal.Duration.compare(dateDuration1, dateDuration3),
"relativeTo is required if two Duration instances are the same length but not identical"
);

assert.throws(
Test262Error,
() => Temporal.Duration.compare(dateDuration1, dateDuration3, { relativeTo }),
"calendar methods are called if two Duration instances are the same length but not identical"
);
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ features: [Temporal]
---*/

let timeZone = "2021-08-19T17:30";
assert.throws(RangeError, () => Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }), "bare date-time string is not a time zone");
assert.throws(RangeError, () => Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }), "bare date-time string is not a time zone");

[
"2021-08-19T17:30-07:00:01",
Expand All @@ -34,7 +34,7 @@ assert.throws(RangeError, () => Temporal.Duration.compare(new Temporal.Duration(
].forEach((timeZone) => {
assert.throws(
RangeError,
() => Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }),
() => Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }),
`ISO string ${timeZone} with a sub-minute offset is not a valid time zone`
);
});
Expand All @@ -57,5 +57,5 @@ assert.throws(RangeError, () => Temporal.Duration.compare(new Temporal.Duration(
"2021-08-19T17:30-0700[UTC]",
"2021-08-19T1730-0700[UTC]",
].forEach((timeZone) => {
Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } });
Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } });
});
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ let timeZone = "2016-12-31T23:59:60+00:00[UTC]";
// A string with a leap second is a valid ISO string, so the following
// operation should not throw

Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } });
Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } });

timeZone = "2021-08-19T17:30:45.123456789+23:59[+23:59:60]";
assert.throws(RangeError, () => Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }), "leap second in time zone name not valid");
assert.throws(RangeError, () => Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }), "leap second in time zone name not valid");
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const invalidStrings = [
invalidStrings.forEach((timeZone) => {
assert.throws(
RangeError,
() => Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }),
() => Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }),
"reject minus zero as extended year"
);
});
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Object.defineProperty(Temporal.TimeZone.prototype, "getOffsetNanosecondsFor", {
// The following are all valid strings so should not throw:

["UTC", "+01:00"].forEach((timeZone) => {
Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } });
Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } });
});

Object.defineProperty(Temporal.TimeZone.prototype, "getPossibleInstantsFor", getPossibleInstantsForOriginal);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const primitiveTests = [
for (const [timeZone, description] of primitiveTests) {
assert.throws(
typeof timeZone === 'string' ? RangeError : TypeError,
() => Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }),
() => Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }),
`${description} does not convert to a valid ISO string`
);
}
Expand All @@ -33,5 +33,5 @@ const typeErrorTests = [
];

for (const [timeZone, description] of typeErrorTests) {
assert.throws(TypeError, () => Temporal.Duration.compare(new Temporal.Duration(), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }), `${description} is not a valid object and does not convert to a string`);
assert.throws(TypeError, () => Temporal.Duration.compare(new Temporal.Duration(1), new Temporal.Duration(), { relativeTo: { year: 2000, month: 5, day: 2, timeZone } }), `${description} is not a valid object and does not convert to a string`);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ const oneYear = new Temporal.Duration(1);
const oneMonth = new Temporal.Duration(0, 1);
const oneWeek = new Temporal.Duration(0, 0, 1);
const oneDay = new Temporal.Duration(0, 0, 0, 1);
const twoDays = new Temporal.Duration(0, 0, 0, 2);

assert.sameValue(Temporal.Duration.compare(oneDay, oneDay), 0, "days do not require relativeTo");
assert.sameValue(Temporal.Duration.compare(oneDay, twoDays), -1, "days do not require relativeTo");

assert.throws(RangeError, () => Temporal.Duration.compare(oneWeek, oneDay), "weeks in left operand require relativeTo");
assert.throws(RangeError, () => Temporal.Duration.compare(oneDay, oneWeek), "weeks in right operand require relativeTo");
Expand Down

0 comments on commit 8e27d12

Please sign in to comment.