Skip to content

js_parser: don't fold [x][0] in call position so this stays the array - #37085

Open
robobun wants to merge 1 commit into
mainfrom
farm/74cbffec/fold-call-this
Open

js_parser: don't fold [x][0] in call position so this stays the array#37085
robobun wants to merge 1 commit into
mainfrom
farm/74cbffec/fold-call-this

Conversation

@robobun

@robobun robobun commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Problem

With minify_syntax on (the default for bun run), the [x][0] -> x fold rebinds this when the folded expression is the callee of a call:

function f() { return this === undefined ? "undefined" : Array.isArray(this) ? "array" : "obj:" + this.tag; }
const o = { tag: "o", f };
console.log([o.f][0](), [0, o.f][1](), [o.f][0]?.(), [o.f]?.[0](), [function () { return Array.isArray(this); }][0]());

Node prints array array array array true. Bun prints undefined array undefined undefined false.

Per ECMA-262 EvaluateCall, when the callee is a property Reference like [o.f][0], thisValue is GetThisValue of that Reference, i.e. the array literal itself. #36730 changed the fold to emit (0, o.f)() for member-expression items "so this is not rebound", but that binds this to undefined, not the array. Items that are plain identifiers or function expressions were inlined bare ([y][0]() -> y()), which is wrong the same way.

Fix

No folded form can reproduce "this is the array literal", so the e_index array fold now bails out entirely when the index expression is a call target. This covers optional calls ([o.f][0]?.()) since e_call sets the same call_target. The multi-item path ([0, y][1]()) goes through the same gate.

Unaffected on purpose:

  • new [C][0]() still folds to new C: e_new does not mark a call target and new does not thread this through the callee Reference.
  • The "foo"[2] -> "o" string fold still fires in call position: a primitive callee throws TypeError either way.
  • The sibling {f: x}.f fold already bails on call targets.
  • The sibling comma/??/||/&&/ternary folds keep emitting (0, x)(): their bases are values, not References, so undefined is the correct receiver there.

Bumps the runtime transpiler cache version since cached minified output changes.

Tagged templates ([o.f][0] followed by a template literal) have the same receiver rule but need the template_tag tracking from #36735; that PR also bails this fold in tag position. Whichever lands first, the other rebases with a trivial conflict here and in the cache version.

Verification

# fail-before (system bun 1.4.0)
$ USE_SYSTEM_BUN=1 bun test test/bundler/transpiler/transpiler.test.js -t "property access inlining"
  (fail) bails out when the index is a call/assignment target
    Expected: "x = [obj.m][0]()"  Received: "x = (0, obj.m)()"
  (fail) preserves runtime semantics when inlining from a literal index
    + "this arr dot: => false (want => true)" (and 4 more)
  6 pass, 2 fail

# pass-after
$ bun bd test test/bundler/transpiler/transpiler.test.js
  187 pass, 0 fail

Also green with the fix: bundler_minify.test.ts (42 pass), esbuild/default.test.ts (192 tests), esbuild/dce.test.ts (95 tests), runtime-transpiler.test.ts (15 pass), transpiler-cache.test.ts (11 pass). The repro above prints array array array array true, matching Node.


[review] gate passed · iteration 0 · 3 files touched

fails on main (without fix)
ASAN without fix: 2 failed, 22 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/transpiler/transpiler.test.js
bun test v1.4.0 (2dfd467fc)

