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

Jump to bottom

fix: use single precision for dtoa with f32 #2908

Merged

CountBleck merged 10 commits into AssemblyScript:main from mattjohnsonpint:issue-2873

Feb 8, 2025

Contributor

mattjohnsonpint commented Feb 6, 2025 •

edited

Loading

Fixes #2873

Changes proposed in this pull request:

Adjusts dtoa and dtoa_buffered to accept either f32 or f64. Implementation details borrowed from Kotlin.

I've read the contributing guidelines
I've added my name and email to the NOTICE file


          fix: use single precision for dtoa with f32

c7cb718

mattjohnsonpint marked this pull request as draft

February 6, 2025 17:57

mattjohnsonpint added 2 commits

February 6, 2025 10:34


          Update existing dtoa tests

75b8ab8


          re-create other text fixtures

c50ecc1

mattjohnsonpint marked this pull request as ready for review

February 6, 2025 18:36

mattjohnsonpint marked this pull request as draft

February 6, 2025 18:40

This comment was marked as outdated.

Sign in to view


          re-create test fixture on Linux

65ebc46

mattjohnsonpint marked this pull request as ready for review

February 6, 2025 19:32

mattjohnsonpint mentioned this pull request

NaN vs -NaN (negative NaN) on macOS/arm64 #2909

Open

Contributor Author

mattjohnsonpint commented Feb 6, 2025 •

edited

Loading

Haha, I just read this line in the Kotlin sources for their original implementation:

// Based on the AssemblyScript implementation [https://github.com/AssemblyScript/assemblyscript/blob/1e0466ef94fa5cacd0984e4f31a0087de51538a8/std/assembly/util/number.ts]

And then it looks like just last week, @skuzmich gave it the 32-bit precision support, for the same reasons. Seems fair to add it back here too!

(Thanks @skuzmich!)

91929af

MaxGraey reviewed

View reviewed changes

std/assembly/util/number.ts Outdated Show resolved Hide resolved

MaxGraey reviewed

View reviewed changes

std/assembly/util/number.ts Outdated Show resolved Hide resolved

MaxGraey reviewed

View reviewed changes

std/assembly/util/number.ts Outdated Show resolved Hide resolved

MaxGraey reviewed

View reviewed changes

std/assembly/util/number.ts Outdated Show resolved Hide resolved

mattjohnsonpint added 2 commits

February 6, 2025 17:04

09013bc

bf9a07b

MaxGraey reviewed

View reviewed changes

tests/compiler/std/string.ts Show resolved Hide resolved

MaxGraey reviewed

View reviewed changes

tests/compiler/std/string.ts Show resolved Hide resolved

MaxGraey reviewed

View reviewed changes

tests/compiler/std/string.ts Show resolved Hide resolved

MaxGraey reviewed

View reviewed changes

tests/compiler/std/string.ts Show resolved Hide resolved

MaxGraey approved these changes

View reviewed changes

Member

MaxGraey left a comment

LGTM


          remove dtoa32/dtoa64

55db705

Member

CountBleck commented Feb 7, 2025

If @MaxGraey says it looks good, we should merge it.

CountBleck reviewed

View reviewed changes

tests/compiler/std/string.ts Outdated Show resolved Hide resolved

mattjohnsonpint added 2 commits

February 6, 2025 21:47


          handle integers as f64, error for non-numeric types

043c480


          simplify

9e8cba5

CountBleck requested a review from MaxGraey

February 7, 2025 23:50

MaxGraey approved these changes

View reviewed changes

CountBleck merged commit be1d94a into AssemblyScript:main

14 checks passed

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

Labels

None yet