Repository navigation
Conversation
…of equality comparisons The transpiler prints an infinite number as "1 / 0" because "Infinity" may be shadowed. The visit pass moves literal operands of ==, !=, === and !== to the right-hand side, but it did not count that division as a literal, so transpiling the transpiler's own output swapped the operands: "a" != Infinity became "a" != 1 / 0 and then 1 / 0 != "a". Teach is_primitive_to_reorder that a division of two number literals whose value is infinite stands for a number literal.
|
Status: reproduced and fixed.
|
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
There was a problem hiding this comment.
LGTM — narrow, well-tested fixed-point fix for the equality-operand reorder.
What was reviewed:
- The new
BinDivarm only fires whenextract_numeric_valuessucceeds, which is limited toENumber/EInlinedEnum(ENumber)operands — both already side-effect-free in this predicate, so the reorder stays safe. - Checked the sole caller in
visit_binary.rs: swap runs only for==/!=/===/!==, so treating1/0as a literal on either side is value-preserving. - Confirmed
-1 / 0works becauseUnNegfolds toENumber(-1.0)unconditionally during visit before the reorder check runs. - Tests cover the fixed-point property directly (transform twice), plus negative cases (
1/2,0/0,x/0) that must not reorder.
Extended reasoning...
Overview
Adds one match arm to SideEffects::is_primitive_to_reorder in src/js_parser/scan/scan_side_effects.rs so that a literal division evaluating to ±Infinity (1 / 0, -1 / 0) counts as a reorderable primitive, matching the ENumber it stands in for. Adds a 22-case describe block to test/bundler/transpiler/transpiler.test.js asserting that transpiler output is a fixed point for equality comparisons against infinite numbers.
Security risks
None. The change is a pure predicate over already-parsed AST nodes. extract_numeric_values only returns Some for ENumber and EInlinedEnum wrapping an ENumber — no user code, no allocation, no I/O.
Level of scrutiny
Low-to-medium. This is an output-stability fix in the transpiler's visit pass: the emitted code was already semantically correct (equality is commutative), only the operand order oscillated between passes. The predicate is called from exactly one site (visit_binary.rs:183-184), gated to the four equality operators, and the new arm is strictly narrower than the existing ENumber arm it mirrors. I traced extract_numeric_value to confirm it cannot match anything with side effects, and traced visit_expr.rs to confirm -1 is folded to ENumber(-1.0) before the reorder check, so the -1 / 0 test cases are sound.
Other factors
The test coverage is thorough: every operator, both signs and both spellings of infinity, three literal left-operand kinds, a shadowed-Infinity file, an inlined enum, minified and plain output, negative cases proving finite/NaN/non-literal divisions are untouched, and a runtime new Function check that the reordered code evaluates to the same values. Each case asserts transformSync(transformSync(x)) === transformSync(x). The PR description explains and rules out the two alternative fix locations (folding 1/0 in visit; printing Infinity) with references to the specific issues that constrain them, and reports fuzzer verification. hideFromStackTrace is already imported in the test file. No outstanding reviewer comments.
|
Updated 10:22 AM PT - Aug 15th, 2026
❌ @robobun, your commit 9cb21b1 has some failures in 🧪 To try this PR locally: bunx bun-pr 39024That installs a local version of the PR into your bun-39024 --bun |
Problem
Bun.Transpiler(andbun build --no-bundle) output is not a fixed point when an equality compares a literal with an infinite number:"a" != Infinitytranspiles to"a" != 1 / 0, and transpiling that output again gives1 / 0 != "a".==,===and!==, for-Infinity,1e999and-1e999, for string, bigint andrequire.mainleft operands, and for inlined enum members whose value is infinite. Found by the transpiler fixed-point fuzzer in test: add seeded differential fuzz oracles for node:path, Bun.Glob, source maps and the transpiler #39006, which keeps infinities out of its literal pool until this lands.print_number(src/js_printer/lib.rs) prints an infiniteENumberas1 / 0whenever the symbol renamer has not run, becauseInfinitymay be shadowed (ReferenceError: Cannot access uninitialized variable. #7263).is_primitive_to_reorderin src/js_parser/scan/scan_side_effects.rs). Pass 1 seesEString != ENumberand leaves it alone; pass 2 parses1 / 0as anEBinary, which the predicate does not count as a literal, so it swaps.Fix
is_primitive_to_reorderalso returns true for a division of two number literals whose value is infinite, which is exactly whatprint_numberemits.1 / 0now takes the same side of a comparison asInfinitydoes:"a" != 1 / 0stays as written and1 / 0 != xbecomesx != 1 / 0, the wayInfinity != xalready did. A division with a finite or NaN value (1 / 2,0 / 0) is not affected, since the printer never produces those for a number.1 / 0back into a number in the visit pass: the bundler prints infinite numbers asInfinity, so folding a user's1 / 0would rewrite it toInfinityin non-minified bundles, and todayexport const Infinity = 1e999already bundles tovar Infinity = Infinity(js_printer: fix require("bun") and other printer literals being captured by same-named locals #35739 and js_printer: guard undefined/NaN/Infinity identifiers against local shadows #36122 reserve the name in the renamer). Folding would also have to track every future change to the folding policy (fix(transpiler): keep numeric binary expressions when fold would inflate output #30206 makes minify folding size-aware); the reorder predicate is correct whether or not the division was folded.Infinity: the transform printer has no scope information, which is why ReferenceError: Cannot access uninitialized variable. #7263 chose1 / 0. js_printer: guard undefined/NaN/Infinity identifiers against local shadows #36122 adds file-level shadow tracking, but it still falls back to1 / 0in files that declareInfinity, so the reorder still has to understand that spelling (covered by the "declares its own Infinity" test).test/bundler/transpiler/transpiler.test.js, newdescribe("equality comparisons against an infinite number"): 15 of the 22 cases fail on the released binary and all pass with this change (plain andminifyWhitespaceoutput, every operator, both infinities in both spellings, a file shadowingInfinity, an inlined enum, the negative cases, and a runtime check of the swapped divisions).transpiler.test.js,transpiler_constant_fold_eqeq.test.ts,bundler_minify.test.tsandregression/issue/07263.test.tspass with the debug build.Infinityand1e999added to its number pool: 9000 programs over five seeds pass with this change; three of those seeds fail on the released binary with this swap (details below).Background
Bun.Transpilerruns that pipeline without the bundler's symbol renamer, so the printer cannot rename a localInfinityout of the way and spells the value1 / 0instead; the bundler runs the renamer and printsInfinity.Infinity,NaNandundefinedthat resolve to the globals are replaced withENumber/EUndefinednodes during the visit (src/js_parser/defines_table.rs), which is why"a" != NaNround-trips: its printed form reads back as the same node kind.typeof x === "undefined",x == void 0, constant comparisons) only has to look at one side. It is unconditional, not a minify-only optimization, so it runs on every pass.minify_syntaxor inlining is on (should_fold_typescript_constant_expressionsin src/js_parser/parse/parse_entry.rs), which is why1 / 0stays anEBinaryin the default transform path.Fuzzer runs and the
bun build --no-bundleprobeReleased binary (1.4.0) with
"Infinity"and"1e999"added toNUMBERSin the #39006 fuzzer:Debug build with this change, same fuzzer: seeds 1 to 4 at 1500 programs each and the default seed at 3000 programs, all pass.
bun build --no-bundleonbefore:
y = "a" != 1 / 0;/z = 1 / 0 != x;, after:y = "a" != 1 / 0;/z = x != 1 / 0;. A real bundle of the same input still prints"a" != Infinityand the user's1 / 0text unchanged, only the operand order of the second line moves, matching howInfinity != xwas already printed.