Repository navigation
fix(ecmascript): do not fold Number.isInteger / isSafeInteger of tiny non-integers to true - #27622
Conversation
… non-integers to true
|
/monitor-oxc run |
Monitor OxcCommit:
|
There was a problem hiding this comment.
🟢 Approval recommended
The focused implementation matches JavaScript number semantics and is covered by relevant regression tests.
0 open findings
What changed in this PR
Corrects constant folding for Number.isInteger and Number.isSafeInteger to require exact integral values.
Changes:
- Replaces epsilon-based checks with finite, exact truncation comparison.
- Adds regression coverage for tiny fractions and special values.
| File | Description |
|---|---|
crates/oxc_ecmascript/src/constant_evaluation/call_expr.rs |
Implements exact integral-number detection. |
crates/oxc_minifier/tests/peephole/replace_known_methods.rs |
Adds integer-folding regression tests. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Merging this PR will not alter performance
Comparing Footnotes
|
### 💥 BREAKING CHANGES - c7d229b transformer: [**BREAKING**] Decode JSX entities to lone surrogates (#26712) (Dunqing) - e3c1b11 ast: [**BREAKING**] Preserve lone surrogates in static property names (#27095) (Dunqing) - a0a987c ast: [**BREAKING**] Preserve lone surrogates in static member names (#26677) (Dunqing) ### 🚀 Features - 7bb2697 minifier: Minimize array join expression (#27905) (Armano) - 5d91db8 ast: Generate node ID setters for enums (#27500) (camc314) - 6b5057b minifier: Add de morgan optimization for void expressions (#27375) (Armano) ### 🐛 Bug Fixes - ce6a088 minifier: Fold string indices by UTF-16 code unit (#27096) (Dunqing) - 7c47246 linter: Check module specifiers and string values that contain lone surrogates (#26717) (Dunqing) - 782ab6e linter: Search string values without UTF-8 conversion (#26716) (Dunqing) - a5d8184 jsdoc: Parse code spans after quotes in descriptions (#27955) (leaysgur) - bd37e78 ecmascript: Look through sequence expressions in `to_primitive` (#27837) (baltasarblanco) - d92789d minifier: Drop debugger statements in unbraced bodies (#27867) (camc314) - 863b359 ecmascript: Correct parseFloat decimal prefix scanning (#27695) (camc314) - a2534c6 minifier: Preserve lexical initializers in dead for loops (#27676) (camc314) - 3543672 ecmascript: Do not fold Number.isInteger / isSafeInteger of tiny non-integers to true (#27622) (Baltasar Blanco) - 6987a61 ecmascript: BigInt loose equality folding (unknown operands and `StringToBigInt`) (#27656) (Baltasar Blanco) - 40e5644 minifier: Preserve indirect access when folding into call targets (#27082) (waltu) - 022306f ecmascript: Do not fold `Math.cbrt` of a negative number to `NaN` (#27621) (Baltasar Blanco) ### ⚡ Performance - faa75d8 transformer: Remove redundant scratch vectors (#27922) (camc314) - 88b5c35 minifier: Reduce code branches when evaluating roots (#27906) (Armano) - fe1d367 codegen: Remove branches from loops printing arrays (#27840) (overlookmotel) - 7bb6b62 codegen: Reduce hashmap lookups in `print_annotation_comment` (#27845) (overlookmotel) - 31ffd08 codegen: Avoid unnecessary comment clone (#27843) (overlookmotel) - a6456b7 codegen: Remove branch from `print_space_before_identifier` (#27842) (overlookmotel) - eeff416 codegen: Simplify `print_space_before_identifier` (#27841) (overlookmotel) - 5d877b8 diagnostics: Speed up zero-context span lookups (#27770) (camc314) - 2ab6b63 packages/codegen: Remove redundant `printSpaceBeforeIdentifier` and `last` updates in `printParams` (#27652) (overlookmotel) - e533278 codegen: Reduce writes in `printDecorators` (#27651) (overlookmotel) ### 📚 Documentation - 29caafd codegen: Format table in doc comment (#27839) (overlookmotel) - 3de03c1 packages/codegen: Correct outdated comments (#27838) (overlookmotel)
Bug
(oxc-minify 0.153.0, default options; same output on
mainat 9a9eb83.) Also through a constant (const x = 5e-324; Number.isInteger(x)→!0).It also changes control flow:
minifies to
var e=setImmediate(function(){console.log(`ran`)});clearImmediate(e);: the original printsran, the minified prints nothing.try_fold_number_methods(crates/oxc_ecmascript/src/constant_evaluation/call_expr.rs) testsextracted.fract().abs() < f64::EPSILON.f64::EPSILONis about2.2e-16, so every number with a smaller fractional part counts as an integer.Number.isIntegerandNumber.isSafeIntegeronly accept an integral Number: "a finite Number value whose mathematical value is in the set of integers". That is an exact check.Fix
is_integral = n.is_finite() && n.trunc() == n, used by both methods.Tests
In
crates/oxc_minifier/tests/peephole/replace_known_methods.rs:test_fold_number_functions_is_safe_integergets5e-324and-1e-20→false, and a newtest_fold_number_functions_is_integercovers1,1e300→trueand1.5,5e-324,1e-300,Infinity,NaN→false. Both functions fail without the change.Checks:
cargo test --all-features -p oxc_ecmascript -p oxc_minifier,cargo clippy --all-targets --all-features(warnings denied) on both crates,cargo fmt --check,cargo doc(warnings denied) on both crates,typos,cargo shear,napi/minifybuild-test+test;cargo minsize,cargo allocsandcargo coverageleave every snapshot unchanged.Found with a differential fuzzer I wrote (it runs the original and the minified program under Node and compares the output). The fix, the tests and this description were written with AI assistance, and I reviewed them. The checks above and the before/after runs of the inputs were done by the AI agent in my local checkout; I did not re-run them by hand.