Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 98 additions & 2 deletions src/js_parser/lower/lower_decorators.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,28 @@
if let Some(info) = map.get(&pi.ref_.inner_index()).copied() {
let mut obj_expr = tgt_idx.target;
self.rewrite_private_accesses_in_expr(&mut obj_expr, map);
let private_access = self.private_get_expr(obj_expr, &info, expr_loc);
// `x.#m(...)` becomes `__privateGet(x, _m).call(x, ...)`, which
// references the receiver twice. Only identifiers and `this` can
// be repeated safely; any other receiver is captured in a
// temporary so its side effects run once and nested private
// calls don't duplicate the whole subtree (the duplication is
// exponential in the length of a chain like `o.#m().#m().#m()`).
let (get_obj, this_arg) = match &obj_expr.data {
js_ast::ExprData::EIdentifier(id) => {
let obj_ref = id.ref_;
(obj_expr, self.use_ref(obj_ref, obj_expr.loc))
}
js_ast::ExprData::EThis(_) => {
(obj_expr, self.new_expr(E::This {}, obj_expr.loc))
}
_ => {
let tmp_ref = self.generate_temp_ref(Some(b"_obj"));
let write = self.assign_to(tmp_ref, obj_expr, expr_loc);
let read = self.use_ref(tmp_ref, expr_loc);
(write, read)
}
Comment thread
robobun marked this conversation as resolved.
};
let private_access = self.private_get_expr(get_obj, &info, expr_loc);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
let call_target = self.new_expr(
E::Dot {
target: private_access,
Expand All @@ -769,7 +790,7 @@
let bump = self.arena;
let orig_args = e.args.slice_mut();
let mut new_args = BumpVec::with_capacity_in(1 + orig_args.len(), bump);
new_args.push(obj_expr);
new_args.push(this_arg);
for arg in orig_args.iter_mut() {
self.rewrite_private_accesses_in_expr(arg, map);
new_args.push(*arg);
Expand Down Expand Up @@ -842,17 +863,75 @@
}
}
js_ast::ExprData::EFunction(e) => {
let temps_before = self.temp_refs_to_declare.len();
let stmts = e.func.body.stmts.slice_mut();
self.rewrite_private_accesses_in_stmts(stmts, map);
e.func.body.stmts = self.declare_capture_temps_in_fn_body(
e.func.body.stmts,
temps_before,
e.func.body.loc,
);
}
js_ast::ExprData::EArrow(e) => {
let temps_before = self.temp_refs_to_declare.len();
let stmts = e.body.stmts.slice_mut();
self.rewrite_private_accesses_in_stmts(stmts, map);
e.body.stmts =
self.declare_capture_temps_in_fn_body(e.body.stmts, temps_before, e.body.loc);
}
_ => {}
}
}

/// Drain receiver-capture temporaries created past `baseline` into a
/// single `var` declaration statement; `None` if none were created.
fn drain_capture_temp_decls(&mut self, baseline: usize, loc: bun_ast::Loc) -> Option<Stmt> {
let total = self.temp_refs_to_declare.len();
if total == baseline {
return None;
}
let bump = self.arena;
let mut capture_decls = BumpVec::<G::Decl>::with_capacity_in(total - baseline, bump);
for i in baseline..total {
let capture_ref = self.temp_refs_to_declare[i].r#ref;
let binding = self.b(B::Identifier { r#ref: capture_ref }, loc);
capture_decls.push(G::Decl {
binding,
value: None,
});
}
self.temp_refs_to_declare.truncate(baseline);
Some(self.s(
S::Local {
decls: DeclList::from_bump_vec(capture_decls),
..Default::default()
},
loc,
))
}

/// Declare receiver-capture temporaries created past `temps_before` at the
/// top of the function body they were created in, so each invocation gets
/// a fresh binding. A binding hoisted outside the function would be shared
/// across invocations, and `__privateGet(_obj = recv, _s, getter)` runs the
/// user getter between the write and the `.call(_obj)` read; re-entering
/// the same call site through that getter would clobber the shared temp.
fn declare_capture_temps_in_fn_body(
&mut self,
stmts: js_ast::StmtNodeList,
temps_before: usize,
body_loc: bun_ast::Loc,
) -> js_ast::StmtNodeList {
let Some(decl_stmt) = self.drain_capture_temp_decls(temps_before, body_loc) else {
return stmts;
};
let old_stmts = stmts.slice();
let mut new_stmts = BumpVec::<Stmt>::with_capacity_in(old_stmts.len() + 1, self.arena);
new_stmts.push(decl_stmt);
new_stmts.extend_from_slice(old_stmts);
js_ast::StmtNodeList::from_bump(new_stmts)
}

fn rewrite_private_accesses_in_stmts(&mut self, stmts: &mut [Stmt], map: &PrivateLoweredMap) {
for stmt_item in stmts.iter_mut() {
match &mut stmt_item.data {
Expand Down Expand Up @@ -1026,6 +1105,12 @@
let p = self;
let bump = p.arena;

// Receiver-capture temporaries created by `rewrite_private_accesses_in_expr`
// land in `temp_refs_to_declare`; everything pushed past this point is
// declared in a `var` statement alongside the other lowering variables
// right before output assembly.
let temp_refs_before = p.temp_refs_to_declare.len();

// ── Phase 1: Setup ───────────────────────────────
let mut class_name_ref: Ref;
let mut class_name_loc: bun_ast::Loc;
Expand Down Expand Up @@ -2353,6 +2438,17 @@
class.has_decorators = false;
class.should_lower_standard_decorators = false;

// Declare the receiver-capture temporaries that were created outside any
// function body (field initializers, static blocks, pre-eval/decorate
// expressions). Temps created inside method/function/arrow bodies were
// already declared there by `declare_capture_temps_in_fn_body`; these
// remaining sites run at most once per class evaluation, so a binding
// hoisted next to the other lowering variables is safe (and matches
// where esbuild hoists them).

Check warning on line 2447 in src/js_parser/lower/lower_decorators.rs

View check run for this annotation

Claude / Claude Code Review

Comment overstates safety: instance field initializers run per-instance, not once per class evaluation

nit: this comment's "at most once per class evaluation" justification doesn't hold for *instance* field initializers — those run in the constructor on every `new C()`, and their temps land here (your own follow-up confirms the test's `@dec r = pick(this).#p("0")` temp is hoisted while "the initializer runs in the constructor"). So the same getter-reentrancy window `declare_capture_temps_in_fn_body` closed for method bodies remains open for instance field initializers. The window is the same extr
Comment thread
robobun marked this conversation as resolved.
Outdated
if let Some(decl_stmt) = p.drain_capture_temp_decls(temp_refs_before, loc) {
prefix_stmts.push(decl_stmt);
}

// ── Phase 8: Assemble output ─────────────────────
if is_expr {
let mut comma_parts = BumpVec::<Expr>::new_in(bump);
Expand Down
172 changes: 172 additions & 0 deletions test/bundler/transpiler/es-decorators.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,178 @@ describe("ES Decorators", () => {
});
});

describe("private member calls in lowered classes", () => {
// When a class is lowered for standard decorators, `recv.#m(...)` becomes
// `__privateGet(recv, _m).call(recv, ...)`. The receiver must be evaluated
// exactly once: duplicating it re-runs side effects and makes the printed
// output grow exponentially for chains like `o.#m().#m().#m()`.
test("chained optional private calls do not explode the transpiled output size", () => {
const chain = "?.Foo.#m()".repeat(20);
const source = `class Foo {
static #x = -0;
static #m = function() {};
@decorator() est() {
return [o${chain}];
}
}`;

const transpiler = new Bun.Transpiler({ loader: "js", target: "bun" });
const output = transpiler.transformSync(source);

// Exponential duplication produced ~47 MB for a 20-call chain; the
// single-evaluation lowering stays in the kilobytes.
expect(output.length).toBeLessThan(50_000);
// The lowered output must still be valid syntax.
expect(() => new Bun.Transpiler({ loader: "js" }).transformSync(output)).not.toThrow();
});

test("double-call private chains in decorated static field initializers stay linear", () => {
// Fuzzer-minimized variant: each `.#method()()` link re-lowers the whole
// receiver, so duplicating it doubles the printed output per link
// (~30 links allocated multiple GB before aborting).
const chain = ".#method()()".repeat(20);
const source = `class C {
@decorator() static s = new C()${chain.slice(0, -2)};
#method() { return 1e999; }
}`;

const transpiler = new Bun.Transpiler({ loader: "ts", target: "bun", deadCodeElimination: true });
const output = transpiler.transformSync(source);

// Exponential duplication produced ~64 MB for 20 links; the
// single-evaluation lowering stays in the kilobytes.
expect(output.length).toBeLessThan(50_000);
// The lowered output must still be valid syntax.
expect(() => new Bun.Transpiler({ loader: "js" }).transformSync(output)).not.toThrow();
});

test("calling the result of a private method call evaluates each link once", async () => {
const { stdout, stderr, exitCode } = await runDecorator(`
function dec(value, ctx) { return value; }
let evals = 0;
class C {
@dec static s = new C().#method()().#method()().#method()();
#method() { evals++; const self = this; return () => self; }
}
console.log(C.s instanceof C, evals);
`);
expect(stderr).toBe("");
expect(stdout).toBe("true 3\n");
expect(exitCode).toBe(0);
});

test("receiver temps are scoped per invocation, not shared across reentrant calls", async () => {
// A private getter runs user code inside __privateGet, between the
// `_obj = recv` write and the `.call(_obj)` read. If the getter reenters
// the same call site, a temp hoisted outside the method would be
// clobbered and the outer call would see the inner receiver. Declaring
// the temp inside the method body gives each invocation its own binding.
const { stdout, stderr, exitCode } = await runDecorator(`
function dec(value, ctx) { return value; }
let nextId = 0;
let depth = 0;
const order = [];
class C {
get #g() {
if (depth++ === 0) make().run();
const self = this;
return function () { order.push(self.id + ":" + this.id); };
}
@dec run() { make().#g(); }
}
function make() { const c = new C(); c.id = ++nextId; return c; }
make().run();
console.log(JSON.stringify(order));
`);
expect(stderr).toBe("");
// Each entry pairs the receiver seen at getter time with the receiver
// the returned function was invoked on; they must always match.
expect(stdout).toBe('["4:4","2:2"]\n');
expect(exitCode).toBe(0);
});

test("private method call receiver is evaluated exactly once", async () => {
const { stdout, stderr, exitCode } = await runDecorator(`
function dec(value, ctx) { return value; }
let receiverEvals = 0;
class Counter {
static #m = function (x) { return [this === Counter, x]; };
@dec test() {
return getCounter().#m(42);
}
}
function getCounter() { receiverEvals++; return Counter; }
console.log(JSON.stringify(new Counter().test()), receiverEvals);
`);
expect(stderr).toBe("");
expect(stdout).toBe("[true,42] 1\n");
expect(exitCode).toBe(0);
});

test("chained optional private method calls return the right value", async () => {
const { stdout, stderr, exitCode } = await runDecorator(`
function dec(value, ctx) { return value; }
class Chain {
#tag;
constructor(tag) { this.#tag = tag; }
#next() { return { Chain: new Chain(this.#tag + 1) }; }
@dec run(o) {
return o?.Chain.#next()?.Chain.#next()?.Chain.#next()?.Chain.tag();
}
tag() { return this.#tag; }
}
console.log(new Chain(0).run({ Chain: new Chain(10) }));
`);
expect(stderr).toBe("");
expect(stdout).toBe("13\n");
expect(exitCode).toBe(0);
});

test("private method calls through `this` and identifier receivers still work", async () => {
const { stdout, stderr, exitCode } = await runDecorator(`
function dec(value, ctx) { return value; }
class Fast {
#p(n) { return "p" + n; }
@dec viaThis() { return this.#p(1); }
@dec viaIdent(other) { return other.#p(2); }
}
const f = new Fast();
console.log(f.viaThis(), f.viaIdent(new Fast()));
`);
expect(stderr).toBe("");
expect(stdout).toBe("p1 p2\n");
expect(exitCode).toBe(0);
});

// Covers both temp placements in a decorated class expression: the method
// body receiver gets a per-invocation `var` inside the method, while the
// field initializer receiver is rewritten outside any function body, so
// its temp is hoisted to the nearest statement list through the
// class-expression path.
test("decorated class expression evaluates chained private call receivers once", async () => {
const { stdout, stderr, exitCode } = await runDecorator(`
function dec(value, ctx) { return value; }
let evals = 0;
let initEvals = 0;
function pick(x) { initEvals++; return x; }
const C = class Foo {
static #m = function (tag) { return { Foo, tag }; };
#p(tag) { return "i" + tag; }
@dec r = pick(this).#p("0");
@dec test(o) {
return o.effectful()?.Foo.#m("a")?.Foo.#m("b");
}
};
const o = { Foo: C, effectful() { evals++; return { Foo: C }; } };
const inst = new C();
console.log(inst.r, inst.test(o).tag, evals, initEvals);
`);
expect(stderr).toBe("");
expect(stdout).toBe("i0 b 1 1\n");
expect(exitCode).toBe(0);
});
});

describe("accessor with TypeScript annotations", () => {
test("accessor with definite assignment assertion (!)", async () => {
using dir = tempDir("es-dec-accessor-bang", {
Expand Down
Loading