Skip to content

feat(ecmascript): Make Math.sumPrecise actually precise #915

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aapoalas merged 1 commit into from
Dec 13, 2025
Merged

feat(ecmascript): Make Math.sumPrecise actually precise #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ sonic-rs = "0.3.17"
unicode-normalization = "0.1.24"
usdt = { git = "https://github.com/aapoalas/usdt.git", branch = "nova-aarch64-branch" }
wtf8 = "0.1"
xsum = "0.1.6"

[workspace.metadata.dylint]
libraries = [{ path = "nova_lint" }]
Expand Down
4 changes: 1 addition & 3 deletions nova_vm/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ sonic-rs = { workspace = true, optional = true }
unicode-normalization = { workspace = true }
usdt = { workspace = true }
wtf8 = { workspace = true }
xsum = { workspace = true }

[features]
default = [
Expand Down Expand Up @@ -80,15 +81,12 @@ annex-b-regexp = ["regexp"]
# Enables all currently supported proposals
proposals = [
"proposal-float16array",
"proposal-math-sum",
"proposal-math-clamp",
"proposal-is-error",
"proposal-atomics-microwait",
]
# Enables the [Float16Array proposal](https://tc39.es/proposal-float16array/)
proposal-float16array = ["array-buffer"]
# Enables the [Math.sumPrecise proposal](https://tc39.es/proposal-math-sum/)
proposal-math-sum = ["math"]
# Enables the [Math.clamp proposal](https://tc39.es/proposal-math-clamp/)
proposal-math-clamp = ["math"]
# Enables the [Error.isError proposal](https://tc39.es/proposal-is-error/)
Expand Down
2 changes: 1 addition & 1 deletion nova_vm/src/builtin_strings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ String Iterator
#[cfg(feature = "array-buffer")]subarray
#[cfg(feature = "annex-b-string")]substr
substring
#[cfg(feature = "proposal-math-sum")]sumPrecise
sumPrecise
#[cfg(feature = "annex-b-string")]sup
symbol
Symbol
Expand Down
99 changes: 47 additions & 52 deletions nova_vm/src/ecmascript/builtins/numbers_and_dates/math_object.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,9 +314,7 @@ impl Builtin for MathObjectF16round {
const BEHAVIOUR: Behaviour = Behaviour::Regular(MathObject::f16round);
}

#[cfg(feature = "proposal-math-sum")]
struct MathObjectSumPrecise;
#[cfg(feature = "proposal-math-sum")]
impl Builtin for MathObjectSumPrecise {
const NAME: String<'static> = BUILTIN_STRING_MEMORY.sumPrecise;

Expand Down Expand Up @@ -1697,7 +1695,6 @@ impl MathObject {
/// > Fast Robust Geometric Predicates by Jonathan Richard Shewchuk. A more
/// > recent algorithm is given in "Fast exact summation using small and large superaccumulators",
/// > code for which is available at https://gitlab.com/radfordneal/xsum.
#[cfg(feature = "proposal-math-sum")]
fn sum_precise<'gc>(
agent: &mut Agent,
_this_value: Value,
Expand All @@ -1709,11 +1706,14 @@ impl MathObject {
operations_on_iterator_objects::{
IteratorRecord, get_iterator, iterator_close_with_error, iterator_step_value,
},
operations_on_objects::throw_not_callable,
operations_on_objects::{throw_not_callable, try_length_of_array_like},
testing_and_comparison::require_object_coercible,
},
execution::agent::ExceptionType,
types::Object,
};
use std::ops::ControlFlow;
use xsum::{Xsum, XsumAuto, XsumLarge, XsumSmall, XsumVariant, constants::XSUM_THRESHOLD};

let items = arguments.get(0).bind(gc.nogc());

Expand All @@ -1722,6 +1722,18 @@ impl MathObject {
.unbind()?
.bind(gc.nogc());

// If `items` is array-like, we get the length so we can size the xsum
// accumulator properly. If this length ends up being wrong, the
// algorithm's correctness won't be affected, but performance might be.
let len_estimate: Option<usize> = if let Ok(items) = Object::try_from(items) {
match try_length_of_array_like(agent, items, gc.nogc()) {
ControlFlow::Continue(len) => Some(len as usize),
ControlFlow::Break(_) => None,
}
} else {
None
};

// 2. Let iteratorRecord be ? GetIterator(items, sync).
let Some(IteratorRecord {
iterator,
Expand All @@ -1738,11 +1750,19 @@ impl MathObject {
let next_method = next_method.scope(agent, gc.nogc());

// 3. Let state be minus-zero.
let mut state = -0.0f64;
// 4. Let sum be 0.
let mut sum = 0.0f64;
// `sum` in the spec is a mathematical number with infinite precision,
// and this algorithm relies on that, so we can't implement it with f64
// addition. Instead we use xsum, which also handles `state`. We use the
// length estimate to choose the variant to use for better performance.
let mut sum = match len_estimate {
Some(XSUM_THRESHOLD..) => XsumVariant::Large(XsumLarge::new()),
Some(_) => XsumVariant::Small(XsumSmall::new()),
None => XsumVariant::Auto(XsumAuto::new()),
};

// 5. Let count be 0.
let mut count = 0;
let mut count: usize = 0;

// 6. Let next be not-started.
// 7. Repeat, while next is not done,
Expand All @@ -1763,7 +1783,7 @@ impl MathObject {
count += 1;
// ii. If count ≥ 2**53, then
// iii. NOTE: The above case is not expected to be reached in practice and is included only so that implementations may rely on inputs being "reasonably sized" without violating this specification.
if count >= 2i64.pow(53) {
if count >= 1 << 53 {
// 1. Let error be ThrowCompletion(a newly created RangeError object).
let error = agent.throw_exception_with_static_message(
ExceptionType::RangeError,
Expand All @@ -1781,43 +1801,26 @@ impl MathObject {

// v. Let n be next.
if let Ok(n) = Number::try_from(next) {
// vi. If state is not not-a-number, then
if !state.is_nan() {
if n.is_nan(agent) {
// 1. If n is NaN, then
// a. Set state to not-a-number.
state = f64::NAN;
} else if n.is_pos_infinity(agent) {
// 2. Else if n is +∞𝔽, then
// a. If state is minus-infinity, set state to not-a-number.
// b. Else, set state to plus-infinity.
state = if state == f64::NEG_INFINITY {
f64::NAN
} else {
f64::INFINITY
};
} else if n.is_neg_infinity(agent) {
// 3. Else if n is -∞𝔽, then
// a. If state is plus-infinity, set state to not-a-number.
// b. Else, set state to minus-infinity.
state = if state == f64::INFINITY {
f64::NAN
} else {
f64::NEG_INFINITY
};
} else if !n.is_neg_zero(agent) && (state == -0.0 || state.is_finite()) {
// 4. Else if n is not -0𝔽 and state is either minus-zero or finite, then
// a. Set state to finite.
state = 0.0;
// b. Set sum to sum + ℝ(n).
sum += n.into_f64(agent);
}
}
// iv. If state is not not-a-number, then
// 1. If n is NaN, then
// a. Set state to not-a-number.
// 2. Else if n is +∞𝔽, then
// a. If state is minus-infinity, set state to not-a-number.
// b. Else, set state to plus-infinity.
// 3. Else if n is -∞𝔽, then
// a. If state is plus-infinity, set state to not-a-number.
// b. Else, set state to minus-infinity.
// 4. Else if n is not -0𝔽 and state is either minus-zero or finite, then
// a. Set state to finite.
// b. Set sum to sum + ℝ(n).

// xsum handles all of this
sum.add(n.into_f64(agent));
} else {
// iv. If next is not a Number, then
// 1. Let error be ThrowCompletion(a newly created TypeError object).
let error = agent.throw_exception_with_static_message(
ExceptionType::RangeError,
ExceptionType::TypeError,
"Iterator may only contain numbers",
gc.nogc(),
);
Expand All @@ -1835,12 +1838,8 @@ impl MathObject {
// 9. If state is plus-infinity, return +∞𝔽.
// 10. If state is minus-infinity, return -∞𝔽.
// 11. If state is minus-zero, return -0𝔽.
if state.is_nan() || state.is_infinite() || state == -0.0 {
Ok(Value::from_f64(agent, state, gc.into_nogc()))
} else {
// 12. Return 𝔽(sum).
Ok(Value::from_f64(agent, sum, gc.into_nogc()))
}
// 12. Return 𝔽(sum).
Ok(Value::from_f64(agent, sum.sum(), gc.into_nogc()))
}

/// ### [1 Math.clamp ( value, min, max )](https://tc39.es/proposal-math-clamp/#sec-math.clamp)
Expand Down Expand Up @@ -1938,13 +1937,10 @@ impl MathObject {
let object_prototype = intrinsics.object_prototype();
let this = intrinsics.math();

let mut property_capacity = 44;
let mut property_capacity = 45;
if cfg!(feature = "proposal-float16array") {
property_capacity += 1;
}
if cfg!(feature = "proposal-math-sum") {
property_capacity += 1;
}
if cfg!(feature = "proposal-math-clamp") {
property_capacity += 1;
}
Expand Down Expand Up @@ -2093,7 +2089,6 @@ impl MathObject {
#[cfg(feature = "proposal-float16array")]
let builder = builder.with_builtin_function_property::<MathObjectF16round>();

#[cfg(feature = "proposal-math-sum")]
let builder = builder.with_builtin_function_property::<MathObjectSumPrecise>();

#[cfg(feature = "proposal-math-clamp")]
Expand Down
10 changes: 0 additions & 10 deletions tests/expectations.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -783,16 +783,6 @@
"built-ins/Math/f16round/value-conversion.js": "FAIL",
"built-ins/Math/pow/applying-the-exp-operator_A6.js": "FAIL",
"built-ins/Math/round/S15.8.2.15_A6.js": "FAIL",
"built-ins/Math/sumPrecise/length.js": "FAIL",
"built-ins/Math/sumPrecise/name.js": "FAIL",
"built-ins/Math/sumPrecise/not-a-constructor.js": "FAIL",
"built-ins/Math/sumPrecise/prop-desc.js": "FAIL",
"built-ins/Math/sumPrecise/sum-is-NaN.js": "FAIL",
"built-ins/Math/sumPrecise/sum-is-infinite.js": "FAIL",
"built-ins/Math/sumPrecise/sum-is-minus-zero.js": "FAIL",
"built-ins/Math/sumPrecise/sum.js": "FAIL",
"built-ins/Math/sumPrecise/takes-iterable.js": "FAIL",
"built-ins/Math/sumPrecise/throws-on-non-number.js": "FAIL",
"built-ins/NativeErrors/EvalError/proto-from-ctor-realm.js": "FAIL",
"built-ins/NativeErrors/RangeError/proto-from-ctor-realm.js": "FAIL",
"built-ins/NativeErrors/ReferenceError/proto-from-ctor-realm.js": "FAIL",
Expand Down