Skip to content

Commit

Permalink
Avoid precision loss in AddDuration
Browse files Browse the repository at this point in the history
According to the spec, the addition in AddDuration must not be performed
in the floating-point domain. Anba wrote some tests that verify this.

For days, d1 + d2 practically speaking is still OK because the result is
directly stuffed into a float64-representable integer anyway, so keep that
the same for simplicity. Perform the other additions in the BigInt domain
and pass BigInts to BalanceDuration.

UPSTREAM_COMMIT=9a0565a8cba336a6dabbf10cc58ccbf665bfe023
  • Loading branch information
ptomato authored and 12wrigja committed Feb 22, 2023
1 parent 58b5601 commit c0f7349
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 43 deletions.
82 changes: 45 additions & 37 deletions lib/ecmascript.ts
Expand Up @@ -3301,12 +3301,12 @@ function BalanceTime(

export function TotalDurationNanoseconds(
daysParam: number,
hoursParam: number,
minutesParam: number,
secondsParam: number,
millisecondsParam: number,
microsecondsParam: number,
nanosecondsParam: number,
hoursParam: number | JSBI,
minutesParam: number | JSBI,
secondsParam: number | JSBI,
millisecondsParam: number | JSBI,
microsecondsParam: number | JSBI,
nanosecondsParam: number | JSBI,
offsetShift: number
) {
const days: JSBI = JSBI.BigInt(daysParam);
Expand Down Expand Up @@ -3407,12 +3407,12 @@ function NanosecondsToDays(nanosecondsParam: JSBI, relativeTo: ReturnType<typeof

export function BalanceDuration(
daysParam: number,
hoursParam: number,
minutesParam: number,
secondsParam: number,
millisecondsParam: number,
microsecondsParam: number,
nanosecondsParam: number,
hoursParam: number | JSBI,
minutesParam: number | JSBI,
secondsParam: number | JSBI,
millisecondsParam: number | JSBI,
microsecondsParam: number | JSBI,
nanosecondsParam: number | JSBI,
largestUnit: Temporal.DateTimeUnit,
relativeTo: ReturnType<typeof ToRelativeTemporalObject> = undefined
) {
Expand All @@ -3436,12 +3436,12 @@ export function BalanceDuration(

export function BalancePossiblyInfiniteDuration(
daysParam: number,
hoursParam: number,
minutesParam: number,
secondsParam: number,
millisecondsParam: number,
microsecondsParam: number,
nanosecondsParam: number,
hoursParam: number | JSBI,
minutesParam: number | JSBI,
secondsParam: number | JSBI,
millisecondsParam: number | JSBI,
microsecondsParam: number | JSBI,
nanosecondsParam: number | JSBI,
largestUnit: Temporal.DateTimeUnit,
relativeTo: ReturnType<typeof ToRelativeTemporalObject> = undefined
) {
Expand Down Expand Up @@ -4865,12 +4865,12 @@ function AddDuration(
years = months = weeks = 0;
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = BalanceDuration(
d1 + d2,
h1 + h2,
min1 + min2,
s1 + s2,
ms1 + ms2,
µs1 + µs2,
ns1 + ns2,
JSBI.add(JSBI.BigInt(h1), JSBI.BigInt(h2)),
JSBI.add(JSBI.BigInt(min1), JSBI.BigInt(min2)),
JSBI.add(JSBI.BigInt(s1), JSBI.BigInt(s2)),
JSBI.add(JSBI.BigInt(ms1), JSBI.BigInt(ms2)),
JSBI.add(JSBI.BigInt(µs1), JSBI.BigInt(µs2)),
JSBI.add(JSBI.BigInt(ns1), JSBI.BigInt(ns2)),
largestUnit
));
} else if (IsTemporalDate(relativeTo)) {
Expand All @@ -4890,12 +4890,12 @@ function AddDuration(
// Signs of date part and time part may not agree; balance them together
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = BalanceDuration(
days,
h1 + h2,
min1 + min2,
s1 + s2,
ms1 + ms2,
µs1 + µs2,
ns1 + ns2,
JSBI.add(JSBI.BigInt(h1), JSBI.BigInt(h2)),
JSBI.add(JSBI.BigInt(min1), JSBI.BigInt(min2)),
JSBI.add(JSBI.BigInt(s1), JSBI.BigInt(s2)),
JSBI.add(JSBI.BigInt(ms1), JSBI.BigInt(ms2)),
JSBI.add(JSBI.BigInt(µs1), JSBI.BigInt(µs2)),
JSBI.add(JSBI.BigInt(ns1), JSBI.BigInt(ns2)),
largestUnit
));
} else {
Expand Down Expand Up @@ -4964,7 +4964,15 @@ function AddDuration(
return { years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds };
}

function AddInstant(epochNanoseconds: JSBI, h: number, min: number, s: number, ms: number, µs: number, ns: number) {
function AddInstant(
epochNanoseconds: JSBI,
h: number | JSBI,
min: number | JSBI,
s: number | JSBI,
ms: number | JSBI,
µs: number | JSBI,
ns: number | JSBI
) {
let sum = ZERO;
sum = JSBI.add(sum, JSBI.BigInt(ns));
sum = JSBI.add(sum, JSBI.multiply(JSBI.BigInt(µs), THOUSAND));
Expand Down Expand Up @@ -5046,12 +5054,12 @@ export function AddZonedDateTime(
months: number,
weeks: number,
days: number,
h: number,
min: number,
s: number,
ms: number,
µs: number,
ns: number,
h: number | JSBI,
min: number | JSBI,
s: number | JSBI,
ms: number | JSBI,
µs: number | JSBI,
ns: number | JSBI,
options?: Temporal.ArithmeticOptions
) {
// If only time is to be added, then use Instant math. It's not OK to fall
Expand Down
6 changes: 0 additions & 6 deletions test/expected-failures-todo-migrated-code.txt
Expand Up @@ -148,10 +148,7 @@ built-ins/Temporal/Calendar/prototype/yearOfWeek/year-zero.js
built-ins/Temporal/Duration/compare/order-of-operations.js
built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-wrong-type.js
built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-safe-integer.js
built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-value-2.js
built-ins/Temporal/Duration/prototype/add/order-of-operations.js
built-ins/Temporal/Duration/prototype/add/precision-exact-mathematical-values.js
built-ins/Temporal/Duration/prototype/add/relativeto-propertybag-calendar-fields-undefined.js
built-ins/Temporal/Duration/prototype/add/relativeto-propertybag-calendar-wrong-type.js
built-ins/Temporal/Duration/prototype/add/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
Expand All @@ -166,10 +163,7 @@ built-ins/Temporal/Duration/prototype/round/relativeto-propertybag-calendar-wron
built-ins/Temporal/Duration/prototype/round/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
built-ins/Temporal/Duration/prototype/round/relativeto-propertybag-timezone-wrong-type.js
built-ins/Temporal/Duration/prototype/round/relativeto-string-datetime.js
built-ins/Temporal/Duration/prototype/subtract/nanoseconds-is-number-max-safe-integer.js
built-ins/Temporal/Duration/prototype/subtract/nanoseconds-is-number-max-value-2.js
built-ins/Temporal/Duration/prototype/subtract/order-of-operations.js
built-ins/Temporal/Duration/prototype/subtract/precision-exact-mathematical-values.js
built-ins/Temporal/Duration/prototype/subtract/relativeto-propertybag-calendar-fields-undefined.js
built-ins/Temporal/Duration/prototype/subtract/relativeto-propertybag-calendar-wrong-type.js
built-ins/Temporal/Duration/prototype/subtract/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
Expand Down

0 comments on commit c0f7349

Please sign in to comment.