From c0f7349a327b68543797d38e045b1fd8c1e0949b Mon Sep 17 00:00:00 2001 From: Philip Chimento Date: Tue, 27 Sep 2022 16:04:44 -0700 Subject: [PATCH] Avoid precision loss in AddDuration 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 --- lib/ecmascript.ts | 82 ++++++++++--------- test/expected-failures-todo-migrated-code.txt | 6 -- 2 files changed, 45 insertions(+), 43 deletions(-) diff --git a/lib/ecmascript.ts b/lib/ecmascript.ts index f538f0ad..aa485fca 100644 --- a/lib/ecmascript.ts +++ b/lib/ecmascript.ts @@ -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); @@ -3407,12 +3407,12 @@ function NanosecondsToDays(nanosecondsParam: JSBI, relativeTo: ReturnType = undefined ) { @@ -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 = undefined ) { @@ -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)) { @@ -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 { @@ -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)); @@ -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 diff --git a/test/expected-failures-todo-migrated-code.txt b/test/expected-failures-todo-migrated-code.txt index 541ce96a..064cec81 100644 --- a/test/expected-failures-todo-migrated-code.txt +++ b/test/expected-failures-todo-migrated-code.txt @@ -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 @@ -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