Skip to content

Commit c0f7349

Browse files
ptomato12wrigja
authored andcommitted
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
1 parent 58b5601 commit c0f7349

File tree

2 files changed

+45
-43
lines changed

2 files changed

+45
-43
lines changed

lib/ecmascript.ts

+45-37
Original file line numberDiff line numberDiff line change
@@ -3301,12 +3301,12 @@ function BalanceTime(
33013301

33023302
export function TotalDurationNanoseconds(
33033303
daysParam: number,
3304-
hoursParam: number,
3305-
minutesParam: number,
3306-
secondsParam: number,
3307-
millisecondsParam: number,
3308-
microsecondsParam: number,
3309-
nanosecondsParam: number,
3304+
hoursParam: number | JSBI,
3305+
minutesParam: number | JSBI,
3306+
secondsParam: number | JSBI,
3307+
millisecondsParam: number | JSBI,
3308+
microsecondsParam: number | JSBI,
3309+
nanosecondsParam: number | JSBI,
33103310
offsetShift: number
33113311
) {
33123312
const days: JSBI = JSBI.BigInt(daysParam);
@@ -3407,12 +3407,12 @@ function NanosecondsToDays(nanosecondsParam: JSBI, relativeTo: ReturnType<typeof
34073407

34083408
export function BalanceDuration(
34093409
daysParam: number,
3410-
hoursParam: number,
3411-
minutesParam: number,
3412-
secondsParam: number,
3413-
millisecondsParam: number,
3414-
microsecondsParam: number,
3415-
nanosecondsParam: number,
3410+
hoursParam: number | JSBI,
3411+
minutesParam: number | JSBI,
3412+
secondsParam: number | JSBI,
3413+
millisecondsParam: number | JSBI,
3414+
microsecondsParam: number | JSBI,
3415+
nanosecondsParam: number | JSBI,
34163416
largestUnit: Temporal.DateTimeUnit,
34173417
relativeTo: ReturnType<typeof ToRelativeTemporalObject> = undefined
34183418
) {
@@ -3436,12 +3436,12 @@ export function BalanceDuration(
34363436

34373437
export function BalancePossiblyInfiniteDuration(
34383438
daysParam: number,
3439-
hoursParam: number,
3440-
minutesParam: number,
3441-
secondsParam: number,
3442-
millisecondsParam: number,
3443-
microsecondsParam: number,
3444-
nanosecondsParam: number,
3439+
hoursParam: number | JSBI,
3440+
minutesParam: number | JSBI,
3441+
secondsParam: number | JSBI,
3442+
millisecondsParam: number | JSBI,
3443+
microsecondsParam: number | JSBI,
3444+
nanosecondsParam: number | JSBI,
34453445
largestUnit: Temporal.DateTimeUnit,
34463446
relativeTo: ReturnType<typeof ToRelativeTemporalObject> = undefined
34473447
) {
@@ -4865,12 +4865,12 @@ function AddDuration(
48654865
years = months = weeks = 0;
48664866
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = BalanceDuration(
48674867
d1 + d2,
4868-
h1 + h2,
4869-
min1 + min2,
4870-
s1 + s2,
4871-
ms1 + ms2,
4872-
µs1 + µs2,
4873-
ns1 + ns2,
4868+
JSBI.add(JSBI.BigInt(h1), JSBI.BigInt(h2)),
4869+
JSBI.add(JSBI.BigInt(min1), JSBI.BigInt(min2)),
4870+
JSBI.add(JSBI.BigInt(s1), JSBI.BigInt(s2)),
4871+
JSBI.add(JSBI.BigInt(ms1), JSBI.BigInt(ms2)),
4872+
JSBI.add(JSBI.BigInt(µs1), JSBI.BigInt(µs2)),
4873+
JSBI.add(JSBI.BigInt(ns1), JSBI.BigInt(ns2)),
48744874
largestUnit
48754875
));
48764876
} else if (IsTemporalDate(relativeTo)) {
@@ -4890,12 +4890,12 @@ function AddDuration(
48904890
// Signs of date part and time part may not agree; balance them together
48914891
({ days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds } = BalanceDuration(
48924892
days,
4893-
h1 + h2,
4894-
min1 + min2,
4895-
s1 + s2,
4896-
ms1 + ms2,
4897-
µs1 + µs2,
4898-
ns1 + ns2,
4893+
JSBI.add(JSBI.BigInt(h1), JSBI.BigInt(h2)),
4894+
JSBI.add(JSBI.BigInt(min1), JSBI.BigInt(min2)),
4895+
JSBI.add(JSBI.BigInt(s1), JSBI.BigInt(s2)),
4896+
JSBI.add(JSBI.BigInt(ms1), JSBI.BigInt(ms2)),
4897+
JSBI.add(JSBI.BigInt(µs1), JSBI.BigInt(µs2)),
4898+
JSBI.add(JSBI.BigInt(ns1), JSBI.BigInt(ns2)),
48994899
largestUnit
49004900
));
49014901
} else {
@@ -4964,7 +4964,15 @@ function AddDuration(
49644964
return { years, months, weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds };
49654965
}
49664966

4967-
function AddInstant(epochNanoseconds: JSBI, h: number, min: number, s: number, ms: number, µs: number, ns: number) {
4967+
function AddInstant(
4968+
epochNanoseconds: JSBI,
4969+
h: number | JSBI,
4970+
min: number | JSBI,
4971+
s: number | JSBI,
4972+
ms: number | JSBI,
4973+
µs: number | JSBI,
4974+
ns: number | JSBI
4975+
) {
49684976
let sum = ZERO;
49694977
sum = JSBI.add(sum, JSBI.BigInt(ns));
49704978
sum = JSBI.add(sum, JSBI.multiply(JSBI.BigInt(µs), THOUSAND));
@@ -5046,12 +5054,12 @@ export function AddZonedDateTime(
50465054
months: number,
50475055
weeks: number,
50485056
days: number,
5049-
h: number,
5050-
min: number,
5051-
s: number,
5052-
ms: number,
5053-
µs: number,
5054-
ns: number,
5057+
h: number | JSBI,
5058+
min: number | JSBI,
5059+
s: number | JSBI,
5060+
ms: number | JSBI,
5061+
µs: number | JSBI,
5062+
ns: number | JSBI,
50555063
options?: Temporal.ArithmeticOptions
50565064
) {
50575065
// If only time is to be added, then use Instant math. It's not OK to fall

test/expected-failures-todo-migrated-code.txt

-6
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,7 @@ built-ins/Temporal/Calendar/prototype/yearOfWeek/year-zero.js
148148
built-ins/Temporal/Duration/compare/order-of-operations.js
149149
built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
150150
built-ins/Temporal/Duration/compare/relativeto-propertybag-timezone-wrong-type.js
151-
built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-safe-integer.js
152-
built-ins/Temporal/Duration/prototype/add/nanoseconds-is-number-max-value-2.js
153151
built-ins/Temporal/Duration/prototype/add/order-of-operations.js
154-
built-ins/Temporal/Duration/prototype/add/precision-exact-mathematical-values.js
155152
built-ins/Temporal/Duration/prototype/add/relativeto-propertybag-calendar-fields-undefined.js
156153
built-ins/Temporal/Duration/prototype/add/relativeto-propertybag-calendar-wrong-type.js
157154
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
166163
built-ins/Temporal/Duration/prototype/round/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js
167164
built-ins/Temporal/Duration/prototype/round/relativeto-propertybag-timezone-wrong-type.js
168165
built-ins/Temporal/Duration/prototype/round/relativeto-string-datetime.js
169-
built-ins/Temporal/Duration/prototype/subtract/nanoseconds-is-number-max-safe-integer.js
170-
built-ins/Temporal/Duration/prototype/subtract/nanoseconds-is-number-max-value-2.js
171166
built-ins/Temporal/Duration/prototype/subtract/order-of-operations.js
172-
built-ins/Temporal/Duration/prototype/subtract/precision-exact-mathematical-values.js
173167
built-ins/Temporal/Duration/prototype/subtract/relativeto-propertybag-calendar-fields-undefined.js
174168
built-ins/Temporal/Duration/prototype/subtract/relativeto-propertybag-calendar-wrong-type.js
175169
built-ins/Temporal/Duration/prototype/subtract/relativeto-propertybag-timezone-instance-does-not-get-timeZone-property.js

0 commit comments

Comments
 (0)