handle RHS side-effects in collapse_vars (#3097)

fixes #3096

lib/compress.js

@@ -295,6 +295,51 @@ merge(Compressor.prototype, {
         self.transform(tt);
     });
 
+    function read_property(obj, key) {
+        key = get_value(key);
+        if (key instanceof AST_Node) return;
+        var value;
+        if (obj instanceof AST_Array) {
+            var elements = obj.elements;
+            if (key == "length") return make_node_from_constant(elements.length, obj);
+            if (typeof key == "number" && key in elements) value = elements[key];
+        } else if (obj instanceof AST_Object) {
+            key = "" + key;
+            var props = obj.properties;
+            for (var i = props.length; --i >= 0;) {
+                var prop = props[i];
+                if (!(prop instanceof AST_ObjectKeyVal)) return;
+                if (!value && props[i].key === key) value = props[i].value;
+            }
+        }
+        return value instanceof AST_SymbolRef && value.fixed_value() || value;
+    }
+
+    function is_modified(compressor, tw, node, value, level, immutable) {
+        var parent = tw.parent(level);
+        var lhs = is_lhs(node, parent);
+        if (lhs) return lhs;
+        if (!immutable
+            && parent instanceof AST_Call
+            && parent.expression === node
+            && !parent.is_expr_pure(compressor)
+            && (!(value instanceof AST_Function)
+                || !(parent instanceof AST_New) && value.contains_this())) {
+            return true;
+        }
+        if (parent instanceof AST_Array) {
+            return is_modified(compressor, tw, parent, parent, level + 1);
+        }
+        if (parent instanceof AST_ObjectKeyVal && node === parent.value) {
+            var obj = tw.parent(level + 1);
+            return is_modified(compressor, tw, obj, obj, level + 2);
+        }
+        if (parent instanceof AST_PropAccess && parent.expression === node) {
+            var prop = read_property(value, parent.property);
+            return !immutable && is_modified(compressor, tw, parent, prop, level + 1);
+        }
+    }
+
     (function(def){
         def(AST_Node, noop);
 
@@ -385,45 +430,6 @@ merge(Compressor.prototype, {
                 || value instanceof AST_This;
         }
 
-        function read_property(obj, key) {
-            key = get_value(key);
-            if (key instanceof AST_Node) return;
-            var value;
-            if (obj instanceof AST_Array) {
-                var elements = obj.elements;
-                if (key == "length") return make_node_from_constant(elements.length, obj);
-                if (typeof key == "number" && key in elements) value = elements[key];
-            } else if (obj instanceof AST_Object) {
-                key = "" + key;
-                var props = obj.properties;
-                for (var i = props.length; --i >= 0;) {
-                    var prop = props[i];
-                    if (!(prop instanceof AST_ObjectKeyVal)) return;
-                    if (!value && props[i].key === key) value = props[i].value;
-                }
-            }
-            return value instanceof AST_SymbolRef && value.fixed_value() || value;
-        }
-
-        function is_modified(tw, node, value, level, immutable) {
-            var parent = tw.parent(level);
-            if (is_lhs(node, parent)
-                || !immutable
-                    && parent instanceof AST_Call
-                    && parent.expression === node
-                    && (!(value instanceof AST_Function)
-                        || !(parent instanceof AST_New) && value.contains_this())) {
-                return true;
-            } else if (parent instanceof AST_Array) {
-                return is_modified(tw, parent, parent, level + 1);
-            } else if (parent instanceof AST_ObjectKeyVal && node === parent.value) {
-                var obj = tw.parent(level + 1);
-                return is_modified(tw, obj, obj, level + 2);
-            } else if (parent instanceof AST_PropAccess && parent.expression === node) {
-                return !immutable && is_modified(tw, parent, read_property(value, parent.property), level + 1);
-            }
-        }
-
         function mark_escaped(tw, d, scope, node, value, level, depth) {
             var parent = tw.parent(level);
             if (value && value.is_constant()) return;
@@ -644,7 +650,7 @@ merge(Compressor.prototype, {
                 } else {
                     d.single_use = false;
                 }
-                if (is_modified(tw, this, value, 0, is_immutable(value))) {
+                if (is_modified(compressor, tw, this, value, 0, is_immutable(value))) {
                     if (d.single_use) {
                         d.single_use = "m";
                     } else {
@@ -1419,7 +1425,7 @@ merge(Compressor.prototype, {
                 var tw = new TreeWalker(function(node) {
                     var sym = root_expr(node);
                     if (sym instanceof AST_SymbolRef || sym instanceof AST_This) {
-                        lvalues[sym.name] = lvalues[sym.name] || is_lhs(node, tw.parent());
+                        lvalues[sym.name] = lvalues[sym.name] || is_modified(compressor, tw, node, node, 0);
                     }
                 });
                 expr.walk(tw);

test/compress/collapse_vars.js

@@ -5327,3 +5327,24 @@ issue_3032: {
     }
     expect_stdout: "42"
 }
+
+issue_3096: {
+    options = {
+        collapse_vars: true,
+    }
+    input: {
+        console.log(function() {
+            var ar = ["a", "b"];
+            var first = ar.pop();
+            return ar + "" + first;
+        }());
+    }
+    expect: {
+        console.log(function() {
+            var ar = ["a", "b"];
+            var first = ar.pop();
+            return ar + "" + first;
+        }());
+    }
+    expect_stdout: "ab"
+}