Duration normalization, part 1 of 3 #2722
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 6, 2023
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2722 +/- ##
==========================================
- Coverage 96.39% 96.38% -0.02%
==========================================
Files 21 21
Lines 12497 12444 -53
Branches 2262 2250 -12
==========================================
- Hits 12047 11994 -53
Misses 395 395
Partials 55 55 ☔ View full report in Codecov by Sentry. |
ptomato
force-pushed
the
duration-normalize-part-1
branch
from
2afc671
to
8988a15
Compare
justingrant
approved these changes
Nov 15, 2023
cjtenny
approved these changes
Nov 15, 2023
ptomato
commented
Nov 15, 2023
gibson042
reviewed
Nov 15, 2023
…nRelative If we're going to limit calendar units, these loops can be replaced by dateAdd + dateUntil and it's no longer necessary to calculate with BigInt. This should not affect any results, but has an observable effect on calls to custom calendar methods.
ptomato
force-pushed
the
duration-normalize-part-1
branch
from
8988a15
to
c53a8d4
Compare
…elative
Similar to previous commit, if we are limiting calendar units then we can
replace these loops with one dateAdd + dateUntil each, and it's no longer
necessary to calculate with BigInt.
This changes some rounding results, in two ways.
(1) The previous looping algorithm didn't add duration units to a
relativeTo date in the correct RFC 5545 order, so results may change when
balancing up to months or years due to different year or month lengths.
(2) We now have to make a special case in BalanceDateDurationRelative for
largestUnit years/months and smallestUnit weeks, because months are not
generally an integer number of weeks. For example, the dateAdd/dateUntil
approach with no special case could result in
const duration = Temporal.Duration.from({ years: 1, weeks: 5 });
duration.round({ smallestUnit: 'weeks', relativeTo: '2023-11-15' })
returning a duration of 1 year, 1 month, 5 days, which clearly contravenes
the requested smallestUnit.
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 has an observable effect on calls to custom calendar methods, but also affects a few results: for example, some rounding operations with extremely long durations will now throw because the calendar calculations become out of range.
In order to calculate the rounded value and totals in RoundDuration, when rounding to years, months, or weeks, we need to divide by the length of the year, month, or week in days. Throw an exception if this would otherwise result in division by zero. This is marked 'editorial' because division by zero with mathematical values is an editorial error in the spec. However, the month/week divisions didn't exist before the normative change in the previous commit, so this would previously only have applied to years. Closes: #2335
ptomato
force-pushed
the
duration-normalize-part-1
branch
from
c53a8d4
to
db2d9f8
Compare
|
Thanks for the reviews, everyone. I think I've addressed all the comments. Click to reveal a convenient diff of what changed since the last versiondiff --git a/polyfill/lib/duration.mjs b/polyfill/lib/duration.mjs
index 8192ca89..04397d6b 100644
--- a/polyfill/lib/duration.mjs
+++ b/polyfill/lib/duration.mjs
@@ -413,6 +413,7 @@ export class Duration {
weeks,
days,
largestUnit,
+ smallestUnit,
plainRelativeTo,
calendarRec
));
diff --git a/polyfill/lib/ecmascript.mjs b/polyfill/lib/ecmascript.mjs
index 5140db62..9e08ad6c 100644
--- a/polyfill/lib/ecmascript.mjs
+++ b/polyfill/lib/ecmascript.mjs
@@ -3535,10 +3535,7 @@ export function BalancePossiblyInfiniteTimeDurationRelative(
}
export function UnbalanceDateDurationRelative(years, months, weeks, days, largestUnit, plainRelativeTo, calendarRec) {
- // calendarRec must be present and must have looked up:
- // * largestUnit month and years !== 0: dateAdd and dateUntil
- // * largestUnit week and (years !== 0 or months !== 0): dateAdd
- // * largestUnit day or smaller, and (years, months, or weeks !== 0): dateAdd
+ // calendarRec must have looked up dateAdd and dateUntil
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');
switch (largestUnit) {
@@ -3565,6 +3562,7 @@ export function UnbalanceDateDurationRelative(years, months, weeks, days, larges
return { years: 0, months: 0, weeks, days: days + yearsMonthsInDays };
}
default: {
+ // largestUnit is "day", or any time unit
if (years === 0 && months === 0 && weeks === 0) return { years: 0, months: 0, weeks: 0, days };
if (!calendarRec) throw new RangeError('a starting point is required for balancing calendar units');
// balance years, months, and weeks down to days
@@ -3575,19 +3573,22 @@ export function UnbalanceDateDurationRelative(years, months, weeks, days, larges
}
}
-export function BalanceDateDurationRelative(years, months, weeks, days, largestUnit, plainRelativeTo, calendarRec) {
- // calendarRec must have looked up:
- // - if largestUnit == year:
- // - if years or months nonzero, dateAdd and dateUntil
- // - if weeks or days nonzero, dateUntil
- // - if largestUnit == month:
- // - if months nonzero, dateAdd and dateUntil
- // - if weeks or days nonzero, dateUntil
- // - if largestUnit == week:
- // - if weeks nonzero, dateAdd and dateUntil
- // - if days nonzero, dateUntil
+export function BalanceDateDurationRelative(
+ years,
+ months,
+ weeks,
+ days,
+ largestUnit,
+ smallestUnit,
+ plainRelativeTo,
+ calendarRec
+) {
+ // calendarRec must have looked up dateAdd and dateUntil
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');
+ // If no nonzero calendar units, then there's nothing to balance.
+ // If largestUnit is 'day' or lower, then the balance is a no-op.
+ // In both cases, return early. Anything after this requires a calendar.
if (
(years === 0 && months === 0 && weeks === 0 && days === 0) ||
(largestUnit !== 'year' && largestUnit !== 'month' && largestUnit !== 'week')
@@ -3602,24 +3603,44 @@ export function BalanceDateDurationRelative(years, months, weeks, days, largestU
switch (largestUnit) {
case 'year': {
- // balance months and days up to years
- const later = AddDate(calendarRec, plainRelativeTo, new TemporalDuration(years, months, 0, days));
+ // There is a special case for smallestUnit === week, because months and
+ // years aren't equal to an integer number of weeks. We don't want "1 year
+ // and 5 weeks" to balance to "1 year, 1 month, and 5 days" which would
+ // contravene the requested smallestUnit.
+ if (smallestUnit === 'week') {
+ // balance months up to years
+ const later = AddDate(calendarRec, plainRelativeTo, new TemporalDuration(years, months));
+ const untilResult = CalendarDateUntil(calendarRec, plainRelativeTo, later, untilOptions);
+ return {
+ years: GetSlot(untilResult, YEARS),
+ months: GetSlot(untilResult, MONTHS),
+ weeks,
+ days: 0
+ };
+ }
+ // balance weeks, months and days up to years
+ const later = AddDate(calendarRec, plainRelativeTo, new TemporalDuration(years, months, weeks, days));
const untilResult = CalendarDateUntil(calendarRec, plainRelativeTo, later, untilOptions);
return {
years: GetSlot(untilResult, YEARS),
months: GetSlot(untilResult, MONTHS),
- weeks,
+ weeks: GetSlot(untilResult, WEEKS),
days: GetSlot(untilResult, DAYS)
};
}
case 'month': {
- // balance days up to months
- const later = AddDate(calendarRec, plainRelativeTo, new TemporalDuration(0, months, 0, days));
+ // Same special case for rounding to weeks as above; in this case we
+ // don't need to balance.
+ if (smallestUnit === 'week') {
+ return { years: 0, months, weeks, days: 0 };
+ }
+ // balance weeks and days up to months
+ const later = AddDate(calendarRec, plainRelativeTo, new TemporalDuration(0, months, weeks, days));
const untilResult = CalendarDateUntil(calendarRec, plainRelativeTo, later, untilOptions);
return {
years: 0,
months: GetSlot(untilResult, MONTHS),
- weeks,
+ weeks: GetSlot(untilResult, WEEKS),
days: GetSlot(untilResult, DAYS)
};
}
@@ -4223,6 +4244,7 @@ export function DifferenceTemporalPlainDate(operation, plainDate, other, options
weeks,
days,
settings.largestUnit,
+ settings.smallestUnit,
plainDate,
calendarRec
));
@@ -4321,6 +4343,7 @@ export function DifferenceTemporalPlainDateTime(operation, plainDateTime, other,
weeks,
days,
settings.largestUnit,
+ settings.smallestUnit,
relativeTo,
calendarRec
));
@@ -4454,7 +4477,16 @@ export function DifferenceTemporalPlainYearMonth(operation, yearMonth, other, op
thisDate,
calendarRec
));
- ({ years, months } = BalanceDateDurationRelative(years, months, 0, 0, settings.largestUnit, thisDate, calendarRec));
+ ({ years, months } = BalanceDateDurationRelative(
+ years,
+ months,
+ 0,
+ 0,
+ settings.largestUnit,
+ settings.smallestUnit,
+ thisDate,
+ calendarRec
+ ));
}
return new Duration(sign * years, sign * months, 0, 0, 0, 0, 0, 0, 0, 0);
@@ -4580,6 +4612,7 @@ export function DifferenceTemporalZonedDateTime(operation, zonedDateTime, other,
weeks,
days,
settings.largestUnit,
+ settings.smallestUnit,
plainRelativeTo,
calendarRec
));
@@ -5577,10 +5610,7 @@ export function RoundDuration(
timeZoneRec = undefined,
precalculatedPlainDateTime = undefined
) {
- // dateAdd must be looked up if:
- // - unit is year, month, or week
- // - unit is day, zonedRelativeTo defined, and any of years...weeks != 0
- // dateUntil must be looked up if unit is year, month, or week, and any of years...weeks != 0
+ // dateAdd and dateUntil must be looked up
const TemporalDuration = GetIntrinsic('%Temporal.Duration%');
if ((unit === 'year' || unit === 'month' || unit === 'week') && !plainRelativeTo) {
diff --git a/spec/duration.html b/spec/duration.html
index f17894a4..93f3cec0 100644
--- a/spec/duration.html
+++ b/spec/duration.html
@@ -476,7 +476,7 @@
1. Let _balanceResult_ be ? BalanceTimeDurationRelative(_roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _largestUnit_, _zonedRelativeTo_, _timeZoneRec_, _precalculatedPlainDateTime_).
1. Else,
1. Let _balanceResult_ be ? BalanceTimeDuration(_roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _largestUnit_).
- 1. Let _result_ be ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _balanceResult_.[[Days]], _largestUnit_, _plainRelativeTo_, _calendarRec_).
+ 1. Let _result_ be ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _balanceResult_.[[Days]], _largestUnit_, _smallestUnit_, _plainRelativeTo_, _calendarRec_).
1. Return ! CreateTemporalDuration(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], _balanceResult_.[[Hours]], _balanceResult_.[[Minutes]], _balanceResult_.[[Seconds]], _balanceResult_.[[Milliseconds]], _balanceResult_.[[Microseconds]], _balanceResult_.[[Nanoseconds]]).
</emu-alg>
</emu-clause>
@@ -1422,7 +1422,7 @@
1. Let _later_ be ? CalendarDateAdd(_calendarRec_, _plainRelativeTo_, _yearsMonthsDuration_).
1. Let _yearsMonthsInDays_ be DaysUntil(_plainRelativeTo_, _later_).
1. Return ? CreateDateDurationRecord(0, 0, _weeks_, _days_ + _yearsMonthsInDays_).
- 1. Assert: _largestUnit_ is *"day"*.
+ 1. NOTE: _largestUnit_ can be any time unit as well as *"day"*.
1. If _years_ = 0, and _months_ = 0, and _weeks_ = 0, return ! CreateDateDurationRecord(0, 0, 0, _days_).
1. If _calendarRec_ is *undefined*, then
1. Throw a *RangeError* exception.
@@ -1442,13 +1442,14 @@
_weeks_: an integer,
_days_: an integer,
_largestUnit_: a String,
+ _smallestUnit_: a String,
_plainRelativeTo_: *undefined* or a Temporal.PlainDate,
_calendarRec_: *undefined* or a Calendar Methods Record,
)
</h1>
<dl class="header">
<dt>description</dt>
- <dd>It converts the calendar units of a duration into a form where lower units are converted into higher units as much as possible, up to _largestUnit_, and returns the result as a Date Duration Record.</dd>
+ <dd>It converts the calendar units of a duration into a form where lower units are converted into higher units as much as possible, up to _largestUnit_ and not lower than _smallestUnit_, and returns the result as a Date Duration Record.</dd>
</dl>
<emu-alg>
1. Assert: If _plainRelativeTo_ is not *undefined*, _calendarRec_ is not *undefined*.
@@ -1458,26 +1459,33 @@
1. Return ! CreateDateDurationRecord(_years_, _months_, _weeks_, _days_).
1. If _plainRelativeTo_ is *undefined*, then
1. Throw a *RangeError* exception.
+ 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateUntil~) is *true*.
1. Let _untilOptions_ be OrdinaryObjectCreate(*null*).
1. Perform ! CreateDataPropertyOrThrow(_untilOptions_, *"largestUnit"*, _largestUnit_).
1. If _largestUnit_ is *"year"*, then
- 1. Assert: If _years_ ≠ 0 or _months_ ≠ 0, CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
- 1. Let _yearsMonthsDaysDuration_ be ! CreateTemporalDuration(_years_, _months_, 0, _days_, 0, 0, 0, 0, 0, 0).
- 1. Let _later_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _yearsMonthsDaysDuration_).
+ 1. If _smallestUnit_ is *"week"*, then
+ 1. Assert: _days_ = 0.
+ 1. Let _yearsMonthsDuration_ be ! CreateTemporalDuration(_years_, _months_, 0, 0, 0, 0, 0, 0, 0, 0).
+ 1. Let _later_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _yearsMonthsDuration_).
+ 1. Let _untilResult_ be ? CalendarDateUntil(_calendarRec_, _plainRelativeTo_, _later_, _untilOptions_).
+ 1. Return ? CreateDateDurationRecord(_untilResult_.[[Years]], _untilResult_.[[Months]], _weeks_, 0).
+ 1. Let _yearsMonthsWeeksDaysDuration_ be ! CreateTemporalDuration(_years_, _months_, _weeks_, _days_, 0, 0, 0, 0, 0, 0).
+ 1. Let _later_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _yearsMonthsWeeksDaysDuration_).
1. Let _untilResult_ be ? CalendarDateUntil(_calendarRec_, _plainRelativeTo_, _later_, _untilOptions_).
- 1. Return ? CreateDateDurationRecord(_untilResult_.[[Years]], _untilResult_.[[Months]], _weeks_, _untilResult_.[[Days]]).
+ 1. Return ? CreateDateDurationRecord(_untilResult_.[[Years]], _untilResult_.[[Months]], _untilResult_.[[Weeks]], _untilResult_.[[Days]]).
1. If _largestUnit_ is *"month"*, then
1. Assert: _years_ = 0.
- 1. Assert: If _months_ ≠ 0, CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
- 1. Let _monthsDaysDuration_ be ! CreateTemporalDuration(0, _months_, 0, _days_, 0, 0, 0, 0, 0, 0).
- 1. Let _later_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _monthsDaysDuration_).
+ 1. If _smallestUnit_ is *"week"*, then
+ 1. Assert: _days_ = 0.
+ 1. Return ! CreateDateDurationRecord(0, _months_, _weeks_, 0).
+ 1. Let _monthsWeeksDaysDuration_ be ! CreateTemporalDuration(0, _months_, _weeks_, _days_, 0, 0, 0, 0, 0, 0).
+ 1. Let _later_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _monthsWeeksDaysDuration_).
1. Let _untilResult_ be ? CalendarDateUntil(_calendarRec_, _plainRelativeTo_, _later_, _untilOptions_).
- 1. Return ? CreateDateDurationRecord(0, _untilResult_.[[Months]], _weeks_, _untilResult_.[[Days]]).
+ 1. Return ? CreateDateDurationRecord(0, _untilResult_.[[Months]], _untilResult_.[[Weeks]], _untilResult_.[[Days]]).
1. Assert: _largestUnit_ is *"week"*.
1. Assert: _years_ = 0.
1. Assert: _months_ = 0.
- 1. Assert: If _weeks_ ≠ 0, CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
1. Let _weeksDaysDuration_ be ! CreateTemporalDuration(0, 0, _weeks_, _days_, 0, 0, 0, 0, 0, 0).
1. Let _later_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _weeksDaysDuration_).
1. Let _untilResult_ be ? CalendarDateUntil(_calendarRec_, _plainRelativeTo_, _later_, _untilOptions_).
@@ -1666,8 +1674,10 @@
1. If _plainRelativeTo_ is not present, set _plainRelativeTo_ to *undefined*.
1. If _zonedRelativeTo_ is not present, set _zonedRelativeTo_ to *undefined*.
1. If _precalculatedPlainDateTime_ is not present, set _precalculatedPlainDateTime_ to *undefined*.
- 1. If _unit_ is *"year"*, *"month"*, or *"week"*, and _plainRelativeTo_ is *undefined*, then
- 1. Throw a *RangeError* exception.
+ 1. If _unit_ is *"year"*, *"month"*, or *"week"*, then
+ 1. If _plainRelativeTo_ is *undefined*, throw a *RangeError* exception.
+ 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
+ 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateUntil~) is *true*.
1. If _unit_ is one of *"year"*, *"month"*, *"week"*, or *"day"*, then
1. Let _nanoseconds_ be TotalDurationNanoseconds(_hours_, _minutes_, _seconds_, _milliseconds_, _microseconds_, _nanoseconds_).
1. If _zonedRelativeTo_ is not *undefined*, then
@@ -1683,8 +1693,6 @@
1. Assert: _fractionalDays_ is not used below.
1. Let _total_ be ~unset~.
1. If _unit_ is *"year"*, then
- 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
- 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateUntil~) is *true*.
1. Let _yearsDuration_ be ! CreateTemporalDuration(_years_, 0, 0, 0, 0, 0, 0, 0, 0, 0).
1. Let _yearsLater_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _yearsDuration_).
1. Let _yearsMonthsWeeks_ be ! CreateTemporalDuration(_years_, _months_, _weeks_, 0, 0, 0, 0, 0, 0, 0).
@@ -1714,8 +1722,6 @@
1. Set _total_ to _fractionalYears_.
1. Set _months_ and _weeks_ to 0.
1. Else if _unit_ is *"month"*, then
- 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
- 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateUntil~) is *true*.
1. Let _yearsMonths_ be ! CreateTemporalDuration(_years_, _months_, 0, 0, 0, 0, 0, 0, 0, 0).
1. Let _yearsMonthsLater_ be ? AddDate(_calendarRec_, _plainRelativeTo_, _yearsMonths_).
1. Let _yearsMonthsWeeks_ be ! CreateTemporalDuration(_years_, _months_, _weeks_, 0, 0, 0, 0, 0, 0, 0).
@@ -1745,8 +1751,6 @@
1. Set _total_ to _fractionalMonths_.
1. Set _weeks_ to 0.
1. Else if _unit_ is *"week"*, then
- 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateAdd~) is *true*.
- 1. Assert: CalendarMethodsRecordHasLookedUp(_calendarRec_, ~dateUntil~) is *true*.
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*).
diff --git a/spec/plaindate.html b/spec/plaindate.html
index 4cae182b..ba6ab12f 100644
--- a/spec/plaindate.html
+++ b/spec/plaindate.html
@@ -1058,7 +1058,7 @@
1. If _roundingGranularityIsNoop_ is *false*, then
1. Let _roundRecord_ be ? RoundDuration(_result_.[[Years]], _result_.[[Months]], _result_.[[Weeks]], _result_.[[Days]], 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _temporalDate_, _calendarRec_).
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
- 1. Set _result_ to ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _roundResult_.[[Days]], _settings_.[[LargestUnit]], _temporalDate_, _calendarRec_).
+ 1. Set _result_ to ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _roundResult_.[[Days]], _settings_.[[LargestUnit]], _settings_.[[SmallestUnit]], _temporalDate_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ × _result_.[[Years]], _sign_ × _result_.[[Months]], _sign_ × _result_.[[Weeks]], _sign_ × _result_.[[Days]], 0, 0, 0, 0, 0, 0).
</emu-alg>
</emu-clause>
diff --git a/spec/plaindatetime.html b/spec/plaindatetime.html
index 7ba4193b..b5d3636f 100644
--- a/spec/plaindatetime.html
+++ b/spec/plaindatetime.html
@@ -1312,7 +1312,7 @@
1. Let _roundRecord_ be ? RoundDuration(_diff_.[[Years]], _diff_.[[Months]], _diff_.[[Weeks]], _diff_.[[Days]], _diff_.[[Hours]], _diff_.[[Minutes]], _diff_.[[Seconds]], _diff_.[[Milliseconds]], _diff_.[[Microseconds]], _diff_.[[Nanoseconds]], _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _relativeTo_, _calendarRec_).
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
1. Let _result_ be ? BalanceTimeDuration(_roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _settings_.[[LargestUnit]]).
- 1. Let _balanceResult_ be ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _result_.[[Days]], _settings_.[[LargestUnit]], _relativeTo_, _calendarRec_).
+ 1. Let _balanceResult_ be ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _result_.[[Days]], _settings_.[[LargestUnit]], _settings_.[[SmallestUnit]], _relativeTo_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ × _balanceResult_.[[Years]], _sign_ × _balanceResult_.[[Months]], _sign_ × _balanceResult_.[[Weeks]], _sign_ × _balanceResult_.[[Days]], _sign_ × _result_.[[Hours]], _sign_ × _result_.[[Minutes]], _sign_ × _result_.[[Seconds]], _sign_ × _result_.[[Milliseconds]], _sign_ × _result_.[[Microseconds]], _sign_ × _result_.[[Nanoseconds]]).
</emu-alg>
</emu-clause>
diff --git a/spec/plainyearmonth.html b/spec/plainyearmonth.html
index e4037227..8d11d360 100644
--- a/spec/plainyearmonth.html
+++ b/spec/plainyearmonth.html
@@ -654,7 +654,7 @@
1. If _settings_.[[SmallestUnit]] is not *"month"* or _settings_.[[RoundingIncrement]] ≠ 1, then
1. Let _roundRecord_ be ? RoundDuration(_result_.[[Years]], _result_.[[Months]], 0, 0, 0, 0, 0, 0, 0, 0, _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _thisDate_, _calendarRec_).
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
- 1. Set _result_ to ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], 0, 0, _settings_.[[LargestUnit]], _thisDate_, _calendarRec_).
+ 1. Set _result_ to ? BalanceDateDurationRelative(_roundResult_.[[Years]], _roundResult_.[[Months]], 0, 0, _settings_.[[LargestUnit]], _settings_.[[SmallestUnit]], _thisDate_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ × _result_.[[Years]], _sign_ × _result_.[[Months]], 0, 0, 0, 0, 0, 0, 0, 0).
</emu-alg>
</emu-clause>
diff --git a/spec/zoneddatetime.html b/spec/zoneddatetime.html
index 722ec816..39001c97 100644
--- a/spec/zoneddatetime.html
+++ b/spec/zoneddatetime.html
@@ -1521,7 +1521,7 @@
1. Let _roundRecord_ be ? RoundDuration(_difference_.[[Years]], _difference_.[[Months]], _difference_.[[Weeks]], _difference_.[[Days]], _difference_.[[Hours]], _difference_.[[Minutes]], _difference_.[[Seconds]], _difference_.[[Milliseconds]], _difference_.[[Microseconds]], _difference_.[[Nanoseconds]], _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _plainRelativeTo_, _calendarRec_, _zonedDateTime_, _timeZoneRec_, _precalculatedPlainDateTime_).
1. Let _roundResult_ be _roundRecord_.[[DurationRecord]].
1. Let _adjustResult_ be ? AdjustRoundedDurationDays(_roundResult_.[[Years]], _roundResult_.[[Months]], _roundResult_.[[Weeks]], _roundResult_.[[Days]], _roundResult_.[[Hours]], _roundResult_.[[Minutes]], _roundResult_.[[Seconds]], _roundResult_.[[Milliseconds]], _roundResult_.[[Microseconds]], _roundResult_.[[Nanoseconds]], _settings_.[[RoundingIncrement]], _settings_.[[SmallestUnit]], _settings_.[[RoundingMode]], _zonedDateTime_, _calendarRec_, _timeZoneRec_, _precalculatedPlainDateTime_).
- 1. Let _result_ be ? BalanceDateDurationRelative(_adjustResult_.[[Years]], _adjustResult_.[[Months]], _adjustResult_.[[Weeks]], _adjustResult_.[[Days]], _settings_.[[LargestUnit]], _plainRelativeTo_, _calendarRec_).
+ 1. Let _result_ be ? BalanceDateDurationRelative(_adjustResult_.[[Years]], _adjustResult_.[[Months]], _adjustResult_.[[Weeks]], _adjustResult_.[[Days]], _settings_.[[LargestUnit]], _settings_.[[SmallestUnit]], _plainRelativeTo_, _calendarRec_).
1. Return ! CreateTemporalDuration(_sign_ × _result_.[[Years]], _sign_ × _result_.[[Months]], _sign_ × _result_.[[Weeks]], _sign_ × _result_.[[Days]], _sign_ × _adjustResult_.[[Hours]], _sign_ × _adjustResult_.[[Minutes]], _sign_ × _adjustResult_.[[Seconds]], _sign_ × _adjustResult_.[[Milliseconds]], _sign_ × _adjustResult_.[[Microseconds]], _sign_ × _adjustResult_.[[Nanoseconds]]).
</emu-alg>
</emu-clause>Would appreciate another pair of eyes on the fix for the bug that Justin discovered (the smallestUnit = weeks special cases in BalanceDateDurationRelative) but on the whole I think we can go ahead with this. |
ptomato
changed the title
ctcpip
approved these changes
Nov 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(Note: stacked on top of #2571, will rebase as appropriate)This is the first half of #2612, split out for easier reviewing. These are the changes that prevent loops in the various duration operations, and an editorial change that removes a path that might have resulted in division by zero. The conversion to 96-bit integer math happens in part 2, which
will remain in #2612.is in #2727.