From a68024f947bcfe42b1376c101b6b5ffc13dca522 Mon Sep 17 00:00:00 2001 From: Jarred Sumner Date: Fri, 11 Jul 2025 15:47:04 -0700 Subject: [PATCH] Improve tree-shaking of `try` statements in dead code (#20934) --- src/js_parser.zig | 85 +++++++++++++++++++---------- test/bundler/bundler_minify.test.ts | 64 ++++++++++++++++++++++ 2 files changed, 119 insertions(+), 30 deletions(-) diff --git a/src/js_parser.zig b/src/js_parser.zig index e55e9227f2..e72026b2e5 100644 --- a/src/js_parser.zig +++ b/src/js_parser.zig @@ -1962,19 +1962,29 @@ pub const SideEffects = enum(u1) { } } - // If this is in a dead branch, then we want to trim as much dead code as we - // can. Everything can be trimmed except for hoisted declarations ("var" and - // "function"), which affect the parent scope. For example: - // - // function foo() { - // if (false) { var x; } - // x = 1; - // } - // - // We can't trim the entire branch as dead or calling foo() will incorrectly - // assign to a global variable instead. - pub fn shouldKeepStmtInDeadControlFlow(p: anytype, stmt: Stmt, allocator: Allocator) bool { - if (!p.options.features.dead_code_elimination) return true; + fn shouldKeepStmtsInDeadControlFlow(stmts: []Stmt, allocator: Allocator) bool { + for (stmts) |child| { + if (shouldKeepStmtInDeadControlFlow(child, allocator)) { + return true; + } + } + return false; + } + + /// If this is in a dead branch, then we want to trim as much dead code as we + /// can. Everything can be trimmed except for hoisted declarations ("var" and + /// "function"), which affect the parent scope. For example: + /// + /// function foo() { + /// if (false) { var x; } + /// x = 1; + /// } + /// + /// We can't trim the entire branch as dead or calling foo() will incorrectly + /// assign to a global variable instead. + /// + /// Caller is expected to first check `p.options.dead_code_elimination` so we only check it once. + pub fn shouldKeepStmtInDeadControlFlow(stmt: Stmt, allocator: Allocator) bool { switch (stmt.data) { // Omit these statements entirely .s_empty, .s_expr, .s_throw, .s_return, .s_break, .s_continue, .s_class, .s_debugger => return false, @@ -2004,8 +2014,22 @@ pub const SideEffects = enum(u1) { }, .s_block => |block| { - for (block.stmts) |child| { - if (shouldKeepStmtInDeadControlFlow(p, child, allocator)) { + return shouldKeepStmtsInDeadControlFlow(block.stmts, allocator); + }, + + .s_try => |try_stmt| { + if (shouldKeepStmtsInDeadControlFlow(try_stmt.body, allocator)) { + return true; + } + + if (try_stmt.catch_) |*catch_stmt| { + if (shouldKeepStmtsInDeadControlFlow(catch_stmt.body, allocator)) { + return true; + } + } + + if (try_stmt.finally) |*finally_stmt| { + if (shouldKeepStmtsInDeadControlFlow(finally_stmt.stmts, allocator)) { return true; } } @@ -2014,43 +2038,43 @@ pub const SideEffects = enum(u1) { }, .s_if => |_if_| { - if (shouldKeepStmtInDeadControlFlow(p, _if_.yes, allocator)) { + if (shouldKeepStmtInDeadControlFlow(_if_.yes, allocator)) { return true; } const no = _if_.no orelse return false; - return shouldKeepStmtInDeadControlFlow(p, no, allocator); + return shouldKeepStmtInDeadControlFlow(no, allocator); }, .s_while => { - return shouldKeepStmtInDeadControlFlow(p, stmt.data.s_while.body, allocator); + return shouldKeepStmtInDeadControlFlow(stmt.data.s_while.body, allocator); }, .s_do_while => { - return shouldKeepStmtInDeadControlFlow(p, stmt.data.s_do_while.body, allocator); + return shouldKeepStmtInDeadControlFlow(stmt.data.s_do_while.body, allocator); }, .s_for => |__for__| { if (__for__.init) |init_| { - if (shouldKeepStmtInDeadControlFlow(p, init_, allocator)) { + if (shouldKeepStmtInDeadControlFlow(init_, allocator)) { return true; } } - return shouldKeepStmtInDeadControlFlow(p, __for__.body, allocator); + return shouldKeepStmtInDeadControlFlow(__for__.body, allocator); }, .s_for_in => |__for__| { - return shouldKeepStmtInDeadControlFlow(p, __for__.init, allocator) or shouldKeepStmtInDeadControlFlow(p, __for__.body, allocator); + return shouldKeepStmtInDeadControlFlow(__for__.init, allocator) or shouldKeepStmtInDeadControlFlow(__for__.body, allocator); }, .s_for_of => |__for__| { - return shouldKeepStmtInDeadControlFlow(p, __for__.init, allocator) or shouldKeepStmtInDeadControlFlow(p, __for__.body, allocator); + return shouldKeepStmtInDeadControlFlow(__for__.init, allocator) or shouldKeepStmtInDeadControlFlow(__for__.body, allocator); }, .s_label => |label| { - return shouldKeepStmtInDeadControlFlow(p, label.stmt, allocator); + return shouldKeepStmtInDeadControlFlow(label.stmt, allocator); }, else => return true, @@ -20043,7 +20067,7 @@ fn NewParser_( if (p.options.features.minify_syntax) { if (effects.ok) { if (effects.value) { - if (data.no == null or !SideEffects.shouldKeepStmtInDeadControlFlow(p, data.no.?, p.allocator)) { + if (data.no == null or !SideEffects.shouldKeepStmtInDeadControlFlow(data.no.?, p.allocator)) { if (effects.side_effects == .could_have_side_effects) { // Keep the condition if it could have side effects (but is still known to be truthy) if (SideEffects.simplifyUnusedExpr(p, data.test_)) |test_| { @@ -20057,7 +20081,7 @@ fn NewParser_( } } else { // The test is falsy - if (!SideEffects.shouldKeepStmtInDeadControlFlow(p, data.yes, p.allocator)) { + if (!SideEffects.shouldKeepStmtInDeadControlFlow(data.yes, p.allocator)) { if (effects.side_effects == .could_have_side_effects) { // Keep the condition if it could have side effects (but is still known to be truthy) if (SideEffects.simplifyUnusedExpr(p, data.test_)) |test_| { @@ -22378,10 +22402,10 @@ fn NewParser_( } var visited_count = visited.items.len; - if (p.is_control_flow_dead) { + if (p.is_control_flow_dead and p.options.features.dead_code_elimination) { var end: usize = 0; for (visited.items) |item| { - if (!SideEffects.shouldKeepStmtInDeadControlFlow(p, item, p.allocator)) { + if (!SideEffects.shouldKeepStmtInDeadControlFlow(item, p.allocator)) { continue; } @@ -22483,9 +22507,10 @@ fn NewParser_( var output = ListManaged(Stmt).initCapacity(p.allocator, stmts.items.len) catch unreachable; + const dead_code_elimination = p.options.features.dead_code_elimination; for (stmts.items) |stmt| { - if (is_control_flow_dead and p.options.features.dead_code_elimination and - !SideEffects.shouldKeepStmtInDeadControlFlow(p, stmt, p.allocator)) + if (is_control_flow_dead and dead_code_elimination and + !SideEffects.shouldKeepStmtInDeadControlFlow(stmt, p.allocator)) { // Strip unnecessary statements if the control flow is dead here continue; diff --git a/test/bundler/bundler_minify.test.ts b/test/bundler/bundler_minify.test.ts index 874eeac967..06f23814dc 100644 --- a/test/bundler/bundler_minify.test.ts +++ b/test/bundler/bundler_minify.test.ts @@ -626,4 +626,68 @@ describe("bundler", () => { "456", ], }); + + itBundled("minify/TrimCodeInDeadControlFlow", { + files: { + "/entry.js": /* js */ ` + // Basic dead code elimination after return + function test1() { + return 'foo'; + try { + return 'bar'; + } catch {} + } + + // Keep var declarations in dead try block + function test2() { + return foo = true; + try { + var foo; + } catch {} + } + + // Keep var declarations in dead catch block + function test3() { + return foo = true; + try {} catch { + var foo; + } + } + + // Complex async function with dead code after early return + async function test4() { + if (true) return { status: "disabled_for_development" }; + try { + const response = await httpClients.releasesApi.get(); + if (!response.ok) return { status: "no_release_found" }; + if (response.statusCode === 204) return { status: "up_to_date" }; + } catch (error) { + return { status: "no_release_found" }; + } + return { status: "downloading" }; + } + + console.log(test1()); + console.log(test2()); + console.log(test3()); + test4().then(result => console.log(result.status)); + `, + }, + minifySyntax: true, + minifyWhitespace: true, + minifyIdentifiers: false, + onAfterBundle(api) { + const file = api.readFile("out.js"); + expect(file).toContain('function test1(){return"foo"}'); + expect(file).toContain("return foo=!0;try{var foo}catch{}"); + expect(file).toContain("return foo=!0;try{}catch{var foo}"); + expect(file).toContain('async function test4(){return{status:"disabled_for_development"}}'); + expect(file).not.toContain("no_release_found"); + expect(file).not.toContain("downloading"); + expect(file).not.toContain("up_to_date"); + }, + run: { + stdout: "foo\ntrue\ntrue\ndisabled_for_development", + }, + }); });