transpiler: don't inline a single-use binding into a return when that creates a tail call

src/js_parser/p.rs

@@ -2097,6 +2097,17 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
                 }
                 js_ast::StmtData::SReturn(mut ret) => {
                     if let Some(value) = ret.value.as_mut() {
+                        // Don't turn `const r = f(); return r;` into the tail call
+                        // `return f();` the user didn't write (see
+                        // `has_call_in_tail_position`). Only when bundling, where the
+                        // user asked for minified output, is the saved binding worth the
+                        // frame; the runtime transpiler forces `minify_syntax` on.
+                        if !self.options.bundle
+                            && self.has_call_in_tail_position(replacement)
+                            && self.is_ref_in_tail_position(*value, r#ref)
+                        {
+                            return false;
+                        }
                         break 'brk js_ast::StoreRef::from_bump(value);
                     }
                 }
@@ -2188,12 +2199,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         'outer: {
             match expr.data {
                 js_ast::ExprData::EIdentifier(ident) => {
-                    if ident.ref_.eql(r#ref)
-                        || self.symbols[ident.ref_.inner_index() as usize]
-                            .link
-                            .get()
-                            .eql(r#ref)
-                    {
+                    if self.identifier_is_use_of(ident, r#ref) {
                         self.ignore_usage(r#ref);
                         return Substitution::Success(replacement);
                     }
@@ -2718,6 +2724,77 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
         Substitution::Failure(expr)
     }
 
+    fn identifier_is_use_of(&self, ident: E::Identifier, r#ref: Ref) -> bool {
+        ident.ref_.eql(r#ref)
+            || self.symbols[ident.ref_.inner_index() as usize]
+                .link
+                .get()
+                .eql(r#ref)
+    }
+
+    /// Whether evaluating `expr` ends in a call, i.e. substituting it into a
+    /// `return` value's tail position would make that call a tail call.
+    ///
+    /// JSC implements proper tail calls: in strict mode code (every ES module),
+    /// a call in tail position replaces the caller's frame, so the caller is
+    /// missing from every stack trace captured inside the callee. Tail position
+    /// propagates through the same operators as in JSC's bytecode generator:
+    /// the last operand of `,` and the right operand of `||`, `&&` and `??`,
+    /// and both branches of `?:`. `new`, `await`, `yield` and `import()` are
+    /// never tail calls.
+    fn has_call_in_tail_position(&self, expr: Expr) -> bool {
+        if !self.stack_check.is_safe_to_recurse() || self.reported_stack_overflow.get() {
+            self.report_stack_overflow(expr.loc);
+            return false;
+        }
+        match expr.data {
+            js_ast::ExprData::ECall(_)
+            | js_ast::ExprData::ERequireString(_)
+            | js_ast::ExprData::ERequireResolveString(_) => true,
+            js_ast::ExprData::ETemplate(template) => template.tag.is_some(),
+            js_ast::ExprData::EBinary(binary) => {
+                Self::is_tail_position_operator(binary.op)
+                    && self.has_call_in_tail_position(binary.right)
+            }
+            js_ast::ExprData::EIf(if_expr) => {
+                self.has_call_in_tail_position(if_expr.yes)
+                    || self.has_call_in_tail_position(if_expr.no)
+            }
+            _ => false,
+        }
+    }
+
+    /// Whether the single use of `ref` is in tail position of `expr`, a
+    /// `return` value (see `has_call_in_tail_position`).
+    fn is_ref_in_tail_position(&self, expr: Expr, r#ref: Ref) -> bool {
+        if !self.stack_check.is_safe_to_recurse() || self.reported_stack_overflow.get() {
+            self.report_stack_overflow(expr.loc);
+            return false;
+        }
+        match expr.data {
+            js_ast::ExprData::EIdentifier(ident) => self.identifier_is_use_of(ident, r#ref),
+            js_ast::ExprData::EBinary(binary) => {
+                Self::is_tail_position_operator(binary.op)
+                    && self.is_ref_in_tail_position(binary.right, r#ref)
+            }
+            js_ast::ExprData::EIf(if_expr) => {
+                self.is_ref_in_tail_position(if_expr.yes, r#ref)
+                    || self.is_ref_in_tail_position(if_expr.no, r#ref)
+            }
+            _ => false,
+        }
+    }
+
+    fn is_tail_position_operator(op: js_ast::op::Code) -> bool {
+        matches!(
+            op,
+            js_ast::op::Code::BinComma
+                | js_ast::op::Code::BinLogicalOr
+                | js_ast::op::Code::BinLogicalAnd
+                | js_ast::op::Code::BinNullishCoalescing
+        )
+    }
+
     pub(crate) fn prepare_for_visit_pass(&mut self) -> Result<(), crate::Error> {
         {
             // The wrapper stores only the arena and a non-capturing

src/jsc/RuntimeTranspilerCache.rs

@@ -51,7 +51,9 @@ bun_core::declare_scope!(cache, visible);
 /// Version 25: Every ModuleInfo record carries a trailing FetchParameters slot
 /// so ImportEntry/ExportEntry/StarExportEntry moduleRequestType matches JSC's
 /// after WebKit 90b2ecf79ae3 keyed m_loadedModules on (specifier, type).
-const EXPECTED_VERSION: u32 = 25;
+/// Version 26: A single-use `const r = f(); return r;` is no longer inlined into
+/// the tail call `return f();`, which lost the caller's frame from stack traces.
+const EXPECTED_VERSION: u32 = 26;
 
 /// Source files smaller than this are not written to / read from the on-disk
 /// transpiler cache. Originally 50 KiB, which excluded almost every file in a

test/bundler/bundler_minify.test.ts

@@ -1398,6 +1398,47 @@ describe("bundler", () => {
       expect(code).toMatch(/=>\s*\{\s*return a \+ 1;?\s*\}/);
     },
   });
+
+  // When the user asks for minified output, a single-use binding is inlined into the `return`
+  // even though that makes the call a tail call (esbuild does the same).
+  itBundled("minify/InlineSingleUseBindingIntoReturn", {
+    files: {
+      "/entry.js": /* js */ `
+        export function wrap() {
+          const result = inner();
+          return result;
+        }
+      `,
+    },
+    minifySyntax: true,
+    minifyIdentifiers: false,
+    onAfterBundle(api) {
+      const code = api.readFile("/out.js");
+      expect(code).toContain("return inner();");
+      expect(code).not.toContain("result");
+    },
+  });
+
+  // Without bundling this is the runtime transpiler's configuration (`bun run`/`bun test` force
+  // minify-syntax on for bun targets). There the binding stays: `return inner();` would be a tail
+  // call the user never wrote, and JSC would drop `wrap`'s frame from stacks captured in `inner`.
+  itBundled("minify/KeepSingleUseBindingBeforeReturnWhenNotBundling", {
+    files: {
+      "/entry.js": /* js */ `
+        export function wrap() {
+          const result = inner();
+          return result;
+        }
+      `,
+    },
+    bundling: false,
+    minifySyntax: true,
+    minifyIdentifiers: false,
+    onAfterBundle(api) {
+      const code = api.readFile("/out.js");
+      expect(code).toMatch(/const result = inner\(\);\s*return result;/);
+    },
+  });
 });
 
 // The runtime transpiler (`bun run`/`bun test`) implicitly enables

test/bundler/transpiler/runtime-transpiler.test.ts

@@ -210,6 +210,133 @@ test("math.pow", () => {
   expect(20.4 ** -0.5 + "").toEqual("0.22140372138502384");
 });
 
+// The runtime transpiler has minify-syntax on, which inlines a single-use `const`/`let` into the
+// statement that uses it. Rewriting `const r = f(); return r;` into `return f();` creates a tail
+// call the user never wrote: JSC implements proper tail calls in strict mode code (every ES
+// module), so the returning function's frame would be gone from every stack trace captured inside
+// `f`. The binding has to stay whenever the initializer ends in a call and the use is the value
+// being returned. Everything else keeps getting inlined.
+describe("single-use inlining does not create tail calls", () => {
+  const transpiler = new Bun.Transpiler({ loader: "js", target: "bun" });
+
+  function transpileBody(body: string): string {
+    const code = transpiler.transformSync(`function hello(c, x) {\n${body}\n}`);
+    const lines = code.trim().split("\n");
+    expect(lines[0]).toBe("function hello(c, x) {");
+    expect(lines.at(-1)).toBe("}");
+    return lines
+      .slice(1, -1)
+      .map(line => line.trim())
+      .join(" ");
+  }
+
+  test.each([
+    ["const r = f(); return r;", "const r = f(); return r;"],
+    ["let r = f(); return r;", "let r = f(); return r;"],
+    ["const r = x.f(); return r;", "const r = x.f(); return r;"],
+    ["const r = f.call(x); return r;", "const r = f.call(x); return r;"],
+    ["const r = f?.(); return r;", "const r = f?.(); return r;"],
+    ["const r = f()(); return r;", "const r = f()(); return r;"],
+    ["const r = f`x`; return r;", "const r = f`x`; return r;"],
+    ['const r = require("./x"); return r;', 'const r = require("./x"); return r;'],
+    ['const r = require.resolve("./x"); return r;', 'const r = require.resolve("./x"); return r;'],
+    ["const r = c ? f() : g(); return r;", "const r = c ? f() : g(); return r;"],
+    ["const r = c ? 1 : g(); return r;", "const r = c ? 1 : g(); return r;"],
+    ["const r = c || f(); return r;", "const r = c || f(); return r;"],
+    ["const r = c && f(); return r;", "const r = c && f(); return r;"],
+    ["const r = c ?? f(); return r;", "const r = c ?? f(); return r;"],
+    ["const r = (g(), f()); return r;", "const r = (g(), f()); return r;"],
+    // `a` is inlined into `b`'s initializer, which then has to stay.
+    ["const a = f(); const b = a; return b;", "const b = f(); return b;"],
+    // A side-effect free initializer may be substituted into a branch of the returned expression,
+    // and those branches are tail positions too.
+    ["const r = /* @__PURE__ */ f(); return c ? r : 0;", "const r = f(); return c ? r : 0;"],
+    ["const r = /* @__PURE__ */ f(); return c || r;", "const r = f(); return c || r;"],
+    ["const r = /* @__PURE__ */ f(); return c ?? r;", "const r = f(); return c ?? r;"],
+  ])("keeps the binding: %s", (input, expected) => {
+    expect(transpileBody(input)).toBe(expected);
+  });
+
+  test.each([
+    // The use is not the returned value, so the call does not end up in tail position.
+    ["const r = f(); return r.x;", "return f().x;"],
+    ["const r = f(); return r();", "return f()();"],
+    ["const r = f(); return r + 1;", "return f() + 1;"],
+    ["const r = f(); return !r;", "return !f();"],
+    ["const r = f(); return r ? 1 : 2;", "return f() ? 1 : 2;"],
+    ["const r = f(); return r || c;", "return f() || c;"],
+    ["const r = f(); return [r];", "return [f()];"],
+    ["const r = f(); throw r;", "throw f();"],
+    ["const r = f(); r.x;", "f().x;"],
+    ["const r = f(); if (r) g();", "if (f()) g();"],
+    // The initializer does not end in a call.
+    ["const r = new F(); return r;", "return new F;"],
+    ["const r = f().x; return r;", "return f().x;"],
+    ["const r = f() + 1; return r;", "return f() + 1;"],
+    ["const r = f() ? 1 : 2; return r;", "return f() ? 1 : 2;"],
+    ["const r = f() || c; return r;", "return f() || c;"],
+    ["const r = `${f()}`; return r;", "return `${f()}`;"],
+    ['const r = import("./x"); return r;', 'return import("./x");'],
+    ["const r = c; return r;", "return c;"],
+    ["const r = 1; return r;", "return 1;"],
+  ])("still inlines: %s", (input, expected) => {
+    expect(transpileBody(input)).toBe(expected);
+  });
+
+  test("the initializer of an awaited call is still inlined", () => {
+    const code = transpiler.transformSync("async function hello() { const r = await f(); return r; }");
+    expect(code).toContain("return await f();");
+  });
+
+  // Nothing in this file's source says it is strict mode code (no import/export, no "use strict"),
+  // but a `.mjs` file is still evaluated as a module, so JSC still tail calls in it.
+  test.concurrent("the returning function stays in the stack trace", async () => {
+    using dir = tempDir("transpiler-inlined-tail-call", {
+      "chain.mjs": /* js */ `
+        function captureStack() {
+          return new Error("captured").stack;
+        }
+        function viaConst() {
+          const r = captureStack();
+          return r;
+        }
+        function* viaGenerator() {
+          const r = captureStack();
+          return r;
+        }
+        const viaArrow = () => {
+          const r = captureStack();
+          return r;
+        };
+        const frames = stack => stack.split("\\n").slice(1, 3).map(line => line.trim().split(" ")[1]);
+        console.log(JSON.stringify({
+          viaConst: frames(viaConst()),
+          viaGenerator: frames(viaGenerator().next().value),
+          viaArrow: frames(viaArrow()),
+        }));
+      `,
+    });
+
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "chain.mjs"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+    expect(stderr).toBe("");
+    expect(JSON.parse(stdout)).toEqual({
+      viaConst: ["captureStack", "viaConst"],
+      viaGenerator: ["captureStack", "viaGenerator"],
+      viaArrow: ["captureStack", "viaArrow"],
+    });
+    expect(exitCode).toBe(0);
+  });
+});
+
 describe("unterminated string literals in large files", () => {
   test("reports an unterminated string literal at the end of a large JavaScript file", async () => {
     using dir = tempDir("transpiler-long-unterminated-js", {

test/js/bun/test/stack.test.ts

@@ -121,6 +121,31 @@ test("throwing inside an error suppresses the error and continues printing prope
   expect(exitCode).toBe(1);
 });
 
+// Modules are strict mode code, where JSC turns `return f()` into a proper tail call and `f` no
+// longer sees the returning function on the stack. The transpiler inlines single-use bindings, so
+// without care `const r = f(); return r;` becomes exactly that tail call.
+test("a function returning a call's result through a binding is in the callee's stack", () => {
+  function captureStack() {
+    return new Error("captured").stack!;
+  }
+  function viaConst() {
+    const result = captureStack();
+    return result;
+  }
+  function viaConditional(flag = true) {
+    const result = /* @__PURE__ */ captureStack();
+    return flag ? result : "";
+  }
+
+  const frames = (stack: string) =>
+    stack
+      .split("\n")
+      .slice(1, 3)
+      .map(line => line.trim().split(" ")[1]);
+  expect(frames(viaConst())).toEqual(["captureStack", "viaConst"]);
+  expect(frames(viaConditional())).toEqual(["captureStack", "viaConditional"]);
+});
+
 test("Async functions frame should be included in stack trace", async () => {
   async function foo() {
     return await bar();