Skip to content

Commit

Permalink
clean up collapse_vars (#1826)
Browse files Browse the repository at this point in the history 
- 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`
  • Loading branch information
@alexlamsl
alexlamsl authored Apr 18, 2017
1 parent 5d9f1da commit 0f4f01b
Show file tree
Hide file tree
Showing 2 changed files with 189 additions and 160 deletions.
245 changes: 117 additions & 128 deletions lib/compress.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
});
Expand Down
Loading

0 comments on commit 0f4f01b

Please sign in to comment.