Skip to content

Commit

Permalink
Normative: Remove loops in RoundDuration
Browse files Browse the repository at this point in the history
In RoundDuration, we already calculated years rounding without a loop. I'm
not sure why we didn't do this for months and weeks, but it's not
necessary to use a loop for those either.

This should not affect any results, but has an observable effect on calls
to custom calendar methods.
  • Loading branch information
ptomato committed Jun 20, 2023
1 parent 014e38f commit 705daa3
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 50 deletions.
11 changes: 9 additions & 2 deletions polyfill/lib/duration.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,14 @@ export class Duration {
) {
calendarRec.lookup('dateAdd');
}
if (largestUnit === 'year' || largestUnit === 'month' || largestUnit === 'week' || smallestUnit === 'year') {
if (
largestUnit === 'year' ||
largestUnit === 'month' ||
largestUnit === 'week' ||
smallestUnit === 'year' ||
smallestUnit === 'month' ||
smallestUnit === 'week'
) {
calendarRec.lookup('dateUntil');
}
}
Expand Down Expand Up @@ -479,7 +486,7 @@ export class Duration {
if (years !== 0 || months !== 0 || weeks !== 0 || unit === 'year' || unit === 'month' || unit === 'week') {
calendarRec.lookup('dateAdd');
}
if (unit === 'year' || (unit === 'month' && years !== 0)) {
if (unit === 'year' || unit === 'month' || unit === 'week') {
calendarRec.lookup('dateUntil');
}
}
Expand Down
89 changes: 61 additions & 28 deletions polyfill/lib/ecmascript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4143,7 +4143,9 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
settings.largestUnit === 'year' ||
settings.largestUnit === 'month' ||
settings.largestUnit === 'week' ||
settings.smallestUnit === 'year'
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week'
) {
calendarRec.lookup('dateUntil');
}
Expand Down Expand Up @@ -4211,7 +4213,9 @@ export function DifferenceTemporalPlainDateTime(operation, plainDateTime, other,
if (
(!datePartsIdentical &&
(settings.largestUnit === 'year' || settings.largestUnit === 'month' || settings.largestUnit === 'week')) ||
settings.smallestUnit === 'year'
settings.smallestUnit === 'year' ||
settings.smallestUnit === 'month' ||
settings.smallestUnit === 'week'
) {
calendarRec.lookup('dateUntil');
}
Expand Down Expand Up @@ -5498,8 +5502,11 @@ export function RoundDuration(
zonedRelativeTo = undefined,
timeZoneRec = undefined
) {
// dateAdd, dateUntil must be looked up if smallestUnit == year
// dateAdd must be looked up if smallestUnit == month or week
// calendarRec must have looked up:
// dateAdd, if
// smallestUnit == year, month, week or
// (smallestUnit == day && any of years, months, weeks days ≠ 0 && zonedRelativeTo defined)
// dateUntil, if smallestUnit == year, month, week && any of years, months, weeks ≠ 0
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');

if ((unit === 'year' || unit === 'month' || unit === 'week') && !plainRelativeTo) {
Expand Down Expand Up @@ -5598,21 +5605,32 @@ export function RoundDuration(
plainRelativeTo = yearsMonthsLater;
days += weeksInDays;

// Months may be different lengths of days depending on the calendar,
// convert days to months in a loop as described above under 'years'.
const sign = MathSign(days);
const isoResult = AddISODate(
GetSlot(plainRelativeTo, ISO_YEAR),
GetSlot(plainRelativeTo, ISO_MONTH),
GetSlot(plainRelativeTo, ISO_DAY),
0,
0,
0,
days,
'constrain'
);
const wholeDaysLater = CreateTemporalDate(isoResult.year, isoResult.month, isoResult.day, calendarRec.receiver);
const untilOptions = ObjectCreate(null);
untilOptions.largestUnit = 'month';
const monthsPassed = GetSlot(DifferenceDate(calendarRec, plainRelativeTo, wholeDaysLater, untilOptions), MONTHS);
months += monthsPassed;
const monthsPassedDuration = new TemporalDuration(0, monthsPassed);
let daysPassed;
({ relativeTo: plainRelativeTo, days: daysPassed } = MoveRelativeDate(
calendarRec,
plainRelativeTo,
monthsPassedDuration
));
days -= daysPassed;
const oneMonth = new TemporalDuration(0, days < 0 ? -1 : 1);
let oneMonthDays;
({ relativeTo: plainRelativeTo, days: oneMonthDays } = MoveRelativeDate(calendarRec, plainRelativeTo, oneMonth));
while (MathAbs(days) >= MathAbs(oneMonthDays)) {
months += sign;
days -= oneMonthDays;
({ relativeTo: plainRelativeTo, days: oneMonthDays } = MoveRelativeDate(
calendarRec,
plainRelativeTo,
oneMonth
));
}
let { days: oneMonthDays } = MoveRelativeDate(calendarRec, plainRelativeTo, oneMonth);

oneMonthDays = MathAbs(oneMonthDays);
const divisor = bigInt(oneMonthDays).multiply(dayLengthNs);
nanoseconds = divisor.multiply(months).plus(bigInt(days).multiply(dayLengthNs)).plus(nanoseconds);
Expand All @@ -5624,17 +5642,32 @@ export function RoundDuration(
break;
}
case 'week': {
// Weeks may be different lengths of days depending on the calendar,
// convert days to weeks in a loop as described above under 'years'.
const sign = MathSign(days);
const isoResult = AddISODate(
GetSlot(plainRelativeTo, ISO_YEAR),
GetSlot(plainRelativeTo, ISO_MONTH),
GetSlot(plainRelativeTo, ISO_DAY),
0,
0,
0,
days,
'constrain'
);
const wholeDaysLater = CreateTemporalDate(isoResult.year, isoResult.month, isoResult.day, calendarRec.receiver);
const untilOptions = ObjectCreate(null);
untilOptions.largestUnit = 'week';
const weeksPassed = GetSlot(DifferenceDate(calendarRec, plainRelativeTo, wholeDaysLater, untilOptions), WEEKS);
weeks += weeksPassed;
const weeksPassedDuration = new TemporalDuration(0, 0, weeksPassed);
let daysPassed;
({ relativeTo: plainRelativeTo, days: daysPassed } = MoveRelativeDate(
calendarRec,
plainRelativeTo,
weeksPassedDuration
));
days -= daysPassed;
const oneWeek = new TemporalDuration(0, 0, days < 0 ? -1 : 1);
let oneWeekDays;
({ relativeTo: plainRelativeTo, days: oneWeekDays } = MoveRelativeDate(calendarRec, plainRelativeTo, oneWeek));
while (MathAbs(days) >= MathAbs(oneWeekDays)) {
weeks += sign;
days -= oneWeekDays;
({ relativeTo: plainRelativeTo, days: oneWeekDays } = MoveRelativeDate(calendarRec, plainRelativeTo, oneWeek));
}
let { days: oneWeekDays } = MoveRelativeDate(calendarRec, plainRelativeTo, oneWeek);

oneWeekDays = MathAbs(oneWeekDays);
const divisor = bigInt(oneWeekDays).multiply(dayLengthNs);
nanoseconds = divisor.multiply(weeks).plus(bigInt(days).multiply(dayLengthNs)).plus(nanoseconds);
Expand Down
48 changes: 30 additions & 18 deletions spec/duration.html
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ <h1>Temporal.Duration.prototype.round ( _roundTo_ )</h1>
1. Set _calendarRec_ to ! CreateCalendarRecord(_calendar_, « »).
1. If _duration_.[[Years]] &ne; 0; or _duration_.[[Months]] &ne; 0; or _duration_.[[Weeks]] &ne; 0; or _duration_.[[Days]] &ne; 0 and _largestUnit_ is any of *"year"*, *"month"*, *"week"*; or _smallestUnit_ is any of *"year"*, *"month"*, *"week"*; then
1. Set _calendarRec_.[[DateAdd]] to ? GetMethod(_calendar_, *"dateAdd"*).
1. If _largestUnit_ is any of *"year"*, *"month"*, *"week"*, or _smallestUnit_ is *"year"*, then
1. If _largestUnit_ is any of *"year"*, *"month"*, *"week"*, or _smallestUnit_ is any of *"year"*, *"month"*, *"week"*, then
1. Set _calendarRec_.[[DateUntil]] to ? GetMethod(_calendar_, *"dateUntil"*).
1. Let _unbalanceResult_ be ? UnbalanceDateDurationRelative(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _largestUnit_, _plainRelativeTo_, _calendarRec_).
1. Let _roundRecord_ be ? RoundDuration(_unbalanceResult_.[[Years]], _unbalanceResult_.[[Months]], _unbalanceResult_.[[Weeks]], _unbalanceResult_.[[Days]], _duration_.[[Hours]], _duration_.[[Minutes]], _duration_.[[Seconds]], _duration_.[[Milliseconds]], _duration_.[[Microseconds]], _duration_.[[Nanoseconds]], _roundingIncrement_, _smallestUnit_, _roundingMode_, _plainRelativeTo_, _calendarRec_, _zonedRelativeTo_, _timeZoneRec_).
Expand Down Expand Up @@ -532,7 +532,7 @@ <h1>Temporal.Duration.prototype.total ( _totalOf_ )</h1>
1. Set _calendarRec_ to ! CreateCalendarRecord(_calendar_, « »).
1. If _duration_.[[Years]] &ne; 0; or _duration_.[[Months]] &ne; 0; or _duration_.[[Weeks]] &ne; 0; or _unit_ is any of *"year"*, *"month"*, *"week"*; then
1. Set _calendarRec_.[[DateAdd]] to ? GetMethod(_calendar_, *"dateAdd"*).
1. If _unit_ is *"year"*, or _unit_ is *"month"* and _duration_.[[Years]] &ne; 0, then
1. If _unit_ is any of *"year"*, *"month"*, *"week"*, then
1. Set _calendarRec_.[[DateUntil]] to ? GetMethod(_calendar_, *"dateUntil"*).
1. Let _unbalanceResult_ be ? UnbalanceDateDurationRelative(_duration_.[[Years]], _duration_.[[Months]], _duration_.[[Weeks]], _duration_.[[Days]], _unit_, _plainRelativeTo_, _calendarRec_).
1. If _zonedRelativeTo_ is not *undefined*, then
Expand Down Expand Up @@ -1656,10 +1656,12 @@ <h1>
<emu-alg>
1. Assert: If either of _plainRelativeTo_ or _zonedRelativeTo_ are present and not *undefined*, _calendarRec_ is not *undefined*.
1. Assert: If _zonedRelativeTo_ is present and not *undefined*, _timeZoneRec_ is not *undefined*.
1. Assert: If _unit_ is *"year"*, neither of _calendarRec_.[[DateAdd]] or _calendarRec_.[[DateUntil]] are *undefined*.
1. Assert: If _unit_ is *"month"* or *"week"*, _calendarRec_.[[DateAdd]] is not *undefined*.
1. If _unit_ is any of *"year"*, *"month"*, *"week"*, then
1. Assert: _calendarRec_.[[DateAdd]] is not *undefined*.
1. Assert: If _years_ &ne; 0, or _months_ &ne; 0, or _weeks_ &ne; 0, _calendarRec_.[[DateUntil]] is not *undefined*.
1. If _plainRelativeTo_ is not present, set _plainRelativeTo_ to *undefined*.
1. If _zonedRelativeTo_ is not present, set _zonedRelativeTo_ to *undefined*.
1. Assert: If _unit_ is *"day"*; and _zonedRelativeTo_ is not *undefined*; and _years_ &ne; 0, or _months_ &ne; 0, or _weeks_ &ne; 0, or _days_ &ne; 0, _calendarRec_.[[DateAdd]] is not *undefined*.
1. If _unit_ is *"year"*, *"month"*, or *"week"*, and _plainRelativeTo_ is *undefined*, then
1. Throw a *RangeError* exception.
1. If _unit_ is one of *"year"*, *"month"*, *"week"*, or *"day"*, then
Expand Down Expand Up @@ -1712,33 +1714,43 @@ <h1>
1. Let _weeksInDays_ be DaysUntil(_yearsMonthsLater_, _yearsMonthsWeeksLater_).
1. Set _plainRelativeTo_ to _yearsMonthsLater_.
1. Set _fractionalDays_ to _fractionalDays_ + _weeksInDays_.
1. Let _isoResult_ be ! AddISODate(_plainRelativeTo_.[[ISOYear]], _plainRelativeTo_.[[ISOMonth]], _plainRelativeTo_.[[ISODay]], 0, 0, 0, truncate(_fractionalDays_), *"constrain"*).
1. Let _wholeDaysLater_ be ! CreateTemporalDate(_isoResult_.[[Year]], _isoResult_.[[Month]], _isoResult_.[[Day]], _calendarRec_.[[Receiver]]).
1. Let _untilOptions_ be OrdinaryObjectCreate(*null*).
1. Perform ! CreateDataPropertyOrThrow(_untilOptions_, *"largestUnit"*, *"month"*).
1. Let _timePassed_ be ? DifferenceDate(_calendarRec_, _plainRelativeTo_, _wholeDaysLater_, _untilOptions_).
1. Let _monthsPassed_ be _timePassed_.[[Months]].
1. Set _months_ to _months_ + _monthsPassed_.
1. Let _monthsPassedDuration_ be ! CreateTemporalDuration(0, _monthsPassed_, 0, 0, 0, 0, 0, 0, 0, 0).
1. Let _moveResult_ be ? MoveRelativeDate(_calendarRec_, _plainRelativeTo_, _monthsPassedDuration_).
1. Set _plainRelativeTo_ to _moveResult_.[[RelativeTo]].
1. Let _daysPassed_ be _moveResult_.[[Days]].
1. Set _fractionalDays_ to _fractionalDays_ - _daysPassed_.
1. If _fractionalDays_ &lt; 0, let _sign_ be -1; else, let _sign_ be 1.
1. Let _oneMonth_ be ! CreateTemporalDuration(0, _sign_, 0, 0, 0, 0, 0, 0, 0, 0).
1. Let _moveResult_ be ? MoveRelativeDate(_calendarRec_, _plainRelativeTo_, _oneMonth_).
1. Set _plainRelativeTo_ to _moveResult_.[[RelativeTo]].
1. Let _oneMonthDays_ be _moveResult_.[[Days]].
1. Repeat, while abs(_fractionalDays_) &ge; abs(_oneMonthDays_),
1. Set _months_ to _months_ + _sign_.
1. Set _fractionalDays_ to _fractionalDays_ - _oneMonthDays_.
1. Set _moveResult_ to ? MoveRelativeDate(_calendarRec_, _plainRelativeTo_, _oneMonth_).
1. Set _plainRelativeTo_ to _moveResult_.[[RelativeTo]].
1. Set _oneMonthDays_ to _moveResult_.[[Days]].
1. Let _fractionalMonths_ be _months_ + _fractionalDays_ / abs(_oneMonthDays_).
1. Set _months_ to RoundNumberToIncrement(_fractionalMonths_, _increment_, _roundingMode_).
1. Set _total_ to _fractionalMonths_.
1. Set _weeks_ to 0.
1. Else if _unit_ is *"week"*, then
1. Let _isoResult_ be ! AddISODate(_plainRelativeTo_.[[ISOYear]], _plainRelativeTo_.[[ISOMonth]], _plainRelativeTo_.[[ISODay]], 0, 0, 0, truncate(_fractionalDays_), *"constrain"*).
1. Let _wholeDaysLater_ be ! CreateTemporalDate(_isoResult_.[[Year]], _isoResult_.[[Month]], _isoResult_.[[Day]], _calendarRec_.[[Receiver]]).
1. Let _untilOptions_ be OrdinaryObjectCreate(*null*).
1. Perform ! CreateDataPropertyOrThrow(_untilOptions_, *"largestUnit"*, *"week"*).
1. Let _timePassed_ be ? DifferenceDate(_calendarRec_, _plainRelativeTo_, _wholeDaysLater_, _untilOptions_).
1. Let _weeksPassed_ be _timePassed_.[[Weeks]].
1. Set _weeks_ to _weeks_ + _weeksPassed_.
1. Let _weeksPassedDuration_ be ! CreateTemporalDuration(0, 0, _weeksPassed_, 0, 0, 0, 0, 0, 0, 0).
1. Let _moveResult_ be ? MoveRelativeDate(_calendarRec_, _plainRelativeTo_, _weeksPassedDuration_).
1. Set _plainRelativeTo_ to _moveResult_.[[RelativeTo]].
1. Let _daysPassed_ be _moveResult_.[[Days]].
1. Set _fractionalDays_ to _fractionalDays_ - _daysPassed_.
1. If _fractionalDays_ &lt; 0, let _sign_ be -1; else, let _sign_ be 1.
1. Let _oneWeek_ be ! CreateTemporalDuration(0, 0, _sign_, 0, 0, 0, 0, 0, 0, 0).
1. Let _moveResult_ be ? MoveRelativeDate(_calendarRec_, _plainRelativeTo_, _oneWeek_).
1. Set _plainRelativeTo_ to _moveResult_.[[RelativeTo]].
1. Let _oneWeekDays_ be _moveResult_.[[Days]].
1. Repeat, while abs(_fractionalDays_) &ge; abs(_oneWeekDays_),
1. Set _weeks_ to _weeks_ + _sign_.
1. Set _fractionalDays_ to _fractionalDays_ - _oneWeekDays_.
1. Set _moveResult_ to ? MoveRelativeDate(_calendarRec_, _plainRelativeTo_, _oneWeek_).
1. Set _plainRelativeTo_ to _moveResult_.[[RelativeTo]].
1. Set _oneWeekDays_ to _moveResult_.[[Days]].
1. Let _fractionalWeeks_ be _weeks_ + _fractionalDays_ / abs(_oneWeekDays_).
1. Set _weeks_ to RoundNumberToIncrement(_fractionalWeeks_, _increment_, _roundingMode_).
1. Set _total_ to _fractionalWeeks_.
Expand Down
2 changes: 1 addition & 1 deletion spec/plaindate.html
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,7 @@ <h1>
1. Let _calendarRec_ be ! CreateCalendarRecord(_temporalDate_.[[Calendar]], « »).
1. If _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Set _calendarRec_.[[DateAdd]] to ? GetMethod(_temporalDate_.[[Calendar]], *"dateAdd"*).
1. If _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. If _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, or _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, *"week"*, then
1. Set _calendarRec_.[[DateUntil]] to ? GetMethod(_temporalDate_.[[Calendar]], *"dateUntil"*).
1. Let _result_ be ? DifferenceDate(_calendarRec_, _temporalDate_, _other_, _resolvedOptions_).
1. If _settings_.[[SmallestUnit]] is not *"day"* or _settings_.[[RoundingIncrement]] &ne; 1, then
Expand Down
2 changes: 1 addition & 1 deletion spec/plaindatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,7 @@ <h1>
1. Let _largestUnitRequiresDateUntilLookup_ be *false*.
1. If _datePartsIdentical_ is *false* and _settings_.[[LargestUnit]] is one of *"year"*, *"month"*, or *"week"*, then
1. Set _largestUnitRequiresDateUntilLookup_ to *true*.
1. If _largestUnitRequiresDateUntilLookup_ is *true*, or _settings_.[[SmallestUnit]] is *"year"*, then
1. If _largestUnitRequiresDateUntilLookup_ is *true*, or _settings_.[[SmallestUnit]] is one of *"year"*, *"month"*, *"week"*, then
1. Set _calendarRec_.[[DateUntil]] to ? GetMethod(_dateTime_.[[Calendar]], *"dateUntil"*).
1. Let _diff_ be ? DifferenceISODateTime(_dateTime_.[[ISOYear]], _dateTime_.[[ISOMonth]], _dateTime_.[[ISODay]], _dateTime_.[[ISOHour]], _dateTime_.[[ISOMinute]], _dateTime_.[[ISOSecond]], _dateTime_.[[ISOMillisecond]], _dateTime_.[[ISOMicrosecond]], _dateTime_.[[ISONanosecond]], _other_.[[ISOYear]], _other_.[[ISOMonth]], _other_.[[ISODay]], _other_.[[ISOHour]], _other_.[[ISOMinute]], _other_.[[ISOSecond]], _other_.[[ISOMillisecond]], _other_.[[ISOMicrosecond]], _other_.[[ISONanosecond]], _dateTime_.[[Calendar]], _settings_.[[LargestUnit]], _resolvedOptions_, _calendarRec_).
1. If _settings_.[[SmallestUnit]] is *"nanosecond"* and _settings_.[[RoundingIncrement]] is 1, then
Expand Down

0 comments on commit 705daa3

Please sign in to comment.