Skip to content

Conversation

cknitt
Copy link
Member

@cknitt cknitt commented Sep 11, 2025

#7864 was incorrect, the return type of charCodeAt was float because the function returns NaN if the index is out of range.

Here is an attempt to fix this and make the API less weird by returning option<int> in that case (using a wrapper), and adding an unsafe version of the function for performance.

Copy link

pkg-pr-new bot commented Sep 11, 2025

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript-lang/rescript@7877

@rescript/darwin-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-arm64@7877

@rescript/darwin-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/darwin-x64@7877

@rescript/linux-arm64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-arm64@7877

@rescript/linux-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/linux-x64@7877

@rescript/runtime

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/runtime@7877

@rescript/win32-x64

npm i https://pkg.pr.new/rescript-lang/rescript/@rescript/win32-x64@7877

commit: b295b3f

@zth
Copy link
Member

zth commented Sep 11, 2025

@cknitt @fhammerschmidt as a general error message thing (thought triggered by this PR) - would it make sense to suggest using the Unsafe version if it exists? Like how we suggest Console.log2 when you call Console.log with 2 args. For Array.get used where a non-option is expected, it could be:

Hint: There's a Array.getUnsafe�function that returns a non-option.

The text needs to be worked through, but you get the idea.

@cknitt
Copy link
Member Author

cknitt commented Sep 11, 2025

@zth Possibly, but we have to be really careful with the wording there so that we don't steer people towards unsafe code.

Like, if you really care about performance here and if you have done your bounds checks, then there is also an unsafe version that you could use, but otherwise just handle the option properly.

@cknitt cknitt merged commit 4b9c715 into rescript-lang:master Sep 12, 2025
25 checks passed
@cknitt cknitt deleted the fix-charcode-at branch September 12, 2025 12:38
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