From c6cfa04d10c648dc1ccdf7ac6369f4162f0a46dc Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Sun, 12 Nov 2017 22:31:47 +0800 Subject: [PATCH 1/9] allow symbol replacement on multiple occurrences (#2472) - all-or-nothing replacement - avoid unmangleable names fixes #2436 --- lib/compress.js | 71 +++++- lib/scope.js | 2 + test/compress/collapse_vars.js | 420 +++++++++++++++++++++++++++++++++ 3 files changed, 481 insertions(+), 12 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index fdf3a2b6..54ac3d75 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -796,8 +796,8 @@ merge(Compressor.prototype, { }); function drop_decl(def) { - def._eliminiated = (def._eliminiated || 0) + 1; - if (def.orig.length == def._eliminiated) { + def.eliminated++; + if (def.orig.length == def.eliminated) { def.scope.functions.del(def.name); def.scope.variables.del(def.name); } @@ -854,10 +854,14 @@ merge(Compressor.prototype, { // Locate symbols which may execute code outside of scanning range var lvalues = get_lvalues(candidate); if (lhs instanceof AST_SymbolRef) lvalues[lhs.name] = false; - var one_off = lhs instanceof AST_Symbol && lhs.definition().references.length == 1; + var replace_all = candidate.multiple; + if (!replace_all && lhs instanceof AST_SymbolRef) { + var def = lhs.definition(); + replace_all = def.references.length - def.replaced == 1; + } var side_effects = value_has_side_effects(candidate); var hit = candidate.name instanceof AST_SymbolFunarg; - var abort = false, replaced = false, can_replace = !args || !hit; + 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 @@ -886,7 +890,8 @@ merge(Compressor.prototype, { && !(node instanceof AST_SymbolDeclaration) && !is_lhs(node, parent) && lhs.equivalent_to(node)) { - CHANGED = replaced = abort = true; + CHANGED = abort = true; + replaced++; compressor.info("Collapsing {name} [{file}:{line},{col}]", { name: node.print_to_string(), file: node.start.file, @@ -897,8 +902,13 @@ merge(Compressor.prototype, { 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 == 1 && !compressor.exposed(def)) { + 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, { @@ -922,7 +932,7 @@ merge(Compressor.prototype, { || side_effects && !references_in_scope(node.definition())) || (sym = lhs_or_def(node)) && (sym instanceof AST_PropAccess || sym.name in lvalues) - || (side_effects || !one_off) + || (side_effects || !replace_all) && (parent instanceof AST_Binary && lazy_op(parent.operator) || parent instanceof AST_Case || parent instanceof AST_Conditional @@ -935,7 +945,7 @@ merge(Compressor.prototype, { if (node instanceof AST_Default || node instanceof AST_Scope) return node; }); if (!can_replace) { - for (var j = compressor.self().argnames.lastIndexOf(candidate.name) + 1; j < args.length; j++) { + for (var j = compressor.self().argnames.lastIndexOf(candidate.name) + 1; !abort && j < args.length; j++) { args[j].transform(tt); } can_replace = true; @@ -943,6 +953,33 @@ merge(Compressor.prototype, { for (var i = stat_index; !abort && i < statements.length; i++) { statements[i].transform(tt); } + if (candidate.multiple) { + var def = candidate.name.definition(); + if (abort && def.references.length > 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; + } + })); + } + } + } if (replaced && !remove_candidate(candidate)) statements.splice(stat_index, 1); } } @@ -956,7 +993,7 @@ merge(Compressor.prototype, { && (iife = compressor.parent()) instanceof AST_Call && iife.expression === fn) { var fn_strict = compressor.has_directive("use strict"); - if (fn_strict && fn.body.indexOf(fn_strict) < 0) fn_strict = false; + if (fn_strict && !member(fn_strict, fn.body)) fn_strict = false; var len = fn.argnames.length; args = iife.args.slice(len); var names = Object.create(null); @@ -1012,12 +1049,22 @@ merge(Compressor.prototype, { } } + function mangleable_var(expr) { + var value = expr.value; + if (!(value instanceof AST_SymbolRef)) return false; + if (value.name == "arguments") return false; + if (value.definition().undeclared) return false; + expr.multiple = true; + return true; + } + function get_lhs(expr) { if (expr instanceof AST_VarDef) { var def = expr.name.definition(); - if (def.orig.length - (def._eliminiated || 0) > 1 - && !(expr.name instanceof AST_SymbolFunarg) - || def.references.length == 1 && !compressor.exposed(def)) { + var declared = def.orig.length - def.eliminated; + var referenced = def.references.length - def.replaced; + if (declared > 1 && !(expr.name instanceof AST_SymbolFunarg) + || (referenced > 1 ? mangleable_var(expr) : !compressor.exposed(def))) { return make_node(AST_SymbolRef, expr.name, expr.name); } } else { diff --git a/lib/scope.js b/lib/scope.js index 8e766a56..0d2a7aeb 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -46,8 +46,10 @@ function SymbolDef(scope, index, orig) { this.name = orig.name; this.orig = [ orig ]; + this.eliminated = 0; this.scope = scope; this.references = []; + this.replaced = 0; this.global = false; this.mangled_name = null; this.undeclared = false; diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index d98dca95..402bd22b 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -3098,3 +3098,423 @@ issue_2437: { }(); } } + +issue_2436_1: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + var o = { + a: 1, + b: 2, + }; + console.log({ + x: o.a, + y: o.b, + }); + } + expect_stdout: true +} + +issue_2436_2: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + o.a = 3; + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + o.a = 3; + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect_stdout: true +} + +issue_2436_3: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + o = { + a: 3, + b: 4, + }; + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + o = { + a: 3, + b: 4, + }; + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect_stdout: true +} + +issue_2436_4: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + var o; + }(o)); + } + expect: { + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }({ + a: 1, + b: 2, + })); + } + expect_stdout: true +} + +issue_2436_5: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(o) { + return { + x: o.a, + y: o.b, + }; + }(o)); + } + expect: { + console.log(function(o) { + return { + x: o.a, + y: o.b, + }; + }({ + a: 1, + b: 2, + })); + } + expect_stdout: true +} + +issue_2436_6: { + options = { + collapse_vars: true, + evaluate: true, + inline: true, + passes: 2, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + unsafe: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + console.log({ + x: 1, + y: 2, + }); + } + expect_stdout: true +} + +issue_2436_7: { + options = { + collapse_vars: true, + hoist_props: true, + inline: true, + passes: 3, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + console.log({ + x: 1, + y: 2, + }); + } + expect_stdout: true +} + +issue_2436_8: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect_stdout: true +} + +issue_2436_9: { + options = { + collapse_vars: true, + inline: true, + pure_getters: "strict", + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = console; + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect: { + var o = console; + console.log(function(c) { + return { + x: c.a, + y: c.b, + }; + }(o)); + } + expect_stdout: true +} + +issue_2436_10: { + options = { + collapse_vars: true, + inline: true, + pure_getters: true, + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + function f(n) { + o = { b: 3 }; + return n; + } + console.log(function(c) { + return [ + c.a, + f(c.b), + c.b, + ]; + }(o).join(" ")); + } + expect: { + var o = { + a: 1, + b: 2, + }; + function f(n) { + o = { b: 3 }; + return n; + } + console.log(function(c) { + return [ + c.a, + f(c.b), + c.b, + ]; + }(o).join(" ")); + } + expect_stdout: "1 2 2" +} + +issue_2436_11: { + options = { + collapse_vars: true, + join_vars: true, + reduce_vars: true, + unused: true, + } + input: { + function matrix() {} + function isCollection() {} + function _randomDataForMatrix() {} + function _randomInt() {} + function f(arg1, arg2) { + if (isCollection(arg1)) { + var size = arg1; + var max = arg2; + var min = 0; + var res = _randomDataForMatrix(size.valueOf(), min, max, _randomInt); + return size && true === size.isMatrix ? matrix(res) : res; + } else { + var min = arg1; + var max = arg2; + return _randomInt(min, max); + } + } + } + expect: { + function matrix() {} + function isCollection() {} + function _randomDataForMatrix() {} + function _randomInt() {} + function f(arg1, arg2) { + if (isCollection(arg1)) { + var size = arg1, max = arg2, min = 0, res = _randomDataForMatrix(size.valueOf(), min, max, _randomInt); + return size && true === size.isMatrix ? matrix(res) : res; + } else { + return _randomInt(min = arg1, max = arg2); + } + } + } +} + +issue_2436_12: { + options = { + collapse_vars: true, + unused: true, + } + input: { + function isUndefined() {} + function f() { + var viewValue = this.$$lastCommittedViewValue; + var modelValue = viewValue; + return isUndefined(modelValue) ? modelValue : null; + } + } + expect: { + function isUndefined() {} + function f() { + var modelValue = this.$$lastCommittedViewValue; + return isUndefined(modelValue) ? modelValue : null; + } + } +} From 2ac5086831f17ae93f1cfec9b7375b8ae19915ca Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Mon, 13 Nov 2017 00:59:41 +0800 Subject: [PATCH 2/9] fix `top_retain` on `hoist_props` (#2474) fixes #2473 --- lib/compress.js | 2 + test/compress/hoist_props.js | 89 ++++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/lib/compress.js b/lib/compress.js index 54ac3d75..0635c41d 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -2769,6 +2769,7 @@ merge(Compressor.prototype, { AST_Scope.DEFMETHOD("hoist_properties", function(compressor){ var self = this; if (!compressor.option("hoist_props") || compressor.has_directive("use asm")) return self; + var top_retain = self instanceof AST_Toplevel && compressor.top_retain || return_false; var defs_by_id = Object.create(null); var var_names = Object.create(null); self.enclosed.forEach(function(def) { @@ -2784,6 +2785,7 @@ merge(Compressor.prototype, { && !(def = sym.definition()).escaped && !def.single_use && !def.direct_access + && !top_retain(def) && (value = sym.fixed_value()) === node.value && value instanceof AST_Object) { var defs = new Dictionary(); diff --git a/test/compress/hoist_props.js b/test/compress/hoist_props.js index 1fa321cf..40d36ace 100644 --- a/test/compress/hoist_props.js +++ b/test/compress/hoist_props.js @@ -411,3 +411,92 @@ new_this: { } expect_stdout: "1 2" } + +issue_2473_1: { + options = { + hoist_props: false, + reduce_vars: true, + top_retain: [ "x", "y" ], + toplevel: true, + unused: true, + } + input: { + var x = {}; + var y = []; + var z = {}; + } + expect: { + var x = {}; + var y = []; + } +} + +issue_2473_2: { + options = { + hoist_props: true, + reduce_vars: true, + top_retain: [ "x", "y" ], + toplevel: true, + unused: true, + } + input: { + var x = {}; + var y = []; + var z = {}; + } + expect: { + var x = {}; + var y = []; + } +} + +issue_2473_3: { + options = { + hoist_props: true, + reduce_vars: true, + top_retain: "o", + toplevel: true, + unused: true, + } + input: { + var o = { + a: 1, + b: 2, + }; + console.log(o.a, o.b); + } + expect: { + var o = { + a: 1, + b: 2, + }; + console.log(o.a, o.b); + } + expect_stdout: "1 2" +} + +issue_2473_4: { + options = { + hoist_props: true, + reduce_vars: true, + top_retain: "o", + toplevel: true, + unused: true, + } + input: { + (function() { + var o = { + a: 1, + b: 2, + }; + console.log(o.a, o.b); + })(); + } + expect: { + (function() { + var o_a = 1, o_b = 2; + console.log(o_a, o_b); + })(); + } + expect_stdout: "1 2" +} 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 3/9] 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 +} From 557636f3b736c58e7830bec427bfe6eebadc0b68 Mon Sep 17 00:00:00 2001 From: kzc Date: Tue, 14 Nov 2017 03:03:25 -0500 Subject: [PATCH 4/9] update documentation for `reduce_funcs` (#2478) --- README.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 22831b12..1f63da75 100644 --- a/README.md +++ b/README.md @@ -689,11 +689,12 @@ If you're using the `X-SourceMap` header instead, you can just omit `sourceMap.u Specify `"strict"` to treat `foo.bar` as side-effect-free only when `foo` is certain to not throw, i.e. not `null` or `undefined`. -- `reduce_funcs` (default: `true`) -- Allows single-use functions - to be inlined as function expressions when permissible. - Enabled by default. Option depends on `reduce_vars` being enabled. - For speed critical code this option should be disabled. - +- `reduce_funcs` (default: `true`) -- Allows single-use functions to be + inlined as function expressions when permissible allowing further + optimization. Enabled by default. Option depends on `reduce_vars` + being enabled. Some code runs faster in the Chrome V8 engine if this + option is disabled. Does not negatively impact other major browsers. + - `reduce_vars` (default: `true`) -- Improve optimization on variables assigned with and used as constant values. From fa7a7c5c5aab369f6bcb11fbd9820e80ef0f2528 Mon Sep 17 00:00:00 2001 From: kzc Date: Tue, 14 Nov 2017 17:00:51 -0500 Subject: [PATCH 5/9] Update ISSUE_TEMPLATE.md (#2481) --- .github/ISSUE_TEMPLATE.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index c5343f3f..60b77c7c 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -15,6 +15,8 @@ UglifyJS alone - without third party tools or libraries. Ideally the input should be as small as possible. Post a link to a gist if necessary. + + Issues without a reproducible test case will be closed. --> **The `uglifyjs` CLI command executed or `minify()` options used.** From ebe761cad09343e514a7a02b591dbb93f651c888 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Thu, 16 Nov 2017 04:37:37 +0800 Subject: [PATCH 6/9] minor consolidations (#2484) - unique symbol generation - `unsafe` on `AST_Call` --- lib/compress.js | 256 +++++++++++++++++++++++++----------------------- 1 file changed, 133 insertions(+), 123 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index a50dd52b..0f72404f 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -2778,18 +2778,29 @@ merge(Compressor.prototype, { return self; }); + AST_Scope.DEFMETHOD("make_var_name", function(prefix) { + var var_names = this.var_names; + if (!var_names) { + this.var_names = var_names = Object.create(null); + this.enclosed.forEach(function(def) { + var_names[def.name] = true; + }); + this.variables.each(function(def, name) { + var_names[name] = true; + }); + } + prefix = prefix.replace(/[^a-z_$]+/ig, "_"); + var name = prefix; + for (var i = 0; var_names[name]; i++) name = prefix + "$" + i; + var_names[name] = true; + return name; + }); + AST_Scope.DEFMETHOD("hoist_properties", function(compressor){ var self = this; if (!compressor.option("hoist_props") || compressor.has_directive("use asm")) return self; var top_retain = self instanceof AST_Toplevel && compressor.top_retain || return_false; var defs_by_id = Object.create(null); - var var_names = Object.create(null); - self.enclosed.forEach(function(def) { - var_names[def.name] = true; - }); - self.variables.each(function(def, name) { - var_names[name] = true; - }); return self.transform(new TreeTransformer(function(node) { if (node instanceof AST_VarDef) { var sym = node.name, def, value; @@ -2829,17 +2840,13 @@ merge(Compressor.prototype, { } function make_sym(key) { - var prefix = sym.name + "_" + key.toString().replace(/[^a-z_$]+/ig, "_"); - var name = prefix; - for (var i = 0; var_names[name]; i++) name = prefix + "$" + i; var new_var = make_node(sym.CTOR, sym, { - name: name, + name: self.make_var_name(sym.name + "_" + key), scope: self }); var def = self.def_variable(new_var); defs.set(key, def); self.enclosed.push(def); - var_names[name] = true; return new_var; } })); @@ -3408,130 +3415,133 @@ merge(Compressor.prototype, { self.args.length = last; } if (compressor.option("unsafe")) { - if (is_undeclared_ref(exp)) { - switch (exp.name) { - case "Array": - if (self.args.length != 1) { - return make_node(AST_Array, self, { - elements: self.args - }).optimize(compressor); - } - break; - case "Object": - if (self.args.length == 0) { - return make_node(AST_Object, self, { - properties: [] - }); - } - break; - case "String": - if (self.args.length == 0) return make_node(AST_String, self, { - value: "" - }); - if (self.args.length <= 1) return make_node(AST_Binary, self, { - left: self.args[0], - operator: "+", - right: make_node(AST_String, self, { value: "" }) + if (is_undeclared_ref(exp)) switch (exp.name) { + case "Array": + if (self.args.length != 1) { + return make_node(AST_Array, self, { + elements: self.args }).optimize(compressor); - break; - case "Number": - if (self.args.length == 0) return make_node(AST_Number, self, { - value: 0 - }); - if (self.args.length == 1) return make_node(AST_UnaryPrefix, self, { - expression: self.args[0], - operator: "+" - }).optimize(compressor); - case "Boolean": - if (self.args.length == 0) return make_node(AST_False, self); - if (self.args.length == 1) return make_node(AST_UnaryPrefix, self, { - expression: make_node(AST_UnaryPrefix, self, { - expression: self.args[0], - operator: "!" - }), - operator: "!" - }).optimize(compressor); - break; } - } - else if (exp instanceof AST_Dot && exp.property == "toString" && self.args.length == 0) { - return make_node(AST_Binary, self, { + break; + case "Object": + if (self.args.length == 0) { + return make_node(AST_Object, self, { + properties: [] + }); + } + break; + case "String": + if (self.args.length == 0) return make_node(AST_String, self, { + value: "" + }); + if (self.args.length <= 1) return make_node(AST_Binary, self, { + left: self.args[0], + operator: "+", + right: make_node(AST_String, self, { value: "" }) + }).optimize(compressor); + break; + case "Number": + if (self.args.length == 0) return make_node(AST_Number, self, { + value: 0 + }); + if (self.args.length == 1) return make_node(AST_UnaryPrefix, self, { + expression: self.args[0], + operator: "+" + }).optimize(compressor); + case "Boolean": + if (self.args.length == 0) return make_node(AST_False, self); + if (self.args.length == 1) return make_node(AST_UnaryPrefix, self, { + expression: make_node(AST_UnaryPrefix, self, { + expression: self.args[0], + operator: "!" + }), + operator: "!" + }).optimize(compressor); + break; + } else if (exp instanceof AST_Dot) switch(exp.property) { + case "toString": + if (self.args.length == 0) return make_node(AST_Binary, self, { left: make_node(AST_String, self, { value: "" }), operator: "+", right: exp.expression }).optimize(compressor); - } - else if (exp instanceof AST_Dot && exp.expression instanceof AST_Array && exp.property == "join") EXIT: { - var separator; - if (self.args.length > 0) { - separator = self.args[0].evaluate(compressor); - if (separator === self.args[0]) break EXIT; // not a constant - } - var elements = []; - var consts = []; - exp.expression.elements.forEach(function(el) { - var value = el.evaluate(compressor); - if (value !== el) { - consts.push(value); - } else { - if (consts.length > 0) { - elements.push(make_node(AST_String, self, { - value: consts.join(separator) - })); - consts.length = 0; + break; + case "join": + if (exp.expression instanceof AST_Array) EXIT: { + var separator; + if (self.args.length > 0) { + separator = self.args[0].evaluate(compressor); + if (separator === self.args[0]) break EXIT; // not a constant + } + var elements = []; + var consts = []; + exp.expression.elements.forEach(function(el) { + var value = el.evaluate(compressor); + if (value !== el) { + consts.push(value); + } else { + if (consts.length > 0) { + elements.push(make_node(AST_String, self, { + value: consts.join(separator) + })); + consts.length = 0; + } + elements.push(el); } - elements.push(el); - } - }); - if (consts.length > 0) { - elements.push(make_node(AST_String, self, { - value: consts.join(separator) - })); - } - if (elements.length == 0) return make_node(AST_String, self, { value: "" }); - if (elements.length == 1) { - if (elements[0].is_string(compressor)) { - return elements[0]; - } - return make_node(AST_Binary, elements[0], { - operator : "+", - left : make_node(AST_String, self, { value: "" }), - right : elements[0] }); - } - if (separator == "") { - var first; - if (elements[0].is_string(compressor) - || elements[1].is_string(compressor)) { - first = elements.shift(); - } else { - first = make_node(AST_String, self, { value: "" }); + if (consts.length > 0) { + elements.push(make_node(AST_String, self, { + value: consts.join(separator) + })); } - return elements.reduce(function(prev, el){ - return make_node(AST_Binary, el, { + if (elements.length == 0) return make_node(AST_String, self, { value: "" }); + if (elements.length == 1) { + if (elements[0].is_string(compressor)) { + return elements[0]; + } + return make_node(AST_Binary, elements[0], { operator : "+", - left : prev, - right : el + left : make_node(AST_String, self, { value: "" }), + right : elements[0] }); - }, first).optimize(compressor); + } + if (separator == "") { + var first; + if (elements[0].is_string(compressor) + || elements[1].is_string(compressor)) { + first = elements.shift(); + } else { + first = make_node(AST_String, self, { value: "" }); + } + return elements.reduce(function(prev, el){ + return make_node(AST_Binary, el, { + operator : "+", + left : prev, + right : el + }); + }, first).optimize(compressor); + } + // need this awkward cloning to not affect original element + // best_of will decide which one to get through. + var node = self.clone(); + node.expression = node.expression.clone(); + node.expression.expression = node.expression.expression.clone(); + node.expression.expression.elements = elements; + return best_of(compressor, self, node); } - // need this awkward cloning to not affect original element - // best_of will decide which one to get through. - var node = self.clone(); - node.expression = node.expression.clone(); - node.expression.expression = node.expression.expression.clone(); - node.expression.expression.elements = elements; - return best_of(compressor, self, node); - } - else if (exp instanceof AST_Dot && exp.expression.is_string(compressor) && exp.property == "charAt") { - var arg = self.args[0]; - var index = arg ? arg.evaluate(compressor) : 0; - if (index !== arg) { - return make_node(AST_Sub, exp, { - expression: exp.expression, - property: make_node_from_constant(index | 0, arg || exp) - }).optimize(compressor); + break; + case "charAt": + if (exp.expression.is_string(compressor)) { + var arg = self.args[0]; + var index = arg ? arg.evaluate(compressor) : 0; + if (index !== arg) { + return make_node(AST_Sub, exp, { + expression: exp.expression, + property: make_node_from_constant(index | 0, arg || exp) + }).optimize(compressor); + } } + break; } } if (compressor.option("unsafe_Func") From ae28a24c7f7919d8de1c3044f28571ebe2036850 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Thu, 16 Nov 2017 10:04:30 +0800 Subject: [PATCH 7/9] fix cross-scope inlining of `AST_Function`s (#2486) fixes #2485 --- bin/uglifyjs | 2 +- lib/ast.js | 4 +-- lib/compress.js | 13 +++++++--- test/compress/reduce_vars.js | 48 ++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/bin/uglifyjs b/bin/uglifyjs index 8cbb3cad..04c402d3 100755 --- a/bin/uglifyjs +++ b/bin/uglifyjs @@ -15,7 +15,7 @@ var path = require("path"); var program = require("commander"); var UglifyJS = require("../tools/node"); -var skip_keys = [ "cname", "enclosed", "parent_scope", "scope", "thedef", "uses_eval", "uses_with" ]; +var skip_keys = [ "cname", "enclosed", "inlined", "parent_scope", "scope", "thedef", "uses_eval", "uses_with" ]; var files = {}; var options = { compress: false, diff --git a/lib/ast.js b/lib/ast.js index 9b243f16..a2aa2b40 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -352,11 +352,11 @@ var AST_Accessor = DEFNODE("Accessor", null, { $documentation: "A setter/getter function. The `name` property is always null." }, AST_Lambda); -var AST_Function = DEFNODE("Function", null, { +var AST_Function = DEFNODE("Function", "inlined", { $documentation: "A function expression" }, AST_Lambda); -var AST_Defun = DEFNODE("Defun", null, { +var AST_Defun = DEFNODE("Defun", "inlined", { $documentation: "A function definition" }, AST_Lambda); diff --git a/lib/compress.js b/lib/compress.js index 0f72404f..2577643b 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -373,6 +373,7 @@ merge(Compressor.prototype, { } } if (node instanceof AST_Defun) { + node.inlined = false; var d = node.name.definition(); if (compressor.exposed(d) || safe_to_read(d)) { d.fixed = false; @@ -389,6 +390,7 @@ merge(Compressor.prototype, { return true; } if (node instanceof AST_Function) { + node.inlined = false; push(); var iife; if (!node.name @@ -4329,16 +4331,21 @@ merge(Compressor.prototype, { d.fixed = fixed = make_node(AST_Function, fixed, fixed); } if (d.single_use && fixed instanceof AST_Function) { - if (!compressor.option("reduce_funcs") && d.scope !== self.scope) { + if (d.scope !== self.scope + && (!compressor.option("reduce_funcs") + || d.escaped + || fixed.inlined)) { d.single_use = false; - } else if (d.escaped && d.scope !== self.scope || recursive_ref(compressor, d)) { + } else if (recursive_ref(compressor, d)) { d.single_use = false; } else if (d.scope !== self.scope || d.orig[0] instanceof AST_SymbolFunarg) { d.single_use = fixed.is_constant_expression(self.scope); if (d.single_use == "f") { var scope = self.scope; do { - if (scope.name) scope.name.definition().single_use = false; + if (scope instanceof AST_Defun || scope instanceof AST_Function) { + scope.inlined = true; + } } while (scope = scope.parent_scope); } } diff --git a/test/compress/reduce_vars.js b/test/compress/reduce_vars.js index e7189492..02ff5e43 100644 --- a/test/compress/reduce_vars.js +++ b/test/compress/reduce_vars.js @@ -4477,3 +4477,51 @@ perf_8: { } expect_stdout: "348150" } + +issue_2485: { + options = { + reduce_funcs: true, + reduce_vars: true, + unused: true, + } + input: { + var foo = function(bar) { + var n = function(a, b) { + return a + b; + }; + var sumAll = function(arg) { + return arg.reduce(n, 0); + }; + var runSumAll = function(arg) { + return sumAll(arg); + }; + bar.baz = function(arg) { + var n = runSumAll(arg); + return (n.get = 1), n; + }; + return bar; + }; + var bar = foo({}); + console.log(bar.baz([1, 2, 3])); + } + expect: { + var foo = function(bar) { + var n = function(a, b) { + return a + b; + }; + var runSumAll = function(arg) { + return function(arg) { + return arg.reduce(n, 0); + }(arg); + }; + bar.baz = function(arg) { + var n = runSumAll(arg); + return (n.get = 1), n; + }; + return bar; + }; + var bar = foo({}); + console.log(bar.baz([1, 2, 3])); + } + expect_stdout: "6" +} From 6142117cddaf4d827df8e269ed9b6fa7d3c4cec7 Mon Sep 17 00:00:00 2001 From: kzc Date: Fri, 17 Nov 2017 00:46:49 -0500 Subject: [PATCH 8/9] document the `webkit` output option (#2490) - workaround for WebKit bugs - PhantomJS users should enable this output option closes #2489 --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 1f63da75..18fc1b15 100644 --- a/README.md +++ b/README.md @@ -865,6 +865,9 @@ can pass additional arguments that control the code output: - `shebang` (default `true`) -- preserve shebang `#!` in preamble (bash scripts) +- `webkit` (default `false`) -- enable workarounds for WebKit bugs. + PhantomJS users should set this option to `true`. + - `width` (default `80`) -- only takes effect when beautification is on, this specifies an (orientative) line width that the beautifier will try to obey. It refers to the width of the line text (excluding indentation). From 667fc4d08b7c720ca66be8e63ebcfd234de54329 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Sat, 18 Nov 2017 23:56:33 +0800 Subject: [PATCH 9/9] v3.1.10 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 982a6fff..ba9cbe8b 100644 --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "homepage": "http://lisperator.net/uglifyjs", "author": "Mihai Bazon (http://lisperator.net/)", "license": "BSD-2-Clause", - "version": "3.1.9", + "version": "3.1.10", "engines": { "node": ">=0.8.0" },