test/bundler/transpiler/transpiler.test.js:
(pass) Bun.Transpiler > handles errors when parsing macros [5.65ms]
(pass) Bun.Transpiler > normalizes \r\n [6.25ms]
1
(pass) Bun.Transpiler > doesn't hang indefinitely #2746 [6.13ms]
(pass) Bun.Transpiler > property access inlining > bails out with spread [12.51ms]
(pass) Bun.Transpiler > property access inlining > bails out with multiple items [4.72ms]
(pass) Bun.Transpiler > property access inlining > works [4.67ms]
(pass) Bun.Transpiler > property access inlining > works nested [4.49ms]
(pass) Bun.Transpiler > property access inlining > bails out when the array item is an optional chain [59.98ms]
76 |     transpiledOutput: code => {
77 |       return ts.parsed(code, false, false);
78 |     },
79 | 
80 |     expectPrintedMin_: (code, out) => {
81 |       expect(ts.parsedMin(code, !out.endsWith(";\n"), false)).toBe(out);
                                                                   ^
error: expect(received).toBe(ex
... (truncated)

release without fix: 3 failed, 22 skipped
bun test v1.4.0-canary.1 (0ffabf64d)

test/bundler/transpiler/transpiler.test.js:
(pass) Bun.Transpiler > handles errors when parsing macros [0.15ms]
(pass) Bun.Transpiler > normalizes \r\n [0.22ms]
1
(pass) Bun.Transpiler > doesn't hang indefinitely #2746 [0.13ms]
(pass) Bun.Transpiler > property access inlining > bails out with spread [0.23ms]
(pass) Bun.Transpiler > property access inlining > bails out with multiple items [0.06ms]
(pass) Bun.Transpiler > property access inlining > works [0.05ms]
(pass) Bun.Transpiler > property access inlining > works nested [0.04ms]
(pass) Bun.Transpiler > property access inlining > bails out when the array item is an optional chain [1.17ms]
76 |     transpiledOutput: code => {
77 |       return ts.parsed(code, false, false);
78 |     },
79 | 
80 |     expectPrintedMin_: (code, out) => {
81 |       expect(ts.parsedMin(code, !out.endsWith(";\n"), false)).toBe(out);
                                                                   ^
error: expect(received).toBe(expected)

Expected: "x = [obj.m][0]()"
Received: "x = (0, obj.m)()"

      at expectPrintedMin_ (/workspace/bun/test/bundler/transpiler/transpiler.test.js:81:63)
      at
... (truncated)
passes on PR (with fix)
ASAN with fix: 22 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/transpiler/transpiler.test.js
bun test v1.4.0 (2dfd467fc)

test/bundler/transpiler/transpiler.test.js:
(pass) Bun.Transpiler > handles errors when parsing macros [8.94ms]
(pass) Bun.Transpiler > normalizes \r\n [9.52ms]
1
(pass) Bun.Transpiler > doesn't hang indefinitely #2746 [6.42ms]
(pass) Bun.Transpiler > property access inlining > bails out with spread [11.20ms]
(pass) Bun.Transpiler > property access inlining > bails out with multiple items [4.72ms]
(pass) Bun.Transpiler > property access inlining > works [3.90ms]
(pass) Bun.Transpiler > property access inlining > works nested [4.35ms]
(pass) Bun.Transpiler > property access inlining > bails out when the array item is an optional chain [53.72ms]
(pass) Bun.Transpiler > property access inlining > bails out when the index is a call/assignment target [30.80ms]
(pass) Bun.Transpiler > property access inlining > preserves runtime semantics when inlining from a literal index [406.62ms]
(pass) Bun.Transpiler > property access inlining > bails out on optional-chain index int
... (truncated)

release with fix: 22 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     2dfd467fc6
  features     baseline

22 deps, 106 codegen, 1175 objects in 1000ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1238] install /workspace/bun
bun install v1.4.0-canary.1 (0ffabf64d)

Checked 124 installs across 170 packages (no changes) [14.00ms]
[2/1238] gen ErrorCode+*.h
[3/1238] gen bindgenv2
[4/1238] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (0ffabf64d)

Checked 1 install across 2 packages (no changes) [12.00ms]
[5/1238] fetch picohttpparser
[picohttpparser] up to date
[6/1238] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (0ffabf64d)

Checked 129 installs across 147 packages (no changes) [15.00ms]
[7/1238] fetch tinycc
[tinycc] up to date
[8/1237] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[9/1237] gen .bind.ts → GeneratedBindings.cpp
[10/1237] fetch zlib
[zlib] up to date
[11/1237] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h fro
... (truncated)
diff hotspot
src/js_parser/visit/visit_expr.rs          | 14 ++++++------
 src/jsc/RuntimeTranspilerCache.rs          |  4 +++-
 test/bundler/transpiler/transpiler.test.js | 34 ++++++++++++++++++++++++------
 3 files changed, 36 insertions(+), 16 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                        reads  edits  tests
src/js_parser/visit/visit_expr.rs               5      1      0
src/jsc/RuntimeTranspilerCache.rs               1      1      0
test/bundler/transpiler/transpiler.test.js      1      3      0

Calling through [x][0]() evaluates the callee as a property Reference,
so this inside the callee is the array literal itself. The fold emitted
(0, x)() for member-expression items and a bare x() for everything else,
binding this to undefined instead. Bail out of the fold for call targets
and bump the runtime transpiler cache version.
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Array literal index folding now skips call targets to preserve this binding. The runtime transpiler cache version increased to 26. Transpiler tests cover call bailouts, retained non-call folding, constructor calls, and runtime receiver behavior.

Changes

Array index call-target folding

Layer / File(s) Summary
Preserve call-target receiver semantics
src/js_parser/visit/visit_expr.rs, src/jsc/RuntimeTranspilerCache.rs
Call-target array index folding is disabled. The removed comma-expression workaround is replaced by cache format version 26.
Validate transpiler output and runtime binding
test/bundler/transpiler/transpiler.test.js
Tests preserve call expressions, retain ordinary and new folding, and verify the temporary array remains this for supported call forms.

Possibly related PRs

  • oven-sh/bun#36730: Overlaps in expression folding and transpiler tests for preserving call this semantics.
  • oven-sh/bun#36734: Also prevents [x][n] folding when reference semantics matter.
  • oven-sh/bun#36735: Also updates expression folding to preserve this for call-like targets.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely identifies the main fix: preventing [x][0] folding in call position to preserve this binding.
Description check ✅ Passed The description explains the problem, fix, scope, cache change, and verification results with sufficient technical detail.

Comment @coderabbitai help to get the list of available commands.

Comment thread src/js_parser/visit/visit_expr.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants