From baa0423f54f45c849234da719eaf49f6917bafbc Mon Sep 17 00:00:00 2001 From: unbyte Date: Sat, 16 Nov 2024 16:47:31 +0800 Subject: [PATCH 1/2] feat: reuse names for non-overlapping symbols --- .../snapshots/snapshots_default.txt | 74 +++--- .../bundler_tests/snapshots/snapshots_ts.txt | 12 +- internal/js_ast/js_ast.go | 3 + internal/js_parser/js_parser.go | 17 ++ internal/renamer/renamer.go | 239 ++++++++++++++++-- 5 files changed, 287 insertions(+), 58 deletions(-) diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index 9465f0e104e..f58d2ad293e 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -22,9 +22,9 @@ export function a(o = foo) { return o; } export class b { - fn(r = foo) { - var f; - return r; + fn(o = foo) { + var r; + return o; } } export let c = [ @@ -53,34 +53,34 @@ TestArgumentsSpecialCaseNoBundle ---------- /out.js ---------- /* @__PURE__ */ (() => { var r; - function t(n = arguments) { + function t(r = arguments) { return arguments; } - (function(n = arguments) { + (function(r = arguments) { return arguments; }); - ({ foo(n = arguments) { + ({ foo(r = arguments) { return arguments; } }); - class u { - foo(e = arguments) { + class n { + foo(r = arguments) { return arguments; } } (class { - foo(n = arguments) { + foo(r = arguments) { return arguments; } }); - function t(n = arguments) { + function t(r = arguments) { var arguments; return arguments; } - (function(n = arguments) { + (function(r = arguments) { var arguments; return arguments; }); - ({ foo(n = arguments) { + ({ foo(r = arguments) { var arguments; return arguments; } }); @@ -131,14 +131,14 @@ TestArrowFnScope ---------- /out.js ---------- // entry.js tests = { - 0: (s = (e) => s + e, t) => s + t, - 1: (s, t = (e) => t + e) => t + s, - 2: (s = (a = (c) => s + a + c, b) => s + a + b, t, e) => s + t + e, - 3: (s, t, e = (a, b = (c) => e + b + c) => e + b + a) => e + s + t, + 0: (s = (t) => s + t, t) => s + t, + 1: (s, t = (s) => t + s) => t + s, + 2: (s = (t = (e) => s + t + e, e) => s + t + e, t, e) => s + t + e, + 3: (s, t, e = (s, t = (s) => e + t + s) => e + t + s) => e + s + t, 4: (x = (s) => x + s, y, x + y), 5: (y, x = (s) => x + s, x + y), - 6: (x = (s = (e) => x + s + e, t) => x + s + t, y, z, x + y + z), - 7: (y, z, x = (s, t = (e) => x + t + e) => x + t + s, x + y + z) + 6: (x = (s = (t) => x + s + t, t) => x + s + t, y, z, x + y + z), + 7: (y, z, x = (s, t = (s) => x + t + s) => x + t + s, x + y + z) }; ================================================================================ @@ -1111,25 +1111,25 @@ ok( TestDirectEvalTaintingNoBundle ---------- /out.js ---------- function test1() { - function add(n, t) { - return n + t; + function add(t, n) { + return t + n; } eval("add(1, 2)"); } function test2() { - function n(t, e) { - return t + e; + function t(t, n) { + return t + n; } (0, eval)("add(1, 2)"); } function test3() { - function n(t, e) { - return t + e; + function t(t, n) { + return t + n; } } function test4(eval) { - function add(n, t) { - return n + t; + function add(t, n) { + return t + n; } eval("add(1, 2)"); } @@ -5235,7 +5235,7 @@ class Foo { }; get #o() { } - set #o(a) { + set #o(o) { } } class Bar { @@ -5247,7 +5247,7 @@ class Bar { }; get #o() { } - set #o(a) { + set #o(o) { } } @@ -5426,21 +5426,21 @@ console.log((0, import_demo_pkg.default)()); TestNonDeterminismIssue2537 ---------- /out.js ---------- // entry.ts -function i(o, e) { +function b(n, e) { let r = "teun"; - if (o) { - let u = function(n) { + if (n) { + let n = function(n) { return n * 2; - }, t = function(n) { + }, u = function(n) { return n / 2; }; - var b = u, f = t; - r = u(e) + t(e); + var t = n, o = u; + r = n(e) + u(e); } return r; } export { - i as aap + b as aap }; ================================================================================ @@ -7196,9 +7196,9 @@ TestWithStatementTaintingNoBundle let outerDead = 3; with ({}) { var hoisted = 4; - let t = 5; + let e = 5; hoisted++; - t++; + e++; if (1) outer++; if (0) outerDead++; } diff --git a/internal/bundler_tests/snapshots/snapshots_ts.txt b/internal/bundler_tests/snapshots/snapshots_ts.txt index 254df136a3b..c25b053bb60 100644 --- a/internal/bundler_tests/snapshots/snapshots_ts.txt +++ b/internal/bundler_tests/snapshots/snapshots_ts.txt @@ -1633,26 +1633,26 @@ mustBeComputed( ================================================================================ TestTSMinifyNamespace ---------- /a.js ---------- -var Foo;(e=>{let a;(p=>foo(e,p))(a=e.Bar||={})})(Foo||={}); +var Foo;(a=>{let e;(e=>foo(a,e))(e=a.Bar||={})})(Foo||={}); ---------- /b.js ---------- -export var Foo;(e=>{let a;(p=>foo(e,p))(a=e.Bar||={})})(Foo||={}); +export var Foo;(a=>{let e;(e=>foo(a,e))(e=a.Bar||={})})(Foo||={}); ================================================================================ TestTSMinifyNamespaceNoArrow ---------- /a.js ---------- -var Foo;(function(e){let a;(function(p){foo(e,p)})(a=e.Bar||={})})(Foo||={}); +var Foo;(function(a){let e;(function(e){foo(a,e)})(e=a.Bar||={})})(Foo||={}); ---------- /b.js ---------- -export var Foo;(function(e){let a;(function(p){foo(e,p)})(a=e.Bar||={})})(Foo||={}); +export var Foo;(function(a){let e;(function(e){foo(a,e)})(e=a.Bar||={})})(Foo||={}); ================================================================================ TestTSMinifyNamespaceNoLogicalAssignment ---------- /a.js ---------- -var Foo;(e=>{let a;(p=>foo(e,p))(a=e.Bar||(e.Bar={}))})(Foo||(Foo={})); +var Foo;(a=>{let e;(e=>foo(a,e))(e=a.Bar||(a.Bar={}))})(Foo||(Foo={})); ---------- /b.js ---------- -export var Foo;(e=>{let a;(p=>foo(e,p))(a=e.Bar||(e.Bar={}))})(Foo||(Foo={})); +export var Foo;(a=>{let e;(e=>foo(a,e))(e=a.Bar||(a.Bar={}))})(Foo||(Foo={})); ================================================================================ TestTSMinifyNestedEnum diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go index f8d3fe32f5b..29143496972 100644 --- a/internal/js_ast/js_ast.go +++ b/internal/js_ast/js_ast.go @@ -1287,6 +1287,9 @@ type Scope struct { Replaced []ScopeMember Generated []ast.Ref + // Record symbols used in the current scope + Used map[ast.Ref]uint32 + // The location of the "use strict" directive for ExplicitStrictMode UseStrictLoc logger.Loc diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 444cc144ef2..890f93b62c9 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -1619,6 +1619,13 @@ func (p *parser) recordUsage(ref ast.Ref) { use := p.symbolUses[ref] use.CountEstimate++ p.symbolUses[ref] = use + + if p.currentScope != nil { + if p.currentScope.Used == nil { + p.currentScope.Used = make(map[ast.Ref]uint32) + } + p.currentScope.Used[ref]++ + } } // The correctness of TypeScript-to-JavaScript conversion relies on accurate @@ -1640,6 +1647,16 @@ func (p *parser) ignoreUsage(ref ast.Ref) { } else { p.symbolUses[ref] = use } + + if p.currentScope != nil && p.currentScope.Used != nil { + use := p.currentScope.Used[ref] + use-- + if use == 0 { + delete(p.currentScope.Used, ref) + } else { + p.currentScope.Used[ref] = use + } + } } // Don't roll back the "tsUseCounts" increment. This must be counted even if diff --git a/internal/renamer/renamer.go b/internal/renamer/renamer.go index 15a9bf6db82..23cab6b8ba9 100644 --- a/internal/renamer/renamer.go +++ b/internal/renamer/renamer.go @@ -310,7 +310,214 @@ func (r *MinifyRenamer) AssignNamesByFrequency(minifier *ast.NameMinifier) { } } -// Returns the number of nested slots +type SymbolsInfo struct { + // Maps scopes to their enclosed symbols (symbols used but not declared in that scope) + enclosed map[*js_ast.Scope][]ast.Ref + + // Maps symbols to the scope they were declared in + declared map[ast.Ref]*js_ast.Scope +} + +func calculateSymbolsInfo(moduleScope *js_ast.Scope, symbols []ast.Symbol) SymbolsInfo { + // Traverse the scope tree using depth-first search + // On the way down (pushing to stack), collect where each symbol is declared + // On the way up (popping from stack), collect which symbols are captured(enclosed) by each scope + // that are not declared in that scope (including all descendant scopes) + + info := SymbolsInfo{ + enclosed: make(map[*js_ast.Scope][]ast.Ref), + declared: make(map[ast.Ref]*js_ast.Scope), + } + + var visit func(*js_ast.Scope) + visit = func(scope *js_ast.Scope) { + // Collect symbols declared in this scope + // As we traverse top-down, when we encounter the same symbol multiple times, + // we can consider the first encountered scope as the declare scope. + // This is because if a symbol is hoisted, we will always encounter the hoisted one first. + for _, member := range scope.Members { + symbol := symbols[member.Ref.InnerIndex] + if symbol.Kind == ast.SymbolUnbound { + continue + } + if _, exists := info.declared[member.Ref]; !exists { + info.declared[member.Ref] = scope + } + } + for _, ref := range scope.Generated { + symbol := symbols[ref.InnerIndex] + if symbol.Kind == ast.SymbolUnbound { + continue + } + if _, exists := info.declared[ref]; !exists { + info.declared[ref] = scope + } + } + + // Visit child scopes + for _, child := range scope.Children { + visit(child) + } + + // Collect enclosed symbols + // enclosed symbols = + // used symbols + // + generated symbols (generated symbols may be hoisted and not be recorded as used in the scope) + // + members + // + enclosed symbols from children + // - symbols declared in this scope + + enclosedSymbols := make(map[ast.Ref]struct{}) + // Collect used symbols + for ref := range scope.Used { + // For unbounded symbols, declareScope is nil + if declareScope := info.declared[ref]; declareScope != scope { + enclosedSymbols[ref] = struct{}{} + } + } + // Collect generated symbols + for _, ref := range scope.Generated { + if declareScope := info.declared[ref]; declareScope != scope { + enclosedSymbols[ref] = struct{}{} + } + } + // Collect members + for _, member := range scope.Members { + if declareScope := info.declared[member.Ref]; declareScope != scope { + enclosedSymbols[member.Ref] = struct{}{} + } + } + for _, child := range scope.Children { + for _, ref := range info.enclosed[child] { + if declareScope := info.declared[ref]; declareScope != scope { + enclosedSymbols[ref] = struct{}{} + } + } + } + + enclosed := make([]ast.Ref, 0, len(enclosedSymbols)) + for ref := range enclosedSymbols { + enclosed = append(enclosed, ref) + } + info.enclosed[scope] = enclosed + } + + visit(moduleScope) + return info + +} + +type slotAssigner struct { + slots ast.SlotCounts + + // cursor tracks our position in the enclosed symbols array when assigning + // slots for ast.SlotDefault namespace. Starts at 1. + // 0 means there is no enclosed symbol so we can assign slots to any number. + cursor int + + // enclosed contains the slot numbers already assigned to enclosed symbols, + // sorted in ascending order + enclosed []int +} + +func newSlotAssigner(symbols []ast.Symbol, enclosed []ast.Ref, assignedSlots ast.SlotCounts) slotAssigner { + enclosedSlots := make([]int, 0, len(enclosed)) + + for _, ref := range enclosed { + // Because symbols with a link will eventually be output with the same name + // as the linked symbols, the slot of the enclosed symbol can be considered as the + // linked one. + // + // Note: here assumes that the linked symbol is always assigned before. + ref = followSymbols(symbols, ref) + + symbol := &symbols[ref.InnerIndex] + // slot is always valid because enclosed symbols are already handled by parent scopes + slot := symbol.NestedScopeSlot.GetIndex() + if ns := symbol.SlotNamespace(); ns != ast.SlotMustNotBeRenamed { + enclosedSlots = append(enclosedSlots, int(slot)) + } + } + sort.Ints(enclosedSlots) + + cursor := 0 + if len(enclosedSlots) > 0 { + cursor = 1 + } + + slots := [4]uint32{ + ast.SlotDefault: 0, + ast.SlotLabel: assignedSlots[ast.SlotLabel], + ast.SlotPrivateName: assignedSlots[ast.SlotPrivateName], + ast.SlotMangledProp: assignedSlots[ast.SlotMangledProp], + } + + return slotAssigner{ + cursor: cursor, + enclosed: enclosedSlots, + slots: slots, + } +} + +func followSymbols(symbols []ast.Symbol, ref ast.Ref) ast.Ref { + symbol := &symbols[ref.InnerIndex] + if symbol.Link == ast.InvalidRef { + return ref + } + + link := followSymbols(symbols, symbol.Link) + + // Only write if needed to avoid concurrent map update hazards + if symbol.Link != link { + symbol.Link = link + } + + return link +} + +// AssignSlot assigns a slot for a symbol according to its namespace. +// Slots are incremental in each scope with certain rules: +// +// - For namespace ast.SlotDefault, slots should be different from enclosed symbols. +// - For other namespaces, slots should follow the slots of outer scope. +func (sa *slotAssigner) AssignSlot(ns ast.SlotNamespace) ast.Index32 { + current := sa.slots[ns] + + if ns == ast.SlotDefault { + cursor := sa.cursor + var upperBound uint32 + + for { + if cursor != 0 { + upperBound = uint32(sa.enclosed[cursor-1]) + } + + if cursor == 0 || current < upperBound { + break + } + // Current equals to the enclosed slot the cursor pointed, + // so increase the current and check with the next enclosed slot. + current++ + cursor++ + + // All enclosed slots have been checked(excluded from the search space), + // so we can ignore them then. + if cursor > len(sa.enclosed) { + cursor = 0 + } + } + + sa.cursor = cursor + sa.slots[ns] = current + 1 + } else { + sa.slots[ns]++ + } + + return ast.MakeIndex32(current) +} + +// AssignNestedScopeSlots traverses the scope tree starting at moduleScope and assigns +// slot numbers to all symbols in nested scopes. It returns the total slot counts needed. func AssignNestedScopeSlots(moduleScope *js_ast.Scope, symbols []ast.Symbol) (slotCounts ast.SlotCounts) { // Temporarily set the nested scope slots of top-level symbols to valid so // they aren't renamed in nested scopes. This prevents us from accidentally @@ -325,9 +532,9 @@ func AssignNestedScopeSlots(moduleScope *js_ast.Scope, symbols []ast.Symbol) (sl symbols[ref.InnerIndex].NestedScopeSlot = validSlot } - // Assign nested scope slots independently for each nested scope + info := calculateSymbolsInfo(moduleScope, symbols) for _, child := range moduleScope.Children { - slotCounts.UnionMax(assignNestedScopeSlotsHelper(child, symbols, ast.SlotCounts{})) + slotCounts.UnionMax(assignNestedScopeSlots(child, info, symbols, ast.SlotCounts{})) } // Then set the nested scope slots of top-level symbols back to zero. Top- @@ -341,44 +548,46 @@ func AssignNestedScopeSlots(moduleScope *js_ast.Scope, symbols []ast.Symbol) (sl return } -func assignNestedScopeSlotsHelper(scope *js_ast.Scope, symbols []ast.Symbol, slot ast.SlotCounts) ast.SlotCounts { +func assignNestedScopeSlots(scope *js_ast.Scope, info SymbolsInfo, symbols []ast.Symbol, assignedSlots ast.SlotCounts) ast.SlotCounts { + slotAllocator := newSlotAssigner(symbols, info.enclosed[scope], assignedSlots) + // Sort member map keys for determinism sortedMembers := make([]int, 0, len(scope.Members)) for _, member := range scope.Members { - sortedMembers = append(sortedMembers, int(member.Ref.InnerIndex)) + // do not rename enclosed symbols + if declareScope := info.declared[member.Ref]; declareScope == scope { + sortedMembers = append(sortedMembers, int(member.Ref.InnerIndex)) + } } sort.Ints(sortedMembers) // Assign slots for this scope's symbols. Only do this if the slot is - // not already assigned. Nested scopes have copies of symbols from parent - // scopes and we want to use the slot from the parent scope, not child scopes. + // not already assigned. for _, innerIndex := range sortedMembers { symbol := &symbols[innerIndex] if ns := symbol.SlotNamespace(); ns != ast.SlotMustNotBeRenamed && !symbol.NestedScopeSlot.IsValid() { - symbol.NestedScopeSlot = ast.MakeIndex32(slot[ns]) - slot[ns]++ + symbol.NestedScopeSlot = slotAllocator.AssignSlot(ns) } } for _, ref := range scope.Generated { symbol := &symbols[ref.InnerIndex] if ns := symbol.SlotNamespace(); ns != ast.SlotMustNotBeRenamed && !symbol.NestedScopeSlot.IsValid() { - symbol.NestedScopeSlot = ast.MakeIndex32(slot[ns]) - slot[ns]++ + symbol.NestedScopeSlot = slotAllocator.AssignSlot(ns) } } // Labels are always declared in a nested scope, so we don't need to check. if scope.Label.Ref != ast.InvalidRef { symbol := &symbols[scope.Label.Ref.InnerIndex] - symbol.NestedScopeSlot = ast.MakeIndex32(slot[ast.SlotLabel]) - slot[ast.SlotLabel]++ + symbol.NestedScopeSlot = slotAllocator.AssignSlot(ast.SlotLabel) } // Assign slots for the symbols of child scopes - slotCounts := slot + slotCounts := slotAllocator.slots for _, child := range scope.Children { - slotCounts.UnionMax(assignNestedScopeSlotsHelper(child, symbols, slot)) + slotCounts.UnionMax(assignNestedScopeSlots(child, info, symbols, slotAllocator.slots)) } + return slotCounts } From 390546715a5a383344ac1e7c64a29c32c5ea131f Mon Sep 17 00:00:00 2001 From: unbyte Date: Sat, 16 Nov 2024 17:33:26 +0800 Subject: [PATCH 2/2] feat: inline vars and rewrite useless var declarations --- .../bundler_tests/snapshots/snapshots_dce.txt | 3 - internal/js_parser/js_parser.go | 127 +++++++++++++++--- internal/js_parser/js_parser_test.go | 20 +-- scripts/uglify-tests.js | 3 + 4 files changed, 124 insertions(+), 29 deletions(-) diff --git a/internal/bundler_tests/snapshots/snapshots_dce.txt b/internal/bundler_tests/snapshots/snapshots_dce.txt index 89dba531d75..91e23e62990 100644 --- a/internal/bundler_tests/snapshots/snapshots_dce.txt +++ b/internal/bundler_tests/snapshots/snapshots_dce.txt @@ -215,9 +215,6 @@ x = [1, 1]; ---------- /out/exprs-before.js ---------- function nested() { - const x = [, "", {}, 0n, /./, function() { - }, () => { - }]; function foo() { return 1; } diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go index 890f93b62c9..7cb80c2e9dd 100644 --- a/internal/js_parser/js_parser.go +++ b/internal/js_parser/js_parser.go @@ -1262,6 +1262,19 @@ func (p *parser) mergeSymbols(old ast.Ref, new ast.Ref) ast.Ref { return new } +// This is similar to "ast.FollowSymbols" but it works with this parser's +// one-level symbol map instead of the linker's two-level symbol map. +func (p *parser) followSymbol(ref ast.Ref) ast.Ref { + symbol := &p.symbols[ref.InnerIndex] + if symbol.Link == ast.InvalidRef { + return ref + } + + link := p.followSymbol(symbol.Link) + + return link +} + type mergeResult int const ( @@ -8838,6 +8851,10 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt } } + // Optimize "var" declarations only in ScopeFunctionBody since "var"s have function-level scope + // and we may not have visited all of their uses yet if we are inside a block. + safeToOptimizeVars := p.currentScope.Kind == js_ast.ScopeFunctionBody + // Merge adjacent statements during mangling result := make([]js_ast.Stmt, 0, len(stmts)) isControlFlowDead := false @@ -8877,12 +8894,10 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt // return fn().prop; // for len(result) > 0 { - // Ignore "var" declarations since those have function-level scope and - // we may not have visited all of their uses yet by this point. We - // should have visited all the uses of "let" and "const" declarations + // We should have visited all the uses of "let" and "const" declarations // by now since they are scoped to this block which we just finished // visiting. - if prevS, ok := result[len(result)-1].Data.(*js_ast.SLocal); ok && prevS.Kind != js_ast.LocalVar { + if prevS, ok := result[len(result)-1].Data.(*js_ast.SLocal); ok && (safeToOptimizeVars || prevS.Kind != js_ast.LocalVar) { last := prevS.Decls[len(prevS.Decls)-1] // The binding must be an identifier that is only used once. @@ -8894,7 +8909,25 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt // case there is actually more than one use even though it says // there is only one. The "__name" use isn't counted so that // tree shaking still works when names are kept. - if symbol := p.symbols[id.Ref.InnerIndex]; symbol.UseCountEstimate == 1 && !symbol.Flags.Has(ast.DidKeepName) { + if symbol := &p.symbols[id.Ref.InnerIndex]; symbol.UseCountEstimate == 1 && !symbol.Flags.Has(ast.DidKeepName) { + // "var" declarations can be inlined only when not linked to function arguments. + // + // function fn(arg) { + // ^^^ + // var arg = 1 + // ^^^ should keep this declaration + // return [arguments, arg] + // ^^^^^^^^^ + // } + // + if prevS.Kind == js_ast.LocalVar { + // the currentScope is ScopeFunctionBody, and currentScope.Parent is ScopeFunctionArgs. + // check if the symbol is linked to an argument + if arg, ok := p.currentScope.Parent.Members[symbol.OriginalName]; ok && p.followSymbol(arg.Ref) == id.Ref { + break + } + } + replacement := last.ValueOrNil // The variable must be initialized, since we will be substituting @@ -8933,6 +8966,63 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt continue case *js_ast.SLocal: + if p.currentScope != p.moduleScope && !p.currentScope.ContainsDirectEval { + isVar := s.Kind == js_ast.LocalVar + + if !isVar || safeToOptimizeVars { + newDecls := make([]js_ast.Decl, 0) + newExprs := make([]js_ast.Expr, 0) + for _, decl := range s.Decls { + if id, ok := decl.Binding.Data.(*js_ast.BIdentifier); ok { + mustKeep := false + if isVar { + // the currentScope is ScopeFunctionBody, and currentScope.Parent is ScopeFunctionArgs. + // check if the symbol is linked to an argument + if arg, ok := p.currentScope.Parent.Members[p.symbols[id.Ref.InnerIndex].OriginalName]; ok && p.followSymbol(arg.Ref) == id.Ref { + mustKeep = true + } + } + if !mustKeep && !p.isSymbolUsed(id.Ref, isVar) { + if decl.ValueOrNil.Data == nil || p.astHelpers.ExprCanBeRemovedIfUnused(decl.ValueOrNil) { + // a declaration can be removed if it is empty or if its initializer is side-effect free + // // before + // var a = 1, b = 2; + // return a; + // + // // after + // var a = 1; + // return a; + continue + } + // save those declarations that cannot be removed because their initializer + // is not side-effect free + newExprs = append(newExprs, decl.ValueOrNil) + } + } + // save those declarations that cannot be removed + newDecls = append(newDecls, decl) + } + // newDecls is the superset of newExprs, so when newDecls is empty the entire + // statement can be removed safely. + if len(newDecls) == 0 { + continue + } + // newDecls and newExprs have the same length only when all newDecls are + // unused and we can replace the original statement with a new comma-seperated + // expression + if len(newDecls) == len(newExprs) { + expr := js_ast.JoinAllWithComma(newExprs) + result = append(result, js_ast.Stmt{ + Data: &js_ast.SExpr{Value: expr}, + Loc: stmt.Loc, + }) + continue + } + // else keep the original statement + s.Decls = newDecls + } + } + // Merge adjacent local statements if len(result) > 0 { prevStmt := result[len(result)-1] @@ -9319,6 +9409,20 @@ func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt return result } +func (p *parser) isSymbolUsed(ref ast.Ref, checkMerged bool) bool { + for { + symbol := p.symbols[ref.InnerIndex] + if symbol.UseCountEstimate > 0 { + return true + } + if !checkMerged || symbol.Link == ast.InvalidRef { + break + } + ref = symbol.Link + } + return false +} + func (p *parser) substituteSingleUseSymbolInStmt(stmt js_ast.Stmt, ref ast.Ref, replacement js_ast.Expr) bool { var expr *js_ast.Expr @@ -16912,13 +17016,7 @@ func (p *parser) appendPart(parts []js_ast.Part, stmts []js_ast.Stmt) []js_ast.P alreadyDeclared := make(map[ast.Ref]bool) for _, local := range p.relocatedTopLevelVars { // Follow links because "var" declarations may be merged due to hoisting - for { - link := p.symbols[local.Ref.InnerIndex].Link - if link == ast.InvalidRef { - break - } - local.Ref = link - } + local.Ref = p.followSymbol(local.Ref) // Only declare a given relocated variable once if !alreadyDeclared[local.Ref] { @@ -17946,10 +18044,7 @@ func (p *parser) toAST(before, parts, after []js_ast.Part, hashbang string, dire // If this symbol was merged, use the symbol at the end of the // linked list in the map. This is the case for multiple "var" // declarations with the same name, for example. - ref := declared.Ref - for p.symbols[ref.InnerIndex].Link != ast.InvalidRef { - ref = p.symbols[ref.InnerIndex].Link - } + ref := p.followSymbol(declared.Ref) p.topLevelSymbolToParts[ref] = append( p.topLevelSymbolToParts[ref], uint32(partIndex)) } diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go index 732ed029f6a..25a7ef21eb8 100644 --- a/internal/js_parser/js_parser_test.go +++ b/internal/js_parser/js_parser_test.go @@ -3481,7 +3481,7 @@ func TestMangleLoopJump(t *testing.T) { expectPrintedMangle(t, "while (x) { y(); if (1) break; z(); }", "for (; x; ) {\n y();\n break;\n}\n") expectPrintedMangle(t, "while (x) { y(); if (1) continue; z(); }", "for (; x; )\n y();\n") expectPrintedMangle(t, "while (x) { y(); debugger; if (1) continue; z(); }", "for (; x; ) {\n y();\n debugger;\n}\n") - expectPrintedMangle(t, "while (x) { let y = z(); if (1) continue; z(); }", "for (; x; ) {\n let y = z();\n}\n") + expectPrintedMangle(t, "while (x) { let y = z(); if (1) continue; z(); }", "for (; x; )\n z();\n") expectPrintedMangle(t, "while (x) { debugger; if (y) { if (1) break; z() } }", "for (; x; ) {\n debugger;\n if (y)\n break;\n}\n") expectPrintedMangle(t, "while (x) { debugger; if (y) { if (1) continue; z() } }", "for (; x; ) {\n debugger;\n y;\n}\n") expectPrintedMangle(t, "while (x) { debugger; if (1) { if (1) break; z() } }", "for (; x; ) {\n debugger;\n break;\n}\n") @@ -3560,8 +3560,8 @@ func TestMangleIndex(t *testing.T) { func TestMangleBlock(t *testing.T) { expectPrintedMangle(t, "while(1) { while (1) {} }", "for (; ; )\n for (; ; )\n ;\n") - expectPrintedMangle(t, "while(1) { const x = y; }", "for (; ; ) {\n const x = y;\n}\n") - expectPrintedMangle(t, "while(1) { let x; }", "for (; ; ) {\n let x;\n}\n") + expectPrintedMangle(t, "while(1) { const x = y; }", "for (; ; )\n y;\n") + expectPrintedMangle(t, "while(1) { let x; }", "for (; ; )\n ;\n") expectPrintedMangle(t, "while(1) { var x; }", "for (; ; )\n var x;\n") expectPrintedMangle(t, "while(1) { class X {} }", "for (; ; ) {\n class X {\n }\n}\n") expectPrintedMangle(t, "while(1) { function x() {} }", "for (; ; )\n var x = function() {\n };\n") @@ -3918,17 +3918,17 @@ func TestMangleIf(t *testing.T) { expectPrintedNormalAndMangle(t, "if (a) if (b) throw c", "if (a) {\n if (b) throw c;\n}\n", "if (a && b) throw c;\n") expectPrintedMangle(t, "if (true) { let a = b; if (c) throw d }", - "{\n let a = b;\n if (c) throw d;\n}\n") + "if (b, c) throw d;\n") expectPrintedMangle(t, "if (true) { if (a) throw b; if (c) throw d }", "if (a) throw b;\nif (c) throw d;\n") expectPrintedMangle(t, "if (false) throw a; else { let b = c; if (d) throw e }", - "{\n let b = c;\n if (d) throw e;\n}\n") + "if (c, d) throw e;\n") expectPrintedMangle(t, "if (false) throw a; else { if (b) throw c; if (d) throw e }", "if (b) throw c;\nif (d) throw e;\n") expectPrintedMangle(t, "if (a) { if (b) throw c; else { let d = e; if (f) throw g } }", - "if (a) {\n if (b) throw c;\n {\n let d = e;\n if (f) throw g;\n }\n}\n") + "if (a) {\n if (b) throw c;\n if (e, f) throw g;\n}\n") expectPrintedMangle(t, "if (a) { if (b) throw c; else if (d) throw e; else if (f) throw g }", "if (a) {\n if (b) throw c;\n if (d) throw e;\n if (f) throw g;\n}\n") @@ -5103,7 +5103,7 @@ func TestMangleInlineLocals(t *testing.T) { "function wrapper(arg0, arg1) {"+strings.ReplaceAll("\n"+b, "\n", "\n ")+"\n}\n") } - check("var x = 1; return x", "var x = 1;\nreturn x;") + check("var x = 1; return x", "return 1;") check("let x = 1; return x", "return 1;") check("const x = 1; return x", "return 1;") @@ -5190,7 +5190,7 @@ func TestMangleInlineLocals(t *testing.T) { check("let x = arg0; return y ? x : z;", "let x = arg0;\nreturn y ? x : z;") check("let x = arg0; return y ? z : x;", "let x = arg0;\nreturn y ? z : x;") check("let x = arg0; return (arg1 ? 1 : 2) ? x : 3;", "return arg0;") - check("let x = arg0; return (arg1 ? 1 : 2) ? 3 : x;", "let x = arg0;\nreturn 3;") + check("let x = arg0; return (arg1 ? 1 : 2) ? 3 : x;", "return 3;") check("let x = arg0; return (arg1 ? y : 1) ? x : 2;", "let x = arg0;\nreturn !arg1 || y ? x : 2;") check("let x = arg0; return (arg1 ? 1 : y) ? x : 2;", "let x = arg0;\nreturn arg1 || y ? x : 2;") check("let x = arg0; return (arg1 ? y : 1) ? 2 : x;", "let x = arg0;\nreturn !arg1 || y ? 2 : x;") @@ -5361,8 +5361,8 @@ func TestTrimCodeInDeadControlFlow(t *testing.T) { expectPrintedMangle(t, "if (1) a(); else { switch (1) { case 1: b() } }", "a();\n") expectPrintedMangle(t, "if (0) {let a = 1} else a()", "a();\n") - expectPrintedMangle(t, "if (1) {let a = 1} else a()", "{\n let a = 1;\n}\n") - expectPrintedMangle(t, "if (0) a(); else {let a = 1}", "{\n let a = 1;\n}\n") + expectPrintedMangle(t, "if (1) {let a = 1} else a()", "") + expectPrintedMangle(t, "if (0) a(); else {let a = 1}", "") expectPrintedMangle(t, "if (1) a(); else {let a = 1}", "a();\n") expectPrintedMangle(t, "if (1) a(); else { var a = b }", "if (1) a();\nelse\n var a;\n") diff --git a/scripts/uglify-tests.js b/scripts/uglify-tests.js index b565d7cb41c..584d5e40d7d 100644 --- a/scripts/uglify-tests.js +++ b/scripts/uglify-tests.js @@ -267,6 +267,9 @@ async function test_case(esbuild, test, basename) { 'classes.js: issue_4722_1', 'classes.js: issue_4722_2', 'classes.js: issue_4722_3', + + // // Corner case of dead zones that inhibits optimizations + 'let.js: issue_4276_1', ].indexOf(`${basename}: ${test.name}`) >= 0 if (!sandbox.same_stdout(test.expect_stdout, actual)) {