Skip to content

fix(number): emit round-trip digits in Number.prototype.toString(radix) - #5645

Open
rishabhknowss wants to merge 1 commit into
boa-dev:mainfrom
rishabhknowss:fix/number-radix-round-trip
Open

rishabhknowss wants to merge 1 commit into
boa-dev:mainfrom
rishabhknowss:fix/number-radix-round-trip

Conversation

@rishabhknowss

Copy link
Copy Markdown

This Pull Request fixes/closes #5636.

the radix path was a port of v8's DoubleToRadixCString, which works in floating point, so for radices that aren't powers of two the digits often don't read back as the input. on main, (0.6206327010376135).toString(36) gives 0.mcc8m4rge and (1e21).toString(36) gives 5v1j4f4ds7c000, neither of which round-trips.

it changes the following:

  • to_js_string_radix now uses burger & dybvig's free-format algorithm on BigUint (num-bigint is already a dependency). it produces the shortest digits that round back to the input, for any radix (Number::toString step 5).
  • removes next_after and BUF_SIZE, which only the old path used.
  • adds to_string_non_power_of_two_radix with the examples from the issue plus a few edge cases (subnormal, MAX_VALUE, negative, exact thirds). it fails on main.

i also checked it against 20,303 doubles × 9 radices with an exact checker: parse the output as a rational, round to the nearest double, and confirm no shorter digit string rounds back. all 182,727 outputs pass. the same checker flags ~71k outputs from node. radix 2 and 16 output is byte-identical to main.

testing:

  • cargo test -p boa_engine: all pass
  • cargo clippy -p boa_engine --all-targets -- -D warnings, with --all-features and --no-default-features: clean
  • cargo fmt --all --check: clean
  • cargo make run-ci (the pre-push hook): passes
  • test262 built-ins/Number/prototype/toString: 90/90. built-ins/Number: 339/340 on both main and this branch (same pre-existing failure)

two things worth a look:

  • issue_2717 expected 0.8aoosla2phj for (0.23046743672210102).toString(36). h, i and j all round back to the input and the spec leaves that choice open, so it now gives 0.8aoosla2phh, the closest one.
  • it's slower: 200k conversions take ~250–830ms vs ~110–220ms before, depending on radix. if that matters i can keep the old path for power-of-two radices, since it was already correct there.

AI assistance: Claude Code

The radix conversion was ported from V8 and computes digits in floating
point, so for radices that are not powers of two the output often does
not read back as the input. Generate the shortest digits with exact
integer arithmetic (Burger and Dybvig's free-format algorithm) instead.

Closes boa-dev#5636

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rishabhknowss
rishabhknowss requested a review from a team as a code owner November 7, 2026 12:11
@github-actions github-actions Bot added the Waiting On Review Waiting on reviews from the maintainers label Nov 7, 2026
@github-actions github-actions Bot added this to the v0.23 milestone Nov 7, 2026
@github-actions github-actions Bot added C-Tests Issues and PRs related to the tests. C-Builtins PRs and Issues related to builtins/intrinsics labels Nov 7, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-Builtins PRs and Issues related to builtins/intrinsics C-Tests Issues and PRs related to the tests. Waiting On Review Waiting on reviews from the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Number.prototype.toString(radix) output does not round-trip for radices that are not powers of two

1 participant