Skip to content

Conversation

jakebailey
Copy link
Member

See golang/go#75834

I rethought this again and came up with a better implementation that feels more like the spec and less like a hack.

It is actually faster in Go 1.25, but tip is slower because float to int conversions now have branching, I guess.

goos: linux
goarch: amd64
pkg: github.com/microsoft/typescript-go/internal/jsnum
cpu: Intel(R) Core(TM) i9-10900K CPU @ 3.70GHz
                                                      │   old.txt   │               new.txt               │            new-gotip.txt            │
                                                      │   sec/op    │   sec/op     vs base                │   sec/op     vs base                │
ToInt32/0.0_(0)-20                                      2.691n ± 2%   1.750n ± 0%  -34.96% (p=0.000 n=10)   2.916n ± 1%   +8.38% (p=0.000 n=10)
ToInt32/NaN_(NaN)-20                                    2.595n ± 1%   1.961n ± 1%  -24.45% (p=0.000 n=10)   3.001n ± 1%  +15.63% (p=0.000 n=10)
ToInt32/+Inf_(+Inf)-20                                  2.635n ± 2%   1.739n ± 1%  -34.02% (p=0.000 n=10)   2.940n ± 1%  +11.53% (p=0.000 n=10)
ToInt32/-Inf_(-Inf)-20                                  2.625n ± 1%   1.750n ± 1%  -33.37% (p=0.000 n=10)   2.938n ± 1%  +11.88% (p=0.000 n=10)
ToInt32/MaxInt32+1_(2.147483648e+09)-20                 7.213n ± 1%   7.234n ± 0%        ~ (p=0.481 n=10)   8.126n ± 1%  +12.65% (p=0.000 n=10)
ToInt32/MinInt32-1_(-2.147483649e+09)-20                7.480n ± 1%   7.426n ± 0%   -0.72% (p=0.004 n=10)   8.137n ± 1%   +8.79% (p=0.000 n=10)
ToInt32/MAX_SAFE_INTEGER_(9.007199254740991e+15)-20     187.2n ± 1%   187.4n ± 0%        ~ (p=0.926 n=10)   180.8n ± 1%   -3.47% (p=0.000 n=10)
ToInt32/MAX_SAFE_INTEGER-1_(9.00719925474099e+15)-20    187.3n ± 0%   186.6n ± 0%        ~ (p=0.109 n=10)   180.6n ± 1%   -3.63% (p=0.000 n=10)
ToInt32/MAX_SAFE_INTEGER+1_(9.007199254740992e+15)-20   13.05n ± 1%   12.91n ± 1%   -1.11% (p=0.001 n=10)   14.10n ± 1%   +7.97% (p=0.000 n=10)
ToInt32/0xDEADBEEF_(3.735928559e+09)-20                 7.247n ± 1%   7.234n ± 1%        ~ (p=0.739 n=10)   8.110n ± 1%  +11.91% (p=0.000 n=10)
ToInt32/TypeFlagsNarrowable_(5.36624127e+08)-20         2.659n ± 4%   1.738n ± 1%  -34.62% (p=0.000 n=10)   2.947n ± 0%  +10.83% (p=0.000 n=10)
geomean                                                 8.750n        7.310n       -16.45%                  9.470n        +8.23%

@Copilot Copilot AI review requested due to automatic review settings October 9, 2025 23:21
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes number conversion in the TypeScript-Go compiler to handle changes in Go 1.26 related to float-to-int conversions. The implementation refactors the toInt32 and toUint32 functions to follow the ECMAScript specification more closely while maintaining performance.

Key changes:

  • Restructures toInt32 and toUint32 function implementations to handle Go 1.26 conversion behavior
  • Updates test cases to use pre-computed values instead of computing them during test execution
  • Implements spec-compliant truncation and modulo operations for number conversion

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
internal/jsnum/jsnum.go Refactored toInt32 and toUint32 functions to follow ECMAScript spec more closely and handle Go 1.26 float-to-int conversion changes
internal/jsnum/jsnum_test.go Updated TestBitwiseNOT to use pre-computed expected values instead of computing them during test execution

@jakebailey jakebailey added this pull request to the merge queue Oct 10, 2025
Merged via the queue into main with commit 7dd4ae5 Oct 10, 2025
22 checks passed
@jakebailey jakebailey deleted the jabaile/fix-jsnum-again branch October 10, 2025 16:33
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.

2 participants