Skip to content

bun-types: correct the tail call note in the callerSourceOrigin JSDoc - #42337

Open
robobun wants to merge 1 commit into
mainfrom
robobun/de76c532/jsc-dts-tail-call-note
Open

robobun wants to merge 1 commit into
mainfrom
robobun/de76c532/jsc-dts-tail-call-note

Conversation

@robobun

@robobun robobun commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • The JSDoc of callerSourceOrigin (packages/bun-types/jsc.d.ts:202) says JavaScriptCore performs proper tail calls in all strict mode code. It concludes that a helper that does return callerSourceOrigin() reports the file of its caller.
  • Upgrade WebKit to cf1b36ec8703 #42319 upgraded WebKit to cf1b36ec8703. Since then, generator, async function and async generator bodies emit no tail call for return f() ([JSC] eval call should recognize tail-position WebKit/WebKit#73407, BytecodeGenerator.cpp:463). A helper of that kind now reports its own file.

Fix

  • The JSDoc names the exception. A new @example shows a plain helper and an async helper in helper.ts, called from index.ts.
  • Comments only. No declaration changes.
  • test/js/bun/jsc/bun-jsc.test.ts gets two tests. One runs the @example and compares stdout with the comments in the example. It fails with the old JSDoc. The other checks seven kinds of helper from a fixture module. Four of them fail on the engine before Upgrade WebKit to cf1b36ec8703 #42319.
  • Verified: both pass on a debug build of main (Linux x64) and on canary 6a92015fc (Windows x64). All 40 tests of the file pass.

Background

  • callerSourceOrigin() walks the stack from its own frame. It skips builtin functions and returns the source URL of the first other JavaScript frame, or null if there is none.
  • A tail call reuses the frame of the calling function. After a tail call to callerSourceOrigin() the helper is not on the stack, so the first frame is the caller of the helper.
  • The ECMAScript specification gives generator and async bodies no tail position. JavaScriptCore now follows that.
Notes

Probe: each helper lives in helper.mjs and does return callerSourceOrigin(). main.mjs calls it. The columns are bun 1.4.3-canary.1+4ff919377 (WebKit dfd696443b9b) and a debug build of main at 6a92015 (WebKit cf1b36ec8703).

helper                              before #42319   main
plain function, arrow, method       main.mjs        main.mjs
function*, generator method         main.mjs        helper.mjs
async function, no await            helper.mjs      helper.mjs
async function, after an await      null            helper.mjs
async arrow, after an await         null            helper.mjs
async function*                     main.mjs        helper.mjs
async function*, after an await     null            helper.mjs
  • Before the upgrade, an async helper with no await already reported its own file. After an await, the body ran from the microtask queue and made the tail call with no other JavaScript frame on the stack, so the result was null.
  • The result does not depend on the JIT tier. On the debug build of main, 300000 calls of the plain helper and 300000 calls of the generator helper each give one answer.
  • The other sentences of the JSDoc hold on main. eval, indirect eval and new Function report the file that created the code. Promise.resolve().then(callerSourceOrigin) gives null.
  • docs/ does not mention callerSourceOrigin, and no other doc sentence makes a claim about tail calls.
  • test/integration/bun-types/bun-types.test.ts gives the same result with and without this diff (11 pass, 10 fail). bun-types test: check against the newest @types/node release, not the latest dist-tag #42230 has the cause (the latest dist-tag of @types/node) and the fix.
  • Add tests for the WebKit cf1b36ec8703 upgrade: no tail calls in generator and async bodies #42335 adds engine-level tests for the same WebKit change (Error.stack frames, the await in an async generator return). It does not touch bun:jsc or the JSDoc.
  • bun-types: correct the JSDoc for spawn lazy and JSON5.stringify quotes #42257 adds test/integration/bun-types/jsdoc-examples.test.ts for examples in bun.d.ts. This PR keeps its test in bun-jsc.test.ts, so the two do not conflict.
  • prettier --check passes on the three files.

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/jsc/bun-jsc.test.ts

…erSourceOrigin

Since the WebKit upgrade in #42319, JavaScriptCore emits no tail call
in generator, async function and async generator bodies. A helper of
that kind that does `return callerSourceOrigin()` now reports its own
file in every case. Before, a generator helper reported the file of its
caller, and an async helper returned null after an await.

