fix(minifier): preserve indirect access when folding into call targets (#27082)

Part of #24331.

When the minifier folds something into a call target, it can drop the
`(0, x)` form. That changes `this` for member calls and turns an
indirect `eval` into a direct one. The tracker lists the optional chain
case:

```js
(function(o) { console.log((0, o?.f)(42)); })({ a: "PASS", f(b) { return this.a || b; } });
// minified: (e?.f)(42), prints PASS instead of 42
```

Checking the other places that move an expression into a call target or
tag found three more paths with the same problem:

```js
// IIFE body substitution
(() => o.f)()();       // -> o.f()
delete (() => o.f)();  // -> delete o.f

// unused assignment removal (identifier and member targets)
let t; (t = o.f)();    // -> o.f()
let t; (t = eval)(x);  // -> eval(x), a direct eval

// single use inlining into a tag
let c = a.b; c``;      // -> a.b``
```

In a debug build, `(t = eval)(x)` and `(() => eval)()(x)` already trip
the `stale direct-eval flags` assertion on main.

Changes:
- `should_keep_indirect_access` treats an optional chain that ends in a
member access like a plain member access, as the `delete` arm already
does.
- A new `preserve_indirect_access_if_needed` helper is used by the IIFE
body substitution and both unused assignment removals.
- Single use inlining into a tag wraps a member or member chain
replacement as `(0, a.b)`. This is what the two commented out tests in
`esbuild.rs` expect, so they are enabled again.

Tests cover the comma, `&&`, `?:` and tagged template forms of the
optional chain case, the tracker repro, the IIFE, assignment and
inlining paths, and controls where nothing should be wrapped. Every new
assertion except the controls fails on main. The outputs were also
checked in Node against the inputs. `cargo minsize` and `cargo allocs`
show no change, and the minifier conformance counts for test262 and
babel match the snapshots.

`const e = eval; e(x)` is covered by the open #26511, so this PR leaves
that path alone.

AI-assisted (Claude Code); reviewed and validated by the contributor.
This commit is contained in:
waltu authored and GitHub committed 2026-10-14 10:06:42 +08:00
1 parent 644ca1c04f
commit 40e5644223
8 files changed
+114 -7

No files matched your search

@@ -2138,6 +2138,15 @@ impl<'a> PeepholeOptimizations {
}
}
Expression::TaggedTemplateExpression(tagged_expr) => {
// A member tag would change `this`: `let c = a.b; c``` => `(0, a.b)```
let keep_indirect = tagged_expr.tag.is_specific_id(search_for)
&& match &*replacement {
match_member_expression!(Expression) => true,
Expression::ChainExpression(chain) => {
matches!(&chain.expression, match_member_expression!(ChainElement))
}
_ => false,
};
if let Some(changed) = Self::substitute_single_use_symbol_in_expression(
&mut tagged_expr.tag,
search_for,
@@ -2145,6 +2154,11 @@ impl<'a> PeepholeOptimizations {
replacement_has_side_effect,
ctx,
) {
if changed && keep_indirect {
ctx.replace_expression_with(&mut tagged_expr.tag, |tag, ctx| {
Self::preserve_indirect_access(tag.span(), tag, ctx)
});
}
return Some(changed);
}
for elem in &mut tagged_expr.quasi.expressions {
@@ -649,6 +649,10 @@ impl<'a> PeepholeOptimizations {
match access_value {
Expression::Identifier(id) => id.name == "eval" && ctx.is_global_reference(id),
match_member_expression!(Expression) => true,
// Example case: `(0, o?.f)()` (`this` is undefined) -> `(o?.f)()` (`this` is `o`)
Expression::ChainExpression(chain) => {
matches!(&chain.expression, match_member_expression!(ChainElement))
}
_ => false,
}
}
@@ -702,6 +706,19 @@ impl<'a> PeepholeOptimizations {
)
}
/// Wrap `expr` as `(0, expr)` only if [`Self::should_keep_indirect_access`] flags it.
pub fn preserve_indirect_access_if_needed(
span: Span,
expr: Expression<'a>,
ctx: &TraverseCtx<'a>,
) -> Expression<'a> {
if Self::should_keep_indirect_access(&expr, ctx) {
Self::preserve_indirect_access(span, expr, ctx)
} else {
expr
}
}
pub fn remove_dead_code_exit_class_body(body: &mut ClassBody<'a>, _ctx: &mut TraverseCtx<'a>) {
body.body.retain(|e| !matches!(e, ClassElement::StaticBlock(s) if s.body.is_empty()));
}
@@ -794,7 +794,10 @@ impl<'a> PeepholeOptimizations {
if symbol_value.references.has_reads() {
return false;
}
let span = assign_expr.span;
let new_expr = assign_expr.right.take_in(ctx);
// `(a = o.f)()` => `(0, o.f)()`
let new_expr = Self::preserve_indirect_access_if_needed(span, new_expr, ctx);
ctx.replace_expression(e, new_expr);
false
}
@@ -869,7 +872,10 @@ impl<'a> PeepholeOptimizations {
// refs dead). Safe in value positions too — a plain `=` assignment's
// value IS the RHS value.
let Expression::AssignmentExpression(assign_expr) = e else { unreachable!() };
let span = assign_expr.span;
let new_expr = assign_expr.right.take_in(ctx);
// `(b.x = o.f)()` => `(0, o.f)()`
let new_expr = Self::preserve_indirect_access_if_needed(span, new_expr, ctx);
ctx.replace_expression(e, new_expr);
false
}
@@ -1919,7 +1919,8 @@ impl<'a> PeepholeOptimizations {
let new_value = if is_pure && Self::is_expression_result_unused(ctx) {
Expression::new_void_0(call_expr.span, ctx)
} else if let Some(taken) = Self::try_take_iife_body(expr, is_pure, ctx) {
taken
// Replace "(() => o.f)()()" with "(0, o.f)()"
Self::preserve_indirect_access_if_needed(call_expr.span, taken, ctx)
} else {
return;
};
@@ -1956,7 +1957,8 @@ impl<'a> PeepholeOptimizations {
Expression::new_void_0(call_expr.span, ctx)
} else if let Some(taken) = Self::try_take_iife_body(argument, is_pure, ctx)
{
taken
// Replace "(() => { return o.f })()()" with "(0, o.f)()"
Self::preserve_indirect_access_if_needed(call_expr.span, taken, ctx)
} else {
return;
};
@@ -1329,11 +1329,11 @@ fn test_flatten_values() {
test("(null ?? x)``", "x``;");
test("(null ?? x.y)``", "(0, x.y)``;");
test("(null ?? x[y])``", "(0, x[y])``;");
// test("function f(a) { let c = a.b; return c`` }", "function f(a) { return (0, a.b)``;}");
// test(
// "function f(a) { let c = a.b; return c`${x}` }",
// "function f(a) { return (0, a.b)`${x}`;}",
// );
test("function f(a) { let c = a.b; return c`` }", "function f(a) { return (0, a.b)``;}");
test(
"function f(a) { let c = a.b; return c`${x}` }",
"function f(a) { return (0, a.b)`${x}`;}",
);
test("return typeof (123, x)", "return typeof (0, x);");
test("return typeof (123, x.y)", "return typeof x.y;");
test("return typeof (123, x); var x", "return typeof x;var x;");
@@ -319,6 +319,17 @@ fn test_inline_past_readonly_variable() {
);
}
#[test]
fn test_inline_into_tag_keeps_this() {
test("function f(a) { let c = a?.b; return c`` }", "function f(a) { return (0, a?.b)`` }");
test("function f(a) { let c = a[b]; return c`` }", "function f(a) { return (0, a[b])`` }");
test(
"function f(a, t) { let c = a.b; return t`${c}` }",
"function f(a, t) { return t`${a.b}` }",
);
test("function f(a) { let c = a; return c.b`` }", "function f(a) { return a.b`` }");
}
#[test]
fn test_inline_read_before_await_tdz() {
// https://github.com/rolldown/rolldown/issues/9959
+13
View File
@@ -97,6 +97,19 @@ fn tagged_template() {
test("(!0 ? o.f : !1)()", "(0, o.f)()");
test("(!0 ? o.f : !1)``", "(0, o.f)``");
test("(1, o?.f)()", "(0, o?.f)()");
test("(1, o?.f)``", "(0, o?.f)``");
test("(!0 && o?.f)()", "(0, o?.f)()");
test("(!0 && o?.f)``", "(0, o?.f)``");
test("(!0 ? o?.f : !1)()", "(0, o?.f)()");
test("(!0 ? o?.f : !1)``", "(0, o?.f)``");
test("(1, o?.[k])()", "(0, o?.[k])()");
test("(1, o?.f.g)()", "(0, o?.f.g)()");
test("(1, o?.f())()", "(o?.f())()");
test_same(
"(function(o) { console.log((0, o?.f)(42)) })({ a: 'PASS', f(b) { return this.a || b } })",
);
test("foo(true && o.f)", "foo(o.f)");
test("foo(true ? o.f : false)", "foo(o.f)");
}
@@ -521,6 +521,14 @@ fn test_fold_iife() {
test("(() => { return x })()", "x;");
test_same("(function () { return x })()");
test("(() => o.f)()()", "(0, o.f)()");
test("(() => { return o.f })()()", "(0, o.f)()");
test("(() => o?.f)()``", "(0, o?.f)``");
test("(() => eval)()(x)", "(0, eval)(x)");
test("delete (() => o.f)()", "o.f");
test("v = typeof (() => x)()", "v = typeof (0, x)");
test("(() => o.f())()()", "o.f()()");
test("var a = /* @__PURE__ */ (() => x)()", "var a = x");
test_same("var a = /* @__PURE__ */ (() => x)(y, z)");
test("(/* @__PURE__ */ (() => !0)() ? () => x() : () => {})();", "x();");
@@ -688,6 +696,42 @@ fn remove_unused_assignment_expression() {
);
test_same_options("function foo(t) { return t = x(); } foo();", &options);
test_options(
"function foo() { let t; return (t = o.f)() } foo();",
"function foo() { return (0, o.f)() } foo();",
&options,
);
test_options(
"function foo() { let t; return (t = o?.f)`` } foo();",
"function foo() { return (0, o?.f)`` } foo();",
&options,
);
test_options(
"function foo() { let t; return (t = eval)(x) } foo();",
"function foo() { return (0, eval)(x) } foo();",
&options,
);
test_options(
"function foo() { let t; return delete (t = o.f) } foo();",
"function foo() { return o.f, !0 } foo();",
&options,
);
test_options(
"function foo() { let t = {}; return (t.x = o.f)() } foo();",
"function foo() { return (0, o.f)() } foo();",
&options,
);
test_options(
"function foo() { let t; return (t = o.f) == null ? void 0 : t() } foo();",
"function foo() { return (0, o.f)?.() } foo();",
&options,
);
test_options(
"function foo() { let t; return t = o.f } foo();",
"function foo() { return o.f } foo();",
&options,
);
// For loops
test_options("for (let i;;) i = 0", "for (;;);", &options);
// `i` reads as the implicit `undefined`, but `void 0` prints longer than a