fix(codegen): map call-chain punctuation for correct V8 stack columns#22789
Merged
graphite-app[bot] merged 1 commit intoJun 1, 2026
Merged
Conversation
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
1c5f694 to
cbc46f8
Compare
a2bc427 to
035c74b
Compare
Member
Author
Merge activity
|
…#22789) ## Summary - `oxc_codegen` produced off-by-one V8 stack-trace and coverage columns for call-chain shapes. When a `(`/`.`/`[` is chained directly onto a call — or any expression ending in `)`/`]` — that punctuation was left unmapped, so the lookup fell back to the closing delimiter one column to the left. Surfaced as vitest failures in vite-ecosystem-ci: `test.each([1, 2])(…)` reporting column 17 instead of 18, and `await expect(…).resolves.toBe(4)` reporting 40 instead of 41. - Root cause: such operands end in `)` or `]`, so there is no trailing identifier token for a source-map consumer to anchor on. The fix maps that punctuation from `print_expr` — keyed on postfix precedence — back to the operand's exclusive end, exactly where V8 reports the frame. This is the trailing end-mapping that Babel and TSC apply to every node, scoped here to just the no-anchor sites. The closing `)`/`}`/`]` keep their existing at-delimiter mapping, which `v8-to-istanbul` relies on for coverage range boundaries (#22001). - Only these no-anchor chain sites are mapped — not the byte past every delimiter — so the added source-map tokens stay at **+0.6%** over baseline. (A blanket variant that mapped every delimiter regressed codegen ~5% on CodSpeed.) - Verified end-to-end through `node --enable-source-maps`: each chained frame resolves to the correct column (the outer `(` for curried calls, the `.` for a getter/member on a call result), and regresses by exactly one column when the mapping is disabled. Covered by the `stacktrace_is_correct` integration test and unit tests in `sourcemap.rs`. ## Example `factory()()` — V8 reports the outer call at the `(` after the inner `)`: ``` without this fix: at <anonymous> (input.js:7:9) ← the inner `)` with this fix: at <anonymous> (input.js:7:10) ← the outer `(` ✓ ``` The same one-column drift affected `expect(x).resolves` (the `.` getter access after the `)`). ## Compared with esbuild and tsc Same input through each tool (identical generated code), visualized with [source-map-viewer](https://source-map-viewer.void.app): ```js const outer = factory()(); // outer `(` of factory()() const member = expect(x).resolves; // `.` of expect(x).resolves const indexed = arr[i](); // `(` of arr[i]() ``` | Source token | oxc (this PR) | tsc | esbuild | | --- | :--: | :--: | :--: | | outer `(` of `factory()()` | ✓ | ✓ | ✗ | | `.` of `expect(x).resolves` | ✓ | ✓ | ✗ | | `(` of `arr[i]()` | ✓ | ✓ | ✗ | - **vs esbuild** ([compare](https://source-map-viewer.void.app/compare?a=QCnJWx7H&b=Y4FwbiMW)): oxc adds the three chain mappings esbuild lacks — esbuild maps the `]` one column left of the `(`, the exact off-by-one. This PR resolves what esbuild still gets wrong. - **vs tsc** ([compare](https://source-map-viewer.void.app/compare?a=UjsQoNTX&b=Y4FwbiMW)): all three positions match tsc exactly. tsc maps ~10 more positions because it gives every node a trailing end-mapping; this PR maps only the no-anchor sites (+0.6% vs tsc's full density). --- This PR was prepared with AI assistance; I reviewed, tested, and verified the changes.
035c74b to
7914807
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
oxc_codegenproduced off-by-one V8 stack-trace and coverage columns for call-chain shapes. When a(/./[is chained directly onto a call — or any expression ending in)/]— that punctuation was left unmapped, so the lookup fell back to the closing delimiter one column to the left. Surfaced as vitest failures in vite-ecosystem-ci:test.each([1, 2])(…)reporting column 17 instead of 18, andawait expect(…).resolves.toBe(4)reporting 40 instead of 41.)or], so there is no trailing identifier token for a source-map consumer to anchor on. The fix maps that punctuation fromprint_expr— keyed on postfix precedence — back to the operand's exclusive end, exactly where V8 reports the frame. This is the trailing end-mapping that Babel and TSC apply to every node, scoped here to just the no-anchor sites. The closing)/}/]keep their existing at-delimiter mapping, whichv8-to-istanbulrelies on for coverage range boundaries (fix(codegen): correct sourcemap end mappings for closing delimiters #22001).node --enable-source-maps: each chained frame resolves to the correct column (the outer(for curried calls, the.for a getter/member on a call result), and regresses by exactly one column when the mapping is disabled. Covered by thestacktrace_is_correctintegration test and unit tests insourcemap.rs.Example
factory()()— V8 reports the outer call at the(after the inner):The same one-column drift affected
expect(x).resolves(the.getter access after the)).Compared with esbuild and tsc
Same input through each tool (identical generated code), visualized with source-map-viewer:
(offactory()().ofexpect(x).resolves(ofarr[i]()]one column left of the(, the exact off-by-one. This PR resolves what esbuild still gets wrong.This PR was prepared with AI assistance; I reviewed, tested, and verified the changes.