mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
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:
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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user