account for cross-scope modifications in `collapse_vars` (#1634)

mostly done by @kzc

fixes #1631

diff --git a/lib/compress.js b/lib/compress.js
index a8fbb8b..cfa8f23 100644 (file)
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -619,12 +619,24 @@ merge(Compressor.prototype, {
                                 || node instanceof AST_IterationStatement
                                 || (parent instanceof AST_If          && node !== parent.condition)
                                 || (parent instanceof AST_Conditional && node !== parent.condition)
+                                || (node instanceof AST_SymbolRef
+                                    && !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;

diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js
index 5a7c001..6f273b9 100644 (file)
--- a/test/compress/collapse_vars.js
+++ b/test/compress/collapse_vars.js
@@ -1415,3 +1415,110 @@ issue_1605_2: {
         (new Object).p = 1;
     }
 }
+
+issue_1631_1: {
+    options = {
+        cascade: true,
+        collapse_vars: true,
+        hoist_funs: true,
+        join_vars: true,
+        sequences: true,
+        side_effects: true,
+    }
+    input: {
+        var pc = 0;
+        function f(x) {
+            pc = 200;
+            return 100;
+        }
+        function x() {
+            var t = f();
+            pc += t;
+            return pc;
+        }
+        console.log(x());
+    }
+    expect: {
+        function f(x) {
+            return pc = 200, 100;
+        }
+        function x() {
+            var t = f();
+            return pc += t;
+        }
+        var pc = 0;
+        console.log(x());
+    }
+    expect_stdout: "300"
+}
+
+issue_1631_2: {
+    options = {
+        cascade: true,
+        collapse_vars: true,
+        hoist_funs: true,
+        join_vars: true,
+        sequences: true,
+        side_effects: true,
+    }
+    input: {
+        var a = 0, b = 1;
+        function f() {
+            a = 2;
+            return 4;
+        }
+        function g() {
+            var t = f();
+            b = a + t;
+            return b;
+        }
+        console.log(g());
+    }
+    expect: {
+        function f() {
+            return a = 2, 4;
+        }
+        function g() {
+            var t = f();
+            return b = a + t;
+        }
+        var a = 0, b = 1;
+        console.log(g());
+    }
+    expect_stdout: "6"
+}
+
+issue_1631_3: {
+    options = {
+        cascade: true,
+        collapse_vars: true,
+        hoist_funs: true,
+        join_vars: true,
+        sequences: true,
+        side_effects: true,
+    }
+    input: {
+        function g() {
+            var a = 0, b = 1;
+            function f() {
+                a = 2;
+                return 4;
+            }
+            var t = f();
+            b = a + t;
+            return b;
+        }
+        console.log(g());
+    }
+    expect: {
+        function g() {
+            function f() {
+                return a = 2, 4;
+            }
+            var a = 0, b = 1, t = f();
+            return b = a + t;
+        }
+        console.log(g());
+    }
+    expect_stdout: "6"
+}

diff --git a/test/mocha/glob.js b/test/mocha/glob.js
index 557489c..e291efc 100644 (file)
--- a/test/mocha/glob.js
+++ b/test/mocha/glob.js
@@ -5,7 +5,7 @@ var path = require("path");
 describe("minify() with input file globs", function() {
     it("minify() with one input file glob string.", function() {
         var result = Uglify.minify("test/input/issue-1242/foo.*");
-        assert.strictEqual(result.code, 'function foo(o){print("Foo:",2*o)}var print=console.log.bind(console);');
+        assert.strictEqual(result.code, 'function foo(o){var n=2*o;print("Foo:",n)}var print=console.log.bind(console);');
     });
     it("minify() with an array of one input file glob.", function() {
         var result = Uglify.minify([
@@ -20,7 +20,7 @@ describe("minify() with input file globs", function() {
         ], {
             compress: { toplevel: true }
         });
-        assert.strictEqual(result.code, 'var print=console.log.bind(console);print("qux",function(n){return 3*n}(3),function(n){return n/2}(12)),function(n){print("Foo:",2*n)}(11);');
+        assert.strictEqual(result.code, 'var print=console.log.bind(console),a=function(n){return 3*n}(3),b=function(n){return n/2}(12);print("qux",a,b),function(n){var o=2*n;print("Foo:",o)}(11);');
     });
     it("should throw with non-matching glob string", function() {
         var glob = "test/input/issue-1242/blah.*";