The JSDoc gets the exception and an @example. bun-jsc.test.ts runs the
example and checks each kind of helper.
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

On-demand reviews are free for the next 9 days. After that, they cost $0.25 per reviewed file.

Or wait 3 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 210db061-bf39-423e-86dc-3d02e4c9c2cd

📥 Commits

Reviewing files that changed from the base of the PR and between 6a92015 and 8424686.

📒 Files selected for processing (3)
  • packages/bun-types/jsc.d.ts
  • test/js/bun/jsc/bun-jsc.test.ts
  • test/js/bun/jsc/caller-source-origin-fixture.ts

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

@robobun

robobun commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Status: ready for review. Comments only in packages/bun-types/jsc.d.ts, plus two tests.

How it was checked:

  • bun bd test test/js/bun/jsc/bun-jsc.test.ts on a debug build of main (6a92015): 40 pass.
  • With packages/bun-types/jsc.d.ts from main, the @example test fails with "the JSDoc of callerSourceOrigin has no @example fence".
  • With bun 1.4.3-canary.1+4ff919377 (the engine before Upgrade WebKit to cf1b36ec8703 #42319), the helper test fails for generator, asyncGenerator, asyncFunctionAfterAwait and asyncArrowAfterAwait.
  • Windows x64, canary 6a92015fc: the three callerSourceOrigin tests pass.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. Because packages/bun-types/ is CODEOWNER-gated, a human sign-off on the JSDoc wording is still worthwhile.

What was reviewed:

  • JSDoc @ example extraction test — traced the regex/split against the actual jsc.d.ts content; the // file:///…/helper.ts output lines don't match the ^\/\/ (\w+\.ts)$ file-header regex, and the lines[i-1] access can't hit index -1 because line 0 is the import statement.
  • Cross-platform path handling — tempDir realpaths os.tmpdir() (macOS /private/var), and both sides of the replaceAll are file:// URLs so Windows backslashes aren't compared.
  • Harness conventions — tempDir/bunExe/bunEnv, await using spawn, concurrent pipe drain, stderr/stdout asserted before exit code, fixture named *-fixture.ts, module-scope import.
Extended reasoning...

Overview

This PR touches three files: a JSDoc-only edit to packages/bun-types/jsc.d.ts (adds one sentence noting that generator/async helpers never tail-call callerSourceOrigin(), plus a two-file @ example), a new seven-variant fixture module test/js/bun/jsc/caller-source-origin-fixture.ts, and two new tests in the existing test/js/bun/jsc/bun-jsc.test.ts. No declaration signatures change and no runtime code is touched — the diff documents an engine-behavior change from a prior WebKit bump and pins it with tests.

Security risks

None. The change is documentation prose, a pure test fixture, and two tests. The subprocess spawn uses bunExe()/bunEnv in a harness-created temp dir with content derived from a checked-in .d.ts file; there is no network access, no untrusted input, and no auth/crypto surface.

Level of scrutiny

Low-to-moderate. The runtime is unchanged, so the review focused on whether the JSDoc claim is accurate (it matches the ECMAScript tail-position rules and the fixture test enforces it against the current engine), whether the @ example-parsing test is robust (traced the slice/regex path against the live file — the lastIndexOf("/**", end)/lastIndexOf("*/", declaration) pair brackets the right comment, the ^[ \t]*\* ? strip handles both * text and bare * lines, and the file-header regex ^\/\/ (\w+\.ts)$ correctly rejects the // file:///… output-comment lines), and whether the tests follow repo conventions (they do: tempDir, await using, Promise.all pipe drain, output asserted before exit code, .toEqual on a combined object, module-scope fixture import, *-fixture.ts naming).

Other factors

.github/CODEOWNERS assigns *.d.ts and /packages/bun-types/ to a named owner, so per the approval guidelines this should get a human look rather than an auto-approve even though the change is small and well-tested. I also checked cross-platform behavior: tempDir builds on fs.realpathSync.native(os.tmpdir()), which resolves the macOS /var → /private/var symlink so Bun.pathToFileURL(String(dir)).href matches what callerSourceOrigin() prints; on Windows both the printed origin and the pathToFileURL result are forward-slash file:/// URLs, so the replaceAll lines up. The PR author reports both new tests pass on a debug build of main and on Windows canary.

This branch has not been deployed

No deployments
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