fix corner case in `collapse_vars` (#3597)

fixes #3596

3 files changed, 57 insertions, 25 deletions

diff --git a/lib/compress.js b/lib/compress.js
index a878ef21..aec31a48 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -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);
@@ -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;
             }
 
@@ -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);
@@ -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") {
@@ -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;
@@ -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 != "="));
             }
@@ -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;
             }
 
diff --git a/lib/output.js b/lib/output.js
index d59176c3..0f8ef1cd 100644
--- a/lib/output.js
+++ b/lib/output.js
@@ -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) {
diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js
index 6ad02b21..81f24175 100644
--- a/test/compress/collapse_vars.js
+++ b/test/compress/collapse_vars.js
@@ -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"
+}