Skip to content

Commit

Permalink
fix corner case in collapse_vars (#3597)
Browse files Browse the repository at this point in the history 
fixes #3596
  • Loading branch information
@alexlamsl
alexlamsl committed Nov 18, 2019
1 parent 02cc4a0 commit c289ba1
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 25 deletions.
37 changes: 22 additions & 15 deletions lib/compress.js
View file
Expand Up @@ -1331,7 +1331,7 @@ merge(Compressor.prototype, {
return side_effects || lhs instanceof AST_PropAccess || may_modify(lhs);
}
if (node instanceof AST_Function) {
return compressor.option("ie8") && node.name && node.name.name in lvalues;
return compressor.option("ie8") && node.name && lvalues.has(node.name.name);
}
if (node instanceof AST_PropAccess) {
return side_effects || node.expression.may_throw_on_access(compressor);
Expand All @@ -1345,10 +1345,10 @@ merge(Compressor.prototype, {
if (node instanceof AST_This) return symbol_in_lvalues(node, parent);
if (node instanceof AST_VarDef) {
if (!node.value) return false;
return node.name.name in lvalues || side_effects && may_modify(node.name);
return lvalues.has(node.name.name) || side_effects && may_modify(node.name);
}
var sym = is_lhs(node.left, node);
if (sym && sym.name in lvalues) return true;
if (sym && lvalues.has(sym.name)) return true;
if (sym instanceof AST_PropAccess) return true;
}

Expand Down Expand Up @@ -1514,7 +1514,16 @@ merge(Compressor.prototype, {
if (parent instanceof AST_Exit) return find_stop_unused(parent, level + 1);
if (parent instanceof AST_If) return find_stop_unused(parent, level + 1);
if (parent instanceof AST_IterationStatement) return node;
if (parent instanceof AST_PropAccess) return find_stop_unused(parent, level + 1);
if (parent instanceof AST_PropAccess) {
var exp = parent.expression;
if (exp === node) return find_stop_unused(parent, level + 1);
var sym = root_expr(exp);
if (!(sym instanceof AST_SymbolRef)) return find_stop_unused(parent, level + 1);
var lvalue = lvalues.get(sym.name);
return !lvalue || all(lvalue, function(lhs) {
return !(lhs instanceof AST_PropAccess);
}) ? find_stop_unused(parent, level + 1) : node;
}
if (parent instanceof AST_Sequence) return find_stop_unused(parent, level + 1);
if (parent instanceof AST_SimpleStatement) return find_stop_unused(parent, level + 1);
if (parent instanceof AST_Switch) return find_stop_unused(parent, level + 1);
Expand Down Expand Up @@ -1612,10 +1621,8 @@ merge(Compressor.prototype, {
}

function get_lvalues(expr) {
var lvalues = Object.create(null);
if (candidate instanceof AST_VarDef) {
lvalues[candidate.name.name] = lhs;
}
var lvalues = new Dictionary();
if (candidate instanceof AST_VarDef) lvalues.add(candidate.name.name, lhs);
var scan_iife = scope instanceof AST_Toplevel;
var tw = new TreeWalker(function(node) {
if (scan_iife && node.TYPE == "Call") {
Expand All @@ -1632,9 +1639,7 @@ merge(Compressor.prototype, {
} else if (node instanceof AST_This) {
value = node;
}
if (value && !lvalues[node.name]) {
lvalues[node.name] = is_modified(compressor, tw, node, value, 0);
}
if (value) lvalues.add(node.name, is_modified(compressor, tw, node, value, 0));
});
expr.walk(tw);
return lvalues;
Expand Down Expand Up @@ -1679,7 +1684,7 @@ merge(Compressor.prototype, {
return sym instanceof AST_SymbolRef
&& sym.definition().scope === scope
&& !(in_loop
&& (sym.name in lvalues && lvalues[sym.name] !== lhs
&& (lvalues.has(sym.name) && lvalues.get(sym.name)[0] !== lhs
|| candidate instanceof AST_Unary
|| candidate instanceof AST_Assign && candidate.operator != "="));
}
Expand Down Expand Up @@ -1707,9 +1712,11 @@ merge(Compressor.prototype, {
}

function symbol_in_lvalues(sym, parent) {
var lvalue = lvalues[sym.name];
if (!lvalue) return;
if (lvalue !== lhs) return true;
var lvalue = lvalues.get(sym.name);
if (!lvalue || all(lvalue, function(lhs) {
return !lhs;
})) return;
if (lvalue[0] !== lhs) return true;
scan_rhs = false;
}

Expand Down
27 changes: 17 additions & 10 deletions lib/output.js
View file
Expand Up @@ -711,16 +711,23 @@ function OutputStream(options) {

PARENS(AST_Sequence, function(output) {
var p = output.parent();
return p instanceof AST_Call // (foo, bar)() or foo(1, (2, 3), 4)
|| p instanceof AST_Unary // !(foo, bar, baz)
|| p instanceof AST_Binary // 1 + (2, 3) + 4 ==> 8
|| p instanceof AST_VarDef // var a = (1, 2), b = a + a; ==> b == 4
|| p instanceof AST_PropAccess // (1, {foo:2}).foo or (1, {foo:2})["foo"] ==> 2
|| p instanceof AST_Array // [ 1, (2, 3), 4 ] ==> [ 1, 3, 4 ]
|| p instanceof AST_ObjectProperty // { foo: (1, 2) }.foo ==> 2
|| p instanceof AST_Conditional /* (false, true) ? (a = 10, b = 20) : (c = 30)
* ==> 20 (side effect, set a := 10 and b := 20) */
;
// (foo, bar)() or foo(1, (2, 3), 4)
return p instanceof AST_Call
// !(foo, bar, baz)
|| p instanceof AST_Unary
// 1 + (2, 3) + 4 ==> 8
|| p instanceof AST_Binary
// var a = (1, 2), b = a + a; ==> b == 4
|| p instanceof AST_VarDef
// (1, {foo:2}).foo or (1, {foo:2})["foo"] ==> 2
|| p instanceof AST_PropAccess && p.expression === this
// [ 1, (2, 3), 4 ] ==> [ 1, 3, 4 ]
|| p instanceof AST_Array
// { foo: (1, 2) }.foo ==> 2
|| p instanceof AST_ObjectProperty
// (false, true) ? (a = 10, b = 20) : (c = 30)
// ==> 20 (side effect, set a := 10 and b := 20)
|| p instanceof AST_Conditional;
});

PARENS(AST_Binary, function(output) {
Expand Down
18 changes: 18 additions & 0 deletions test/compress/collapse_vars.js
View file
Expand Up @@ -6549,3 +6549,21 @@ issue_3581_2: {
}
expect_stdout: "PASS PASS"
}

issue_3596: {
options = {
collapse_vars: true,
pure_getters: "strict",
}
input: {
console.log(function f() {
return f[[ ][f.undefined = 42, 0]] += !1;
}());
}
expect: {
console.log(function f() {
return f[[ ][f.undefined = 42, 0]] += !1;
}());
}
expect_stdout: "42"
}

0 comments on commit c289ba1

Please sign in to comment.