Skip to content

Conversation

past-due
Copy link
Contributor

@past-due past-due commented Aug 25, 2025

Port bellard/quickjs dtoa library commits

added new dtoa library to print and parse float64 numbers. It is necessary to fix corner cases (e.g. radix != 10) and to have correct behavior regardless of the libc implementation.

Port of:
bellard/quickjs@9936606
bellard/quickjs@dbbca3d

Includes alternative fix for bellard/quickjs@638ec8c from 0191aea

Remaining TODO items (would appreciate a look @saghul @bnoordhuis):

  • Amalgamation fails due to duplicate symbols (not sure how you'd prefer to fix this?)
  • Codegen update This was actually a side effect of the root cause of the subsequent issue
  • New test error 🤔 (see below - would appreciate a look at this one)
Result: 65/78606 errors, 3796 excluded, 5454 skipped, 2 new
test262/test/built-ins/TypedArrayConstructors/internals/Set/resized-out-of-bounds-to-in-bounds-index.js:18: unexpected error: Test262Error: Expected SameValue(«0», «100») to be true
test262/test/built-ins/TypedArrayConstructors/internals/Set/resized-out-of-bounds-to-in-bounds-index.js:18: strict mode: unexpected error: Test262Error: Expected SameValue(«0», «100») to be true

@bnoordhuis
Copy link
Contributor

I have a fix for the first item in bnoordhuis/quickjs@2c073d7 that you can cherry-pick.

@saghul
Copy link
Contributor

saghul commented Sep 1, 2025

Can you run make gen? Looks like we're one bug away!

@past-due past-due force-pushed the bellard_dtoa_port branch 2 times, most recently from 57ad718 to 45997cf Compare September 18, 2025 19:27
@past-due
Copy link
Contributor Author

past-due commented Sep 18, 2025

It turns out that I missed that the bellard/quickjs js_atof still used JS_NewFloat64 where quickjs-ng requires js_number() (since #319). Fixing that resolved the new test error (and also eliminated the codegen changes).

Something to keep a look out for in the future - any ports from bellard/quickjs that use JS_NewFloat64 should be double-checked.

@past-due past-due marked this pull request as ready for review September 18, 2025 19:52
Copy link
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Great work!

@past-due
Copy link
Contributor Author

past-due commented Sep 25, 2025

@bnoordhuis Anything else you're looking for to merge? (CI tests check out, and some local testing checks out fine as well.)

EDIT: Rebased on latest master

past-due and others added 6 commits September 25, 2025 14:37
added new dtoa library to print and parse float64 numbers. It is necessary to fix corner cases (e.g. radix != 10) and to have correct behavior regardless of the libc implementation.

Port of:
bellard/quickjs@9936606
bellard/quickjs@dbbca3d

Includes alternative fix for bellard/quickjs@638ec8c from 0191aea

Update CMakeLists.txt and meson.build

Co-Authored-By: Fabrice Bellard <[email protected]>
Re-apply quickjs-ng#319, use of js_number()
@saghul saghul merged commit 844bb02 into quickjs-ng:master Sep 25, 2025
127 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants