From: Alex Lam S.L Date: Tue, 18 Apr 2017 13:45:34 +0000 (+0800) Subject: clean up `collapse_vars` (#1826) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=0f4f01b66cb2aa4356309c70a7d6a95618603630;p=UglifyJS.git clean up `collapse_vars` (#1826) - remove overlap in functionality of singular, consecutive reference of constant value - remove workarounds for previous bugs in `lib/scope.js` - distribute recursive `collapse_single_use_vars()` calls to their respective `OPT(AST_Node)` - enable collapsing of variables within a single `AST_Definitions` --- diff --git a/lib/compress.js b/lib/compress.js index 596b03fa..a3641573 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -642,151 +642,134 @@ merge(Compressor.prototype, { // and if it has exactly one reference then attempt to replace its reference // in the statement with the var value and then erase the var definition. - var self = compressor.self(); - var var_defs_removed = false; + var scope = compressor.find_parent(AST_Scope); var toplevel = compressor.option("toplevel"); for (var stat_index = statements.length; --stat_index >= 0;) { var stat = statements[stat_index]; - if (stat instanceof AST_Definitions) continue; - - // Process child blocks of statement if present. - [stat, stat.body, stat.alternative, stat.bcatch, stat.bfinally].forEach(function(node) { - node && node.body && collapse_single_use_vars(node.body, compressor); - }); - // The variable definition must precede a statement. - if (stat_index <= 0) break; - var prev_stat_index = stat_index - 1; - var prev_stat = statements[prev_stat_index]; - if (!(prev_stat instanceof AST_Definitions)) continue; - var var_defs = prev_stat.definitions; - if (var_defs == null) continue; - - var var_names_seen = {}; + var var_names_seen = Object.create(null); var side_effects_encountered = false; var lvalues_encountered = false; - var lvalues = {}; + var lvalues = Object.create(null); + var prev_stat_index, var_defs, var_defs_index; // Scan variable definitions from right to left. - for (var var_defs_index = var_defs.length; --var_defs_index >= 0;) { - - // Obtain var declaration and var name with basic sanity check. - var var_decl = var_defs[var_defs_index]; - if (var_decl.value == null) break; - var var_name = var_decl.name.name; - if (!var_name || !var_name.length) break; - - // Bail if we've seen a var definition of same name before. - if (var_name in var_names_seen) break; - var_names_seen[var_name] = true; - - // Only interested in cases with just one reference to the variable. - var def = self.find_variable && self.find_variable(var_name); - if (!def || !def.references || def.references.length !== 1 - || var_name == "arguments" || (!toplevel && def.global)) { - side_effects_encountered = true; - continue; + if (stat instanceof AST_Definitions) { + prev_stat_index = stat_index; + var_defs = stat.definitions; + for (var_defs_index = var_defs.length - 1; --var_defs_index >= 0;) { + if (collapse(var_defs[var_defs_index + 1])) break; } - var ref = def.references[0]; - - // Don't replace ref if eval() or with statement in scope. - if (ref.scope.uses_eval || ref.scope.uses_with) break; - - // Constant single use vars can be replaced in any scope. - if (var_decl.value.is_constant()) { - var ctt = new TreeTransformer(function(node) { - var parent = ctt.parent(); - if (parent instanceof AST_IterationStatement - && (parent.condition === node || parent.init === node)) { - return node; - } - if (node === ref) - return replace_var(node, parent, true); - }); - stat.transform(ctt); - continue; + } else if (stat_index > 0) { + // The variable definition must precede a statement. + prev_stat_index = stat_index - 1; + var prev_stat = statements[prev_stat_index]; + if (!(prev_stat instanceof AST_Definitions)) continue; + var_defs = prev_stat.definitions; + for (var_defs_index = var_defs.length; --var_defs_index >= 0;) { + if (collapse(stat)) break; } + } + } - // Restrict var replacement to constants if side effects encountered. - if (side_effects_encountered |= lvalues_encountered) continue; + return statements; - var value_has_side_effects = var_decl.value.has_side_effects(compressor); - // Non-constant single use vars can only be replaced in same scope. - if (ref.scope !== self) { - side_effects_encountered |= value_has_side_effects; - continue; - } + function collapse(stat) { + var var_decl = var_defs[var_defs_index]; + // `drop_unused()` shuffles variables without values to the top, + // so we can terminate upon first sighting as an optimization. + if (var_decl.value == null) return true; + var var_name = var_decl.name.name; + + // Bail if we've seen a var definition of same name before. + if (var_name in var_names_seen) return true; + var_names_seen[var_name] = true; + + // Only interested in non-constant values. + if (var_decl.value.is_constant()) return; + + // Only interested in cases with just one reference to the variable. + var def = var_decl.name.definition(); + if (def.references.length !== 1 + || var_name == "arguments" || (!toplevel && def.global)) { + side_effects_encountered = true; + return; + } + var ref = def.references[0]; + + // Don't replace ref if eval() or with statement in scope. + if (ref.scope.uses_eval || ref.scope.uses_with) return true; + + // Restrict var replacement to constants if side effects encountered. + if (side_effects_encountered |= lvalues_encountered) return; - // Detect lvalues in var value. - var tw = new TreeWalker(function(node){ - if (node instanceof AST_SymbolRef && is_lvalue(node, tw.parent())) { - lvalues[node.name] = lvalues_encountered = true; + var value_has_side_effects = var_decl.value.has_side_effects(compressor); + // Non-constant single use vars can only be replaced in same scope. + if (ref.scope !== scope) { + side_effects_encountered |= value_has_side_effects; + return; + } + + // Detect lvalues in var value. + var tw = new TreeWalker(function(node){ + if (node instanceof AST_SymbolRef && is_lvalue(node, tw.parent())) { + lvalues[node.name] = lvalues_encountered = true; + } + }); + var_decl.value.walk(tw); + + // Replace the non-constant single use var in statement if side effect free. + var unwind = false; + var tt = new TreeTransformer( + function preorder(node) { + if (unwind || node instanceof AST_Scope && node !== scope) return node; + var parent = tt.parent(); + if (node instanceof AST_Try + || node instanceof AST_With + || node instanceof AST_Case + || node instanceof AST_IterationStatement + || (parent instanceof AST_If && node !== parent.condition) + || (parent instanceof AST_Conditional && node !== parent.condition) + || (node instanceof AST_SymbolRef + && value_has_side_effects + && !are_references_in_scope(node.definition(), scope)) + || (parent instanceof AST_Binary + && (parent.operator == "&&" || parent.operator == "||") + && node === parent.right) + || (parent instanceof AST_Switch && node !== parent.expression)) { + return side_effects_encountered = unwind = true, node; } - }); - var_decl.value.walk(tw); - - // Replace the non-constant single use var in statement if side effect free. - var unwind = false; - var tt = new TreeTransformer( - function preorder(node) { - if (unwind) return node; - var parent = tt.parent(); - if (node instanceof AST_Lambda - || node instanceof AST_Try - || node instanceof AST_With - || node instanceof AST_Case - || node instanceof AST_IterationStatement - || (parent instanceof AST_If && node !== parent.condition) - || (parent instanceof AST_Conditional && node !== parent.condition) - || (node instanceof AST_SymbolRef - && value_has_side_effects - && !are_references_in_scope(node.definition(), self)) - || (parent instanceof AST_Binary - && (parent.operator == "&&" || parent.operator == "||") - && node === parent.right) - || (parent instanceof AST_Switch && node !== parent.expression)) { - return side_effects_encountered = unwind = true, node; - } - function are_references_in_scope(def, scope) { - if (def.orig.length === 1 - && def.orig[0] instanceof AST_SymbolDefun) return true; - if (def.scope !== scope) return false; - var refs = def.references; - for (var i = 0, len = refs.length; i < len; i++) { - if (refs[i].scope !== scope) return false; - } - return true; - } - }, - function postorder(node) { - if (unwind) return node; - if (node === ref) - return unwind = true, replace_var(node, tt.parent(), false); - if (side_effects_encountered |= node.has_side_effects(compressor)) - return unwind = true, node; - if (lvalues_encountered && node instanceof AST_SymbolRef && node.name in lvalues) { - side_effects_encountered = true; - return unwind = true, node; + function are_references_in_scope(def, scope) { + if (def.orig.length === 1 + && def.orig[0] instanceof AST_SymbolDefun) return true; + if (def.scope !== scope) return false; + var refs = def.references; + for (var i = 0, len = refs.length; i < len; i++) { + if (refs[i].scope !== scope) return false; } + return true; } - ); - stat.transform(tt); - } - } - - // Remove extraneous empty statments in block after removing var definitions. - // Leave at least one statement in `statements`. - if (var_defs_removed) for (var i = statements.length; --i >= 0;) { - if (statements.length > 1 && statements[i] instanceof AST_EmptyStatement) - statements.splice(i, 1); + }, + function postorder(node) { + if (unwind) return node; + if (node === ref) + return unwind = true, replace_var(var_decl, node, tt.parent(), false); + if (side_effects_encountered |= node.has_side_effects(compressor)) + return unwind = true, node; + if (lvalues_encountered && node instanceof AST_SymbolRef && node.name in lvalues) { + side_effects_encountered = true; + return unwind = true, node; + } + } + ); + stat.transform(tt); } - return statements; - function is_lvalue(node, parent) { return node instanceof AST_SymbolRef && is_lhs(node, parent); } - function replace_var(node, parent, is_constant) { + + function replace_var(var_decl, node, parent, is_constant) { if (is_lvalue(node, parent)) return node; // Remove var definition and return its value to the TreeTransformer to replace. @@ -795,14 +778,19 @@ merge(Compressor.prototype, { var_defs.splice(var_defs_index, 1); if (var_defs.length === 0) { - statements[prev_stat_index] = make_node(AST_EmptyStatement, self); - var_defs_removed = true; + statements.splice(prev_stat_index, 1); + stat_index--; } // Further optimize statement after substitution. stat.reset_opt_flags(compressor); - compressor.info("Collapsing " + (is_constant ? "constant" : "variable") + - " " + var_name + " [{file}:{line},{col}]", node.start); + compressor.info("Collapsing {type} {name} [{file}:{line},{col}]", { + type: is_constant ? "constant" : "variable", + name: var_decl.name.name, + file: node.start.file, + line: node.start.line, + col: node.start.col + }); CHANGED = true; return value; } @@ -1746,6 +1734,7 @@ merge(Compressor.prototype, { def(AST_SymbolRef, function(compressor){ return this.undeclared(); }); + def(AST_SymbolDeclaration, return_false); def(AST_Object, function(compressor){ return any(this.properties, compressor); }); diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index c01572dd..0f82b743 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -2,7 +2,7 @@ collapse_vars_side_effects_1: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1() { @@ -151,7 +151,7 @@ collapse_vars_issue_721: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { define(["require", "exports", 'handlebars'], function (require, exports, hb) { @@ -217,7 +217,7 @@ collapse_vars_properties: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1(obj) { @@ -244,7 +244,7 @@ collapse_vars_if: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1() { @@ -294,7 +294,7 @@ collapse_vars_while: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:false, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1(y) { @@ -393,9 +393,9 @@ collapse_vars_do_while: { } function f3(y) { function fn(n) { console.log(n); } - var a = 2; + var a = 2, x = 7; do { - fn(a = 7); + fn(a = x); break; } while (y); } @@ -468,8 +468,9 @@ collapse_vars_do_while_drop_assign: { } function f3(y) { function fn(n) { console.log(n); } + var x = 7; do { - fn(7); + fn(x); break; } while (y); } @@ -670,7 +671,7 @@ collapse_vars_lvalues: { function f4(x) { var a = (x -= 3); return x + a; } function f5(x) { var w = e1(), v = e2(), c = v = --x; return (w = x) - c; } function f6(x) { var w = e1(), v = e2(); return (v = --x) - (w = x); } - function f7(x) { var w = e1(), v = e2(), c = v - x; return (w = x) - c; } + function f7(x) { var w = e1(), c = e2() - x; return (w = x) - c; } function f8(x) { var w = e1(), v = e2(); return (w = x) - (v - x); } function f9(x) { var w = e1(); return e2() - x - (w = x); } } @@ -702,7 +703,7 @@ collapse_vars_lvalues_drop_assign: { function f4(x) { var a = (x -= 3); return x + a; } function f5(x) { var v = (e1(), e2()), c = v = --x; return x - c; } function f6(x) { e1(), e2(); return --x - x; } - function f7(x) { var v = (e1(), e2()), c = v - x; return x - c; } + function f7(x) { var c = (e1(), e2() - x); return x - c; } function f8(x) { var v = (e1(), e2()); return x - (v - x); } function f9(x) { e1(); return e2() - x - x; } } @@ -712,7 +713,7 @@ collapse_vars_misc1: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f0(o, a, h) { @@ -789,7 +790,7 @@ collapse_vars_repeated: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1() { @@ -812,19 +813,17 @@ collapse_vars_repeated: { } expect: { function f1() { - return -3 + return -3; } function f2(x) { - return x + return x; } (function(x){ - var a = "GOOD" + x, e = "BAD", e = a; - console.log(e + "!"); - })("!"), + console.log("GOOD!!"); + })(), (function(x){ - var a = "GOOD" + x, e = "BAD" + x, e = a; - console.log(e + "!"); - })("!"); + console.log("GOOD!!"); + })(); } expect_stdout: true } @@ -833,7 +832,7 @@ collapse_vars_closures: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function constant_vars_can_be_replaced_in_any_scope() { @@ -923,7 +922,7 @@ collapse_vars_try: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1() { @@ -1121,7 +1120,7 @@ collapse_vars_constants: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, reduce_vars:true } input: { function f1(x) { @@ -1159,7 +1158,7 @@ collapse_vars_arguments: { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, - toplevel:true + toplevel:true, reduce_vars:true } input: { var outer = function() { @@ -1286,6 +1285,7 @@ collapse_vars_regexp: { join_vars: true, cascade: true, side_effects: true, + reduce_vars: true, } input: { function f1() { @@ -1319,8 +1319,8 @@ collapse_vars_regexp: { }; } (function(){ - var result, s = "acdabcdeabbb", rx = /ab*/g; - while (result = rx.exec(s)) + var result, rx = /ab*/g; + while (result = rx.exec("acdabcdeabbb")) console.log(result[0]); })(); } @@ -1344,7 +1344,10 @@ issue_1537: { issue_1562: { options = { collapse_vars: true, + evaluate: true, + reduce_vars: true, toplevel: true, + unused: true, } input: { var v = 1, B = 2; @@ -1363,14 +1366,11 @@ issue_1562: { var v = 1; for (v in objs) f(2); - var x = 3; - while(x + 2) bar(10); + while(5) bar(10); - var y = 4; - do bar(20); while(y + 2); + do bar(20); while(6); - var z = 5; - for (; f(z + 2) ;) bar(30); + for (; f(7) ;) bar(30); } } @@ -1614,3 +1614,43 @@ reduce_vars_assign: { } expect_stdout: "0" } + +iife_1: { + options = { + collapse_vars: true, + reduce_vars: true, + toplevel: true, + unused: true, + } + input: { + var log = function(x) { + console.log(x); + }, foo = bar(); + log(foo); + } + expect: { + (function(x) { + console.log(x); + })(bar()); + } +} + +iife_2: { + options = { + collapse_vars: true, + reduce_vars: false, + toplevel: true, + unused: false, + } + input: { + var foo = bar(); + !function(x) { + console.log(x); + }(foo); + } + expect: { + !function(x) { + console.log(x); + }(bar()); + } +}