Temporal: Avoid calling user code in no-op round operations

This shortcut path now exists in all round(), since(), and until()
operations.
In Instant, PlainDate, PlainDateTime, and PlainTime, the change isn't
observable, so no tests could be added. This adds test coverage for

- Duration.p.round()
- PlainYearMonth.p.since()
- PlainYearMonth.p.until()
- ZonedDateTime.p.round()
- ZonedDateTime.p.since()
- ZonedDateTime.p.until()

As well as a few cases where we are testing that certain calendar methods
get called during a round operation, but previously were doing so with
options that now become a no-op and no longer call those calendar methods.
In those cases, round to 2 ns, rather than 1 ns.
This commit is contained in:
Philip Chimento 2023-03-02 18:13:06 -08:00 committed by Ms2ger
parent 1db9a49eb9
commit f44bbe4035
9 changed files with 163 additions and 9 deletions

View File

@ -1947,6 +1947,30 @@ var TemporalHelpers = {
});
},
/*
* A custom time zone that does not allow any of its methods to be called, for
* the purpose of asserting that a particular operation does not call into
* user code.
*/
timeZoneThrowEverything() {
class TimeZoneThrowEverything extends Temporal.TimeZone {
constructor() {
super("UTC");
}
getOffsetNanosecondsFor() {
TemporalHelpers.assertUnreachable("getOffsetNanosecondsFor should not be called");
}
getPossibleInstantsFor() {
TemporalHelpers.assertUnreachable("getPossibleInstantsFor should not be called");
}
toString() {
TemporalHelpers.assertUnreachable("toString should not be called");
}
}
return new TimeZoneThrowEverything();
},
/*
* Returns an object that will append logs of any Gets or Calls of its valueOf
* or toString properties to the array calls. Both valueOf and toString will

View File

@ -0,0 +1,69 @@
// 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.prototype.round
description: >
No calendar or time zone methods are called under circumstances where rounding
is a no-op
includes: [temporalHelpers.js]
features: [Temporal]
---*/
const calendar = TemporalHelpers.calendarThrowEverything();
const timeZone = TemporalHelpers.timeZoneThrowEverything();
const plainRelativeTo = new Temporal.PlainDate(2000, 1, 1, calendar);
const zonedRelativeTo = new Temporal.ZonedDateTime(0n, timeZone, calendar);
const d = new Temporal.Duration(0, 0, 0, 0, 23, 59, 59, 999, 999, 997);
const noopRoundingOperations = [
[d, { smallestUnit: "nanoseconds" }, "smallestUnit ns"],
[d, { smallestUnit: "nanoseconds", relativeTo: plainRelativeTo }, "smallestUnit ns and plain relativeTo"],
[d, { smallestUnit: "nanoseconds", relativeTo: zonedRelativeTo }, "smallestUnit ns and zoned relativeTo"],
[d, { smallestUnit: "nanoseconds", roundingIncrement: 1 }, "round to 1 ns"],
// No balancing because largestUnit is already the largest unit and no time units overflow:
[d, { largestUnit: "hours" }, "largestUnit hours"],
// Unless relativeTo is ZonedDateTime, no-op is still possible with days>0:
[new Temporal.Duration(0, 0, 0, 1), { smallestUnit: "nanoseconds" }, "days>0 and smallestUnit ns"],
[new Temporal.Duration(0, 0, 0, 1), { smallestUnit: "nanoseconds", relativeTo: plainRelativeTo }, "days>0, smallestUnit ns, and plain relativeTo"],
];
for (const [duration, options, descr] of noopRoundingOperations) {
const result = duration.round(options);
assert.notSameValue(result, duration, "rounding result should be a new object");
TemporalHelpers.assertDurationsEqual(result, duration, `rounding should be a no-op with ${descr}`);
}
// These operations are not no-op rounding operations, but still should not call
// any calendar methods:
const roundingOperationsNotCallingCalendarMethods = [
[d, { smallestUnit: "microseconds" }, "round to 1 µs"],
[d, { smallestUnit: "nanoseconds", roundingIncrement: 2 }, "round to 2 ns"],
[new Temporal.Duration(0, 0, 0, 0, 24), { largestUnit: "days" }, "upwards balancing requested"],
[d, { largestUnit: "minutes" }, "downwards balancing requested"],
[new Temporal.Duration(0, 0, 0, 0, 1, 120), { smallestUnit: "nanoseconds" }, "time units could overflow"],
[new Temporal.Duration(0, 0, 0, 1, 24), { smallestUnit: "nanoseconds" }, "hours-to-days conversion could occur"],
];
for (const [duration, options, descr] of roundingOperationsNotCallingCalendarMethods) {
const result = duration.round(options);
let equal = true;
for (const prop of ['years', 'months', 'weeks', 'days', 'hours', 'minutes', 'seconds', 'milliseconds', 'microseconds', 'nanoseconds']) {
if (result[prop] !== duration[prop]) {
equal = false;
break;
}
}
assert(!equal, `round result ${result} should be different from ${duration} with ${descr}`);
}
// These operations should not be short-circuited because they have to call
// calendar methods:
const roundingOperationsCallingCalendarMethods = [
[new Temporal.Duration(0, 0, 1), { smallestUnit: "nanoseconds", relativeTo: plainRelativeTo }, "calendar units present"],
[d, { largestUnit: "days", relativeTo: zonedRelativeTo }, "largestUnit days with zoned relativeTo"],
[new Temporal.Duration(0, 0, 0, 1), { smallestUnit: "nanoseconds", relativeTo: zonedRelativeTo }, "hours-to-days conversion could occur with zoned relativeTo"],
];
for (const [duration, options, descr] of roundingOperationsCallingCalendarMethods) {
assert.throws(Test262Error, () => duration.round(options), `rounding should not be a no-op with ${descr}`);
}

