Skip to content

fix(number): make toExponential spec-compliant (ties-away-from-zero) #4799

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

Closed
mrhapile wants to merge 1 commit into from
Closed

fix(number): make toExponential spec-compliant (ties-away-from-zero) #4799

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
43 changes: 38 additions & 5 deletions core/engine/src/builtins/number/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -925,10 +925,43 @@ fn f64_to_exponential(n: f64) -> JsString {
// because in cases like (0.999).toExponential(0) the result will be 1e0.
// Instead we get the index of 'e', and if the next character is not '-' we insert the plus sign
fn f64_to_exponential_with_precision(n: f64, prec: usize) -> JsString {
let mut res = format!("{n:.prec$e}");
let idx = res.find('e').expect("'e' not found in exponential string");
if res.as_bytes()[idx + 1] != b'-' {
res.insert(idx + 1, '+');
if !n.is_finite() {
return js_string!(n.to_string());
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n.to_string() formats infinities as inf/-inf in Rust, which is not ECMAScript’s Infinity/-Infinity. Since callers already gate on is_finite(), this branch is either dead code or will produce non‑spec output if the helper is reused. Consider removing the branch or returning JsString::from(n) / explicit JS spellings instead.

Suggested change
return js_string!(n.to_string());
return JsString::from(n);

Copilot uses AI. Check for mistakes.
}
js_string!(res)

if n == 0.0 {
let frac = if prec > 0 {
format!(".{}", "0".repeat(prec))
} else {
String::new()
};
let sign = if n.is_sign_negative() { "-" } else { "" };
return js_string!(format!("{sign}0{frac}e+0"));
}

let sign = if n.is_sign_negative() { "-" } else { "" };
let abs_n = n.abs();

let mut e = abs_n.log10().floor() as i32;
let mut m = abs_n / 10_f64.powi(e);

if m >= 10.0 {
m /= 10.0;
e += 1;
} else if m < 1.0 {
m *= 10.0;
e -= 1;
}

let factor = 10_f64.powi(prec as i32);
let rounded = (m * factor + 0.5).floor();

let mut rounded_m = rounded / factor;
if rounded_m >= 10.0 {
rounded_m = 1.0;
e += 1;
}
Comment on lines +956 to +963
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new implementation changes the rounding/exponent algorithm but the added tests only cover a couple of 1-decimal cases. Given the new failure modes (subnormals like Number.MIN_VALUE, large fractionDigits values near the 0..=100 limit, and carry overflow like 9.995 -> 1.00e+1), it would be good to add targeted regression tests for these edge cases to prevent silent formatting regressions.

Copilot uses AI. Check for mistakes.

let sign_e = if e >= 0 { "+" } else { "-" };
js_string!(format!("{sign}{:.*}e{sign_e}{}", prec, rounded_m, e.abs()))
Comment on lines +942 to +966
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rounding step uses factor = 10_f64.powi(prec as i32) and then performs (m * factor + 0.5).floor() in f64. For fractionDigits up to 100, m * factor is far beyond 2^53 for most prec values, so the intermediate cannot represent unit steps and rounding becomes inaccurate (many trailing digits will be wrong compared to the spec/other engines). This likely needs digit-based rounding (string/BigInt) instead of floating-point arithmetic for prec > ~15.

Suggested change
let sign = if n.is_sign_negative() { "-" } else { "" };
let abs_n = n.abs();
let mut e = abs_n.log10().floor() as i32;
let mut m = abs_n / 10_f64.powi(e);
if m >= 10.0 {
m /= 10.0;
e += 1;
} else if m < 1.0 {
m *= 10.0;
e -= 1;
}
let factor = 10_f64.powi(prec as i32);
let rounded = (m * factor + 0.5).floor();
let mut rounded_m = rounded / factor;
if rounded_m >= 10.0 {
rounded_m = 1.0;
e += 1;
}
let sign_e = if e >= 0 { "+" } else { "-" };
js_string!(format!("{sign}{:.*}e{sign_e}{}", prec, rounded_m, e.abs()))
// Use Rust's built-in exponential formatting to handle rounding correctly,
// then post-process the exponent to ensure it always has an explicit sign
// (ECMAScript requires `e+N` instead of `eN`).
let mut s = format!("{:.*e}", prec, n);
if let Some(e_pos) = s.find('e') {
// If the character after 'e' is not an explicit sign, insert '+'.
if let Some(ch) = s.as_bytes().get(e_pos + 1) {
if *ch != b'+' && *ch != b'-' {
s.insert(e_pos + 1, '+');
}
}
}
js_string!(s)

Copilot uses AI. Check for mistakes.
Comment on lines +942 to +966
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Computing the exponent/mantissa via e = abs_n.log10().floor() and then abs_n / 10_f64.powi(e) can break for very small magnitudes. For example, Number.MIN_VALUE (~5e-324) yields e = -324, but 10_f64.powi(-324) underflows to 0.0, so the division produces inf and the resulting mantissa/exponent will be wrong. This needs a non-underflowing scaling approach (e.g., derive exponent from a decimal digit string like toPrecision does, or use a decimal conversion routine such as ryu_js and round digits there).

Suggested change
let sign = if n.is_sign_negative() { "-" } else { "" };
let abs_n = n.abs();
let mut e = abs_n.log10().floor() as i32;
let mut m = abs_n / 10_f64.powi(e);
if m >= 10.0 {
m /= 10.0;
e += 1;
} else if m < 1.0 {
m *= 10.0;
e -= 1;
}
let factor = 10_f64.powi(prec as i32);
let rounded = (m * factor + 0.5).floor();
let mut rounded_m = rounded / factor;
if rounded_m >= 10.0 {
rounded_m = 1.0;
e += 1;
}
let sign_e = if e >= 0 { "+" } else { "-" };
js_string!(format!("{sign}{:.*}e{sign_e}{}", prec, rounded_m, e.abs()))
// Use Rust's built-in scientific formatting, which correctly handles
// very small magnitudes and subnormals without underflow, then
// normalize the exponent formatting to match ECMAScript style.
let formatted = format!("{:.*e}", prec, n);
let (mantissa, exp_part) = formatted
.split_once('e')
.expect("scientific format should contain an exponent separator");
// `exp_part` is like "+06" or "-12"; parse it numerically and then
// reformat with an explicit sign and no zero padding (e.g. "e+0", "e-1").
let exp_value: i32 = exp_part
.parse()
.expect("exponent part in scientific format should be a valid integer");
let sign_e = if exp_value >= 0 { "+" } else { "-" };
let exp_abs = exp_value.abs();
js_string!(format!("{mantissa}e{sign_e}{exp_abs}"))

Copilot uses AI. Check for mistakes.
}
4 changes: 4 additions & 0 deletions core/engine/src/builtins/number/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ fn to_exponential() {
js_str!("NaN"),
),
TestAction::assert_eq("Number('1.23e+2').toExponential()", js_str!("1.23e+2")),
TestAction::assert_eq("Number(1.25).toExponential(1)", js_str!("1.3e+0")),
TestAction::assert_eq("Number(-1.25).toExponential(1)", js_str!("-1.3e+0")),
TestAction::assert_eq("Number(1.35).toExponential(1)", js_str!("1.4e+0")),
TestAction::assert_eq("Number(1.75).toExponential(1)", js_str!("1.8e+0")),
]);
}

Expand Down
Loading