From 49fbe9c5ac090b2082ebd4702772af6ccafa13cd Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Mon, 13 Nov 2017 07:37:42 +0800 Subject: [PATCH] fix replacement logic in `collapse_vars` (#2475) --- lib/compress.js | 216 +++++++++++++++++---------------- test/compress/collapse_vars.js | 60 +++++++++ 2 files changed, 174 insertions(+), 102 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index 0635c41d..a50dd52b 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -839,6 +839,113 @@ merge(Compressor.prototype, { var args; var candidates = []; var stat_index = statements.length; + var scanner = new TreeTransformer(function(node, descend) { + if (abort) return node; + // Skip nodes before `candidate` as quickly as possible + if (!hit) { + if (node === candidate) { + hit = true; + return node; + } + return; + } + // Stop immediately if these node types are encountered + var parent = scanner.parent(); + if (node instanceof AST_Assign && node.operator != "=" && lhs.equivalent_to(node.left) + || node instanceof AST_Call && lhs instanceof AST_PropAccess && lhs.equivalent_to(node.expression) + || node instanceof AST_Debugger + || node instanceof AST_IterationStatement && !(node instanceof AST_For) + || node instanceof AST_SymbolRef && !node.is_declared(compressor) + || node instanceof AST_Try + || node instanceof AST_With + || parent instanceof AST_For && node !== parent.init) { + abort = true; + return node; + } + // Replace variable with assignment when found + if (can_replace + && !(node instanceof AST_SymbolDeclaration) + && lhs.equivalent_to(node)) { + if (is_lhs(node, parent)) { + if (candidate.multiple) replaced++; + return node; + } + CHANGED = abort = true; + replaced++; + compressor.info("Collapsing {name} [{file}:{line},{col}]", { + name: node.print_to_string(), + file: node.start.file, + line: node.start.line, + col: node.start.col + }); + if (candidate instanceof AST_UnaryPostfix) { + return make_node(AST_UnaryPrefix, candidate, candidate); + } + if (candidate instanceof AST_VarDef) { + if (candidate.multiple) { + abort = false; + return node; + } + var def = candidate.name.definition(); + if (def.references.length - def.replaced == 1 && !compressor.exposed(def)) { + def.replaced++; + return maintain_this_binding(parent, node, candidate.value); + } + return make_node(AST_Assign, candidate, { + operator: "=", + left: make_node(AST_SymbolRef, candidate.name, candidate.name), + right: candidate.value + }); + } + candidate.write_only = false; + return candidate; + } + // These node types have child nodes that execute sequentially, + // but are otherwise not safe to scan into or beyond them. + var sym; + if (node instanceof AST_Call + || node instanceof AST_Exit + || node instanceof AST_PropAccess + && (side_effects || node.expression.may_throw_on_access(compressor)) + || node instanceof AST_SymbolRef + && (lvalues[node.name] + || side_effects && !references_in_scope(node.definition())) + || (sym = lhs_or_def(node)) + && (sym instanceof AST_PropAccess || sym.name in lvalues) + || (side_effects || !replace_all) + && (parent instanceof AST_Binary && lazy_op(parent.operator) + || parent instanceof AST_Case + || parent instanceof AST_Conditional + || parent instanceof AST_If)) { + if (!(node instanceof AST_Scope)) descend(node, scanner); + abort = true; + return node; + } + // Skip (non-executed) functions and (leading) default case in switch statements + if (node instanceof AST_Default || node instanceof AST_Scope) return node; + }); + var multi_replacer = new TreeTransformer(function(node) { + if (abort) return node; + // Skip nodes before `candidate` as quickly as possible + if (!hit) { + if (node === candidate) { + hit = true; + return node; + } + return; + } + // Replace variable when found + if (node instanceof AST_SymbolRef + && node.name == def.name) { + if (!--replaced) abort = true; + if (is_lhs(node, multi_replacer.parent())) return node; + def.replaced++; + value_def.replaced--; + return candidate.value; + } + // Skip (non-executed) functions and (leading) default case in switch statements + if (node instanceof AST_Default || node instanceof AST_Scope) return node; + }); while (--stat_index >= 0) { // Treat parameters as collapsible in IIFE, i.e. // function(a, b){ ... }(x()); @@ -862,121 +969,24 @@ merge(Compressor.prototype, { var side_effects = value_has_side_effects(candidate); var hit = candidate.name instanceof AST_SymbolFunarg; var abort = false, replaced = 0, can_replace = !args || !hit; - var tt = new TreeTransformer(function(node, descend) { - if (abort) return node; - // Skip nodes before `candidate` as quickly as possible - if (!hit) { - if (node === candidate) { - hit = true; - return node; - } - return; - } - // Stop immediately if these node types are encountered - var parent = tt.parent(); - if (node instanceof AST_Assign && node.operator != "=" && lhs.equivalent_to(node.left) - || node instanceof AST_Call && lhs instanceof AST_PropAccess && lhs.equivalent_to(node.expression) - || node instanceof AST_Debugger - || node instanceof AST_IterationStatement && !(node instanceof AST_For) - || node instanceof AST_SymbolRef && !node.is_declared(compressor) - || node instanceof AST_Try - || node instanceof AST_With - || parent instanceof AST_For && node !== parent.init) { - abort = true; - return node; - } - // Replace variable with assignment when found - if (can_replace - && !(node instanceof AST_SymbolDeclaration) - && !is_lhs(node, parent) - && lhs.equivalent_to(node)) { - CHANGED = abort = true; - replaced++; - compressor.info("Collapsing {name} [{file}:{line},{col}]", { - name: node.print_to_string(), - file: node.start.file, - line: node.start.line, - col: node.start.col - }); - if (candidate instanceof AST_UnaryPostfix) { - return make_node(AST_UnaryPrefix, candidate, candidate); - } - if (candidate instanceof AST_VarDef) { - if (candidate.multiple) { - abort = false; - return node; - } - var def = candidate.name.definition(); - if (def.references.length - def.replaced == 1 && !compressor.exposed(def)) { - def.replaced++; - return maintain_this_binding(parent, node, candidate.value); - } - return make_node(AST_Assign, candidate, { - operator: "=", - left: make_node(AST_SymbolRef, candidate.name, candidate.name), - right: candidate.value - }); - } - candidate.write_only = false; - return candidate; - } - // These node types have child nodes that execute sequentially, - // but are otherwise not safe to scan into or beyond them. - var sym; - if (node instanceof AST_Call - || node instanceof AST_Exit - || node instanceof AST_PropAccess - && (side_effects || node.expression.may_throw_on_access(compressor)) - || node instanceof AST_SymbolRef - && (lvalues[node.name] - || side_effects && !references_in_scope(node.definition())) - || (sym = lhs_or_def(node)) - && (sym instanceof AST_PropAccess || sym.name in lvalues) - || (side_effects || !replace_all) - && (parent instanceof AST_Binary && lazy_op(parent.operator) - || parent instanceof AST_Case - || parent instanceof AST_Conditional - || parent instanceof AST_If)) { - if (!(node instanceof AST_Scope)) descend(node, tt); - abort = true; - return node; - } - // Skip (non-executed) functions and (leading) default case in switch statements - if (node instanceof AST_Default || node instanceof AST_Scope) return node; - }); if (!can_replace) { for (var j = compressor.self().argnames.lastIndexOf(candidate.name) + 1; !abort && j < args.length; j++) { - args[j].transform(tt); + args[j].transform(scanner); } can_replace = true; } for (var i = stat_index; !abort && i < statements.length; i++) { - statements[i].transform(tt); + statements[i].transform(scanner); } if (candidate.multiple) { var def = candidate.name.definition(); - if (abort && def.references.length > replaced) replaced = false; + if (abort && def.references.length - def.replaced > replaced) replaced = false; else { abort = false; hit = candidate.name instanceof AST_SymbolFunarg; var value_def = candidate.value.definition(); for (var i = stat_index; !abort && i < statements.length; i++) { - statements[i].transform(new TreeTransformer(function(node) { - if (abort) return node; - if (!hit) { - if (node === candidate) { - hit = true; - return node; - } - return; - } - if (node instanceof AST_SymbolRef && node.name == def.name) { - def.replaced++; - value_def.replaced--; - if (!--replaced) abort = true; - return candidate.value; - } - })); + statements[i].transform(multi_replacer); } } } @@ -2549,12 +2559,14 @@ merge(Compressor.prototype, { var def = tail.pop(); compressor.warn("Converting duplicated definition of variable {name} to assignment [{file}:{line},{col}]", template(def.name)); remove(var_defs, def); - drop_decl(def.name.definition()); side_effects.unshift(make_node(AST_Assign, def, { operator: "=", left: make_node(AST_SymbolRef, def.name, def.name), right: def.value })); + def = def.name.definition(); + drop_decl(def); + def.replaced--; } } if (head.length > 0 || tail.length > 0) { diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index 402bd22b..10735b28 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -3518,3 +3518,63 @@ issue_2436_12: { } } } + +issue_2436_13: { + options = { + collapse_vars: true, + reduce_vars: true, + unused: true, + } + input: { + var a = "PASS"; + (function() { + function f(b) { + (function g(b) { + var b = b && (b.null = "FAIL"); + })(a); + } + f(); + })(); + console.log(a); + } + expect: { + var a = "PASS"; + (function() { + (function(b) { + (function(b) { + a && (a.null = "FAIL"); + })(); + })(); + })(); + console.log(a); + } + expect_stdout: "PASS" +} + +issue_2436_14: { + options = { + collapse_vars: true, + reduce_vars: true, + unused: true, + } + input: { + var a = "PASS"; + var b = {}; + (function() { + var c = a; + c && function(c, d) { + console.log(c, d); + }(b, c); + })(); + } + expect: { + var a = "PASS"; + var b = {}; + (function() { + a && function(c, d) { + console.log(c, d); + }(b, a); + })(); + } + expect_stdout: true +} -- 2.34.1