From: Alex Lam S.L Date: Tue, 18 Apr 2017 20:49:09 +0000 (+0800) Subject: improve `collapse_vars` on `AST_Var` (#1828) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=4dcff038cb1a6951a0b20d1345bfdb27d756301c;p=UglifyJS.git improve `collapse_vars` on `AST_Var` (#1828) Perform the same cascaded scanning within `var` statement as we do on array of statements. --- diff --git a/lib/compress.js b/lib/compress.js index eb54f75d..2612d9ab 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -644,21 +644,21 @@ merge(Compressor.prototype, { 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]; - - var var_names_seen = Object.create(null); - var side_effects_encountered = false; - var lvalues_encountered = false; - var lvalues = Object.create(null); - var prev_stat_index, var_defs, var_defs_index; - + var stat_index; + var prev_stat_index; + var def_stat_index; + var stat; + var var_defs; + var var_defs_index; + for (stat_index = statements.length; --stat_index >= 0;) { + stat = statements[stat_index]; // Scan variable definitions from right to left. 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; + for (def_stat_index = var_defs.length; --def_stat_index >= 1;) { + stat = var_defs[def_stat_index]; + scan_var_defs(def_stat_index); } } else if (stat_index > 0) { // The variable definition must precede a statement. @@ -666,103 +666,107 @@ merge(Compressor.prototype, { 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; - } + scan_var_defs(var_defs.length); } } return statements; - 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; + function scan_var_defs(end_pos) { + var var_names_seen = Object.create(null); + var side_effects_encountered = false; + var lvalues_encountered = false; + var lvalues = Object.create(null); + for (var_defs_index = end_pos; --var_defs_index >= 0;) { + 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) break; + 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) break; + var_names_seen[var_name] = true; + + // Only interested in non-constant values. + if (var_decl.value.is_constant()) continue; + + // 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; + continue; + } + var ref = def.references[0]; - // Restrict var replacement to constants if side effects encountered. - if (side_effects_encountered |= lvalues_encountered) return; + // Don't replace ref if eval() or with statement in scope. + if (ref.scope.uses_eval || ref.scope.uses_with) break; - 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; - } + // Restrict var replacement to constants if side effects encountered. + if (side_effects_encountered |= lvalues_encountered) continue; - // 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; + continue; } - }); - 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; + + // 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; } - 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; + }); + 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; + } + 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(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; } - return true; - } - }, - 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); + ); + stat.transform(tt); + } } function is_lvalue(node, parent) { @@ -777,6 +781,7 @@ merge(Compressor.prototype, { var_decl.value = null; var_defs.splice(var_defs_index, 1); + def_stat_index--; if (var_defs.length === 0) { statements.splice(prev_stat_index, 1); stat_index--; diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index 0f82b743..94515a6f 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -1654,3 +1654,26 @@ iife_2: { }(bar()); } } + +var_defs: { + 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 + } + input: { + var f1 = function(x, y) { + var a, b, r = x + y, q = r * r, z = q - r, a = z, b = 7; + console.log(a + b); + }; + f1("1", 0); + } + expect: { + var f1 = function(x, y) { + var r = x + y, a = r * r - r, b = 7; + console.log(a + b); + }; + f1("1", 0); + } + expect_stdout: "97" +}