You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(wasm): Number.isNaN / isFinite / isInteger / isSafeInteger on the web target - #14615
On --target web, Number.isNaN, Number.isFinite, Number.isInteger and Number.isSafeInteger return undefined: the WASM emitter has no arm for the four Expr::Number* predicates, so they fall through to the TAG_UNDEFINED catch-all. One of the gaps listed in #14595.
Changes
emit/expr/regex_globals.rs: one arm for the four variants. Number.isNaN / Number.isFinite reuse the existing is_nan / is_finite bridges, which already implement the non-coercing Number versions.
wasm_runtime.js: number_is_integer and number_is_safe_integer bridges.
emit/string_collection.rs: interns the two new bridge names and walks the argument.
emit/tests.rs: number_predicates_answer_like_node (including a string argument, which none of them coerce).
Noticed while reading this code, not changed here: the global isNaN(x) lowers to Expr::IsNaN, which uses the same non-coercing is_nan bridge, so isNaN("abc") is false on the web target (Node: true). That needs its own bridge; separate PR.
Test plan
Base 2d6ddf6d58 (current main).
without the change
this branch
new test
undefined ×12
matches node --experimental-strip-types
cargo test -p perry-codegen-wasm
—
47 passed
Built on macOS (Apple M6), nightly from rust-toolchain.toml, LLVM 23.1.5, Node 26.13.1. rustfmt --check on the touched Rust files.
Checklist
I have NOT bumped the workspace version or edited CHANGELOG.md
My commits follow the loose feat: / fix: / docs: / chore: prefix convention used in the log
I've read CONTRIBUTING.md and agree to the Code of Conduct
Corrected Number.isNaN, Number.isFinite, Number.isInteger, and Number.isSafeInteger behavior for web-targeted builds. These checks now return accurate results without coercing string arguments.
No actionable comments were generated in the recent review.
Walkthrough
The WebAssembly target now emits Number.isNaN, Number.isFinite, Number.isInteger, and Number.isSafeInteger as tagged booleans. Runtime bridges implement the integer checks. A semantics test covers results for numeric values, strings, NaN, and Infinity.
Changes: WebAssembly Number predicates
Layer / File(s)
Summary
Emit and evaluate Number predicates crates/perry-codegen-wasm/src/emit/expr/regex_globals.rs, crates/perry-codegen-wasm/src/emit/string_collection.rs, crates/perry-codegen-wasm/src/wasm_runtime.js, crates/perry-codegen-wasm/src/emit/tests.rs, changelog.d/14615-wasm-number-predicates.md
The code generator emits the four predicates through runtime bridges and converts their results to tagged booleans. String collection includes the integer predicate bridge names and operands. The runtime implements integer and safe-integer checks. The test checks predicate results without string coercion, and the changelog records the behavior under --target web.
This change makes four Number predicates return correct booleans on the web target instead of undefined. No actionable merge-blocking risk was found.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the four Number predicates and the web target, which matches the changeset: a new emitter arm for the four predicates, two new runtime bridges, and a test. The title is specific and fo…
Description check
The description follows the template's main sections: Summary, Changes, Related issue, and Test plan. The Summary states the bug and its cause. Changes lists each touched file. The related issue is gi…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the four Number predicates and the web target, which matches the changeset: a new emitter arm for the four predicates, two new runtime bridges, and a test. The title is specific and follows the fix(wasm): prefix convention. It is somewhat long at 80 characters, but it is clear.
Full details: Description check
Explanation
The description follows the template's main sections: Summary, Changes, Related issue, and Test plan. The Summary states the bug and its cause. Changes lists each touched file. The related issue is given as Refs #14595. The test plan gives concrete before/after results and the cargo test count. The template's checklist items are present and checked. The description does not paste the exact cargo test --workspace command from the template, and it does not mention docs/src updates, which the template asks for when CLI or runtime API changes. The description also omits a Screenshots section, which is optional. These gaps are minor.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 33fdcefb-f5c3-4b45-adb6-242e0e4b6b6c
Commits
Reviewing files that changed from the base of the PR and between b118dbc and d8d0e88.
Files selected for processing: 1
changelog.d/14615-wasm-number-predicates.md
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 2 remain after this review.
… web target
The WASM emitter had no arm for `Expr::NumberIsNaN`, `NumberIsFinite`,
`NumberIsInteger` or `NumberIsSafeInteger`, so all four fell through to the
`TAG_UNDEFINED` catch-all, and string collection did not walk their argument.
`Number.isNaN` / `isFinite` reuse the existing `is_nan` / `is_finite` bridges,
which already implement the non-coercing Number versions; `isInteger` and
`isSafeInteger` get bridges of their own.
`number_predicates_answer_like_node` prints `undefined` twelve times without
this change and matches Node with it.
Refs PerryTS#14595.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CUQanVsnitcrmYLAWWPc3p
Rebased onto 46b2339d82 (current main, which includes #14604, so the formatting / clippy / tokio-ban / tls-budget reds are gone from this branch). cargo test -p perry-codegen-wasm re-run on the new base: 47 passed. The base SHA in the description refers to the earlier base.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On
--target web,Number.isNaN,Number.isFinite,Number.isIntegerandNumber.isSafeIntegerreturnundefined: the WASM emitter has no arm for the fourExpr::Number*predicates, so they fall through to theTAG_UNDEFINEDcatch-all. One of the gaps listed in #14595.Changes
emit/expr/regex_globals.rs: one arm for the four variants.Number.isNaN/Number.isFinitereuse the existingis_nan/is_finitebridges, which already implement the non-coercing Number versions.wasm_runtime.js:number_is_integerandnumber_is_safe_integerbridges.emit/string_collection.rs: interns the two new bridge names and walks the argument.emit/tests.rs:number_predicates_answer_like_node(including a string argument, which none of them coerce).Related issue
Refs #14595.
Noticed while reading this code, not changed here: the global
isNaN(x)lowers toExpr::IsNaN, which uses the same non-coercingis_nanbridge, soisNaN("abc")isfalseon the web target (Node:true). That needs its own bridge; separate PR.Test plan
Base
2d6ddf6d58(currentmain).undefined×12node --experimental-strip-typescargo test -p perry-codegen-wasmBuilt on macOS (Apple M6), nightly from
rust-toolchain.toml, LLVM 23.1.5, Node 26.13.1.rustfmt --checkon the touched Rust files.Checklist
feat:/fix:/docs:/chore:prefix convention used in the log🤖 Generated with Claude Code
https://claude.ai/code/session_01CUQanVsnitcrmYLAWWPc3p
Summary by CodeRabbit
Number.isNaN,Number.isFinite,Number.isInteger, andNumber.isSafeIntegerbehavior for web-targeted builds. These checks now return accurate results without coercing string arguments.