View File

@ -117,6 +117,11 @@ function createOptionsObserver({ smallestUnit = "months", largestUnit = "auto",
// clear any observable things that happened while constructing the objects
actual.splice(0);
// code path that skips RoundDuration:
instance.since(otherYearMonthPropertyBag, createOptionsObserver({ smallestUnit: "months", roundingIncrement: 1 }));
assert.compareArray(actual, expected, "order of operations with no rounding");
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest year:
const expectedOpsForYearRounding = expected.concat([
"get this.calendar.dateAdd", // 9.b

View File

@ -117,6 +117,11 @@ function createOptionsObserver({ smallestUnit = "months", largestUnit = "auto",
// clear any observable things that happened while constructing the objects
actual.splice(0);
// code path that skips RoundDuration:
instance.since(otherYearMonthPropertyBag, createOptionsObserver({ smallestUnit: "months", roundingIncrement: 1 }));
assert.compareArray(actual, expected, "order of operations with no rounding");
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest year:
const expectedOpsForYearRounding = expected.concat([
"get this.calendar.dateAdd", // 9.b

View File

@ -18,7 +18,7 @@ class Calendar extends Temporal.Calendar {
const zdt = new Temporal.ZonedDateTime(0n, "UTC", new Calendar());
const units = ["day", "hour", "minute", "second", "millisecond", "microsecond", "nanosecond"];
const units = ["hour", "minute", "second", "millisecond", "microsecond", "nanosecond"];
for (const smallestUnit of units) {
assert.throws(RangeError, () => zdt.round({ smallestUnit }));
assert.throws(RangeError, () => zdt.round({ smallestUnit, roundingIncrement: 2 }));
}

View File

@ -42,7 +42,7 @@ const actual = [];
const options = TemporalHelpers.propertyBagObserver(actual, {
smallestUnit: "nanoseconds",
roundingMode: "halfExpand",
roundingIncrement: 1,
roundingIncrement: 2,
}, "options");
const instance = new Temporal.ZonedDateTime(

View File

@ -0,0 +1,35 @@
// 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.zoneddatetime.prototype.round
description: >
No calendar or time zone methods are called under circumstances where rounding
is a no-op
includes: [temporalHelpers.js]
features: [Temporal]
---*/
const calendar = TemporalHelpers.calendarThrowEverything();
const timeZone = TemporalHelpers.timeZoneThrowEverything();
const instance = new Temporal.ZonedDateTime(0n, timeZone, calendar);
const noopRoundingOperations = [
[{ smallestUnit: "nanoseconds" }, "smallestUnit ns"],
[{ smallestUnit: "nanoseconds", roundingIncrement: 1 }, "round to 1 ns"],
];
for (const [options, descr] of noopRoundingOperations) {
const result = instance.round(options);
assert.notSameValue(result, instance, "rounding result should be a new object");
assert.sameValue(result.epochNanoseconds, instance.epochNanoseconds, "instant should be unchanged");
assert.sameValue(result.getCalendar(), instance.getCalendar(), "calendar should be preserved");
assert.sameValue(result.getTimeZone(), instance.getTimeZone(), "time zone should be preserved");
}
const notNoopRoundingOperations = [
[{ smallestUnit: "microseconds" }, "round to 1 µs"],
[{ smallestUnit: "nanoseconds", roundingIncrement: 2 }, "round to 2 ns"],
];
for (const [options, descr] of notNoopRoundingOperations) {
assert.throws(Test262Error, () => instance.round(options), `rounding should not be a no-op with ${descr}`);
}

View File

@ -186,6 +186,9 @@ const expectedOpsForCalendarDifference = [
"call this.calendar.dateAdd",
"get this.timeZone.getPossibleInstantsFor",
"call this.timeZone.getPossibleInstantsFor",
];
const expectedOpsForCalendarRounding = [
// RoundDuration → ToTemporalDate
"get this.timeZone.getOffsetNanosecondsFor",
"call this.timeZone.getOffsetNanosecondsFor",
@ -213,8 +216,13 @@ const expectedOpsForCalendarDifference = [
"call this.timeZone.getPossibleInstantsFor",
];
// code path that skips RoundDuration:
instance.since(otherDateTimePropertyBag, createOptionsObserver({ largestUnit: "years", smallestUnit: "nanoseconds", roundingIncrement: 1 }));
assert.compareArray(actual, expected.concat(expectedOpsForCalendarDifference), "order of operations with largestUnit years and no rounding");
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest year:
const expectedOpsForYearRounding = expected.concat(expectedOpsForCalendarDifference, [
const expectedOpsForYearRounding = expected.concat(expectedOpsForCalendarDifference, expectedOpsForCalendarRounding, [
"get this.calendar.dateAdd", // 9.b
"call this.calendar.dateAdd", // 9.c
"call this.calendar.dateAdd", // 9.e
@ -229,7 +237,7 @@ assert.compareArray(actual, expectedOpsForYearRounding, "order of operations wit
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest month:
const expectedOpsForMonthRounding = expected.concat(expectedOpsForCalendarDifference, [
const expectedOpsForMonthRounding = expected.concat(expectedOpsForCalendarDifference, expectedOpsForCalendarRounding, [
"get this.calendar.dateAdd", // 10.b
"call this.calendar.dateAdd", // 10.c
"call this.calendar.dateAdd", // 10.e
@ -240,7 +248,7 @@ assert.compareArray(actual, expectedOpsForMonthRounding, "order of operations wi
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest week:
const expectedOpsForWeekRounding = expected.concat(expectedOpsForCalendarDifference, [
const expectedOpsForWeekRounding = expected.concat(expectedOpsForCalendarDifference, expectedOpsForCalendarRounding, [
"get this.calendar.dateAdd", // 11.c
"call this.calendar.dateAdd", // 11.d MoveRelativeDate
]); // (11.g.iii MoveRelativeDate not called because days already balanced)

View File

@ -186,6 +186,9 @@ const expectedOpsForCalendarDifference = [
"call this.calendar.dateAdd",
"get this.timeZone.getPossibleInstantsFor",
"call this.timeZone.getPossibleInstantsFor",
];
const expectedOpsForCalendarRounding = [
// RoundDuration → ToTemporalDate
"get this.timeZone.getOffsetNanosecondsFor",
"call this.timeZone.getOffsetNanosecondsFor",
@ -213,8 +216,13 @@ const expectedOpsForCalendarDifference = [
"call this.timeZone.getPossibleInstantsFor",
];
// code path that skips RoundDuration:
instance.until(otherDateTimePropertyBag, createOptionsObserver({ largestUnit: "years", smallestUnit: "nanoseconds", roundingIncrement: 1 }));
assert.compareArray(actual, expected.concat(expectedOpsForCalendarDifference), "order of operations with largestUnit years and no rounding");
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest year:
const expectedOpsForYearRounding = expected.concat(expectedOpsForCalendarDifference, [
const expectedOpsForYearRounding = expected.concat(expectedOpsForCalendarDifference, expectedOpsForCalendarRounding, [
"get this.calendar.dateAdd", // 9.b
"call this.calendar.dateAdd", // 9.c
"call this.calendar.dateAdd", // 9.e
@ -229,7 +237,7 @@ assert.compareArray(actual, expectedOpsForYearRounding, "order of operations wit
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest month:
const expectedOpsForMonthRounding = expected.concat(expectedOpsForCalendarDifference, [
const expectedOpsForMonthRounding = expected.concat(expectedOpsForCalendarDifference, expectedOpsForCalendarRounding, [
"get this.calendar.dateAdd", // 10.b
"call this.calendar.dateAdd", // 10.c
"call this.calendar.dateAdd", // 10.e
@ -240,7 +248,7 @@ assert.compareArray(actual, expectedOpsForMonthRounding, "order of operations wi
actual.splice(0); // clear
// code path through RoundDuration that rounds to the nearest week:
const expectedOpsForWeekRounding = expected.concat(expectedOpsForCalendarDifference, [
const expectedOpsForWeekRounding = expected.concat(expectedOpsForCalendarDifference, expectedOpsForCalendarRounding, [
"get this.calendar.dateAdd", // 11.c
"call this.calendar.dateAdd", // 11.d MoveRelativeDate
]); // (11.g.iii MoveRelativeDate not called because days already balanced)