handle exceptional flow correctly in collapse_vars (#2574)

fixes #2571

lib/compress.js

@@ -868,6 +868,21 @@ merge(Compressor.prototype, {
             var stat_index = statements.length;
             var scanner = new TreeTransformer(function(node, descend) {
                 if (abort) return node;
+                // Scan case expressions first in a switch statement
+                if (node instanceof AST_Switch) {
+                    node.expression = node.expression.transform(scanner);
+                    for (var i = 0, len = node.body.length; !abort && i < len; i++) {
+                        var branch = node.body[i];
+                        if (branch instanceof AST_Case)
+                            branch.expression = branch.expression.transform(scanner);
+                    }
+                    for (i = 0; !abort && i < len; i++) {
+                        var branch = node.body[i];
+                        for (var j = 0, len2 = branch.body.length; j < len2; j++)
+                            branch.body[j] = branch.body[j].transform(scanner);
+                    }
+                    return node;
+                }
                 // Skip nodes before `candidate` as quickly as possible
                 if (!hit) {
                     if (node === candidate) {
@@ -944,17 +959,17 @@ merge(Compressor.prototype, {
                             || side_effects && !references_in_scope(node.definition()))
                     || (sym = lhs_or_def(node))
                         && (sym instanceof AST_PropAccess || sym.name in lvalues)
+                    || may_throw && node.has_side_effects(compressor)
                     || (side_effects || !replace_all)
                         && (parent instanceof AST_Binary && lazy_op(parent.operator)
-                            || parent instanceof AST_Case
                             || parent instanceof AST_Conditional
                             || parent instanceof AST_If)) {
                     if (!(node instanceof AST_Scope)) descend(node, scanner);
                     abort = true;
                     return node;
                 }
-                // Skip (non-executed) functions and (leading) default case in switch statements
+                // Skip (non-executed) functions
-                if (node instanceof AST_Default || node instanceof AST_Scope) return node;
+                if (node instanceof AST_Scope) return node;
             });
             var multi_replacer = new TreeTransformer(function(node) {
                 if (abort) return node;
@@ -1000,6 +1015,7 @@ merge(Compressor.prototype, {
                         replace_all = def.references.length - def.replaced == 1;
                     }
                     var side_effects = value_has_side_effects(candidate);
+                    var may_throw = candidate.may_throw(compressor);
                     var funarg = candidate.name instanceof AST_SymbolFunarg;
                     var hit = funarg;
                     var abort = false, replaced = 0, can_replace = !args || !hit;
@@ -2200,15 +2216,6 @@ merge(Compressor.prototype, {
         def(AST_Constant, return_false);
         def(AST_This, return_false);
 
-        def(AST_Call, function(compressor){
-            if (!this.is_expr_pure(compressor)) return true;
-            for (var i = this.args.length; --i >= 0;) {
-                if (this.args[i].has_side_effects(compressor))
-                    return true;
-            }
-            return false;
-        });
-
         function any(list, compressor) {
             for (var i = list.length; --i >= 0;)
                 if (list[i].has_side_effects(compressor))
@@ -2219,6 +2226,10 @@ merge(Compressor.prototype, {
         def(AST_Block, function(compressor){
             return any(this.body, compressor);
         });
+        def(AST_Call, function(compressor){
+            return !this.is_expr_pure(compressor)
+                || any(this.args, compressor);
+        });
         def(AST_Switch, function(compressor){
             return this.expression.has_side_effects(compressor)
                 || any(this.body, compressor);
@@ -2243,8 +2254,7 @@ merge(Compressor.prototype, {
         def(AST_SimpleStatement, function(compressor){
             return this.body.has_side_effects(compressor);
         });
-        def(AST_Defun, return_true);
+        def(AST_Lambda, return_false);
-        def(AST_Function, return_false);
         def(AST_Binary, function(compressor){
             return this.left.has_side_effects(compressor)
                 || this.right.has_side_effects(compressor);
@@ -2282,14 +2292,121 @@ merge(Compressor.prototype, {
                 || this.property.has_side_effects(compressor);
         });
         def(AST_Sequence, function(compressor){
-            return this.expressions.some(function(expression, index) {
+            return any(this.expressions, compressor);
-                return expression.has_side_effects(compressor);
+        });
-            });
+        def(AST_Definitions, function(compressor){
+            return any(this.definitions, compressor);
+        });
+        def(AST_VarDef, function(compressor){
+            return this.value;
         });
     })(function(node, func){
         node.DEFMETHOD("has_side_effects", func);
     });
 
+    // determine if expression may throw
+    (function(def){
+        def(AST_Node, return_true);
+
+        def(AST_Constant, return_false);
+        def(AST_EmptyStatement, return_false);
+        def(AST_Lambda, return_false);
+        def(AST_SymbolDeclaration, return_false);
+        def(AST_This, return_false);
+
+        function any(list, compressor) {
+            for (var i = list.length; --i >= 0;)
+                if (list[i].may_throw(compressor))
+                    return true;
+            return false;
+        }
+
+        def(AST_Array, function(compressor){
+            return any(this.elements, compressor);
+        });
+        def(AST_Assign, function(compressor){
+            return this.operator != "=" && this.left.may_throw(compressor)
+                || this.right.may_throw(compressor);
+        });
+        def(AST_Binary, function(compressor){
+            return this.left.may_throw(compressor)
+                || this.right.may_throw(compressor);
+        });
+        def(AST_Block, function(compressor){
+            return any(this.body, compressor);
+        });
+        def(AST_Call, function(compressor){
+            if (any(this.args, compressor)) return true;
+            if (this.is_expr_pure(compressor)) return false;
+            if (this.expression.may_throw(compressor)) return true;
+            return !(this.expression instanceof AST_Lambda)
+                || any(this.expression.body, compressor);
+        });
+        def(AST_Case, function(compressor){
+            return this.expression.may_throw(compressor)
+                || any(this.body, compressor);
+        });
+        def(AST_Conditional, function(compressor){
+            return this.condition.may_throw(compressor)
+                || this.consequent.may_throw(compressor)
+                || this.alternative.may_throw(compressor);
+        });
+        def(AST_Definitions, function(compressor){
+            return any(this.definitions, compressor);
+        });
+        def(AST_Dot, function(compressor){
+            return this.expression.may_throw_on_access(compressor)
+                || this.expression.may_throw(compressor);
+        });
+        def(AST_If, function(compressor){
+            return this.condition.may_throw(compressor)
+                || this.body && this.body.may_throw(compressor)
+                || this.alternative && this.alternative.may_throw(compressor);
+        });
+        def(AST_LabeledStatement, function(compressor){
+            return this.body.may_throw(compressor);
+        });
+        def(AST_Object, function(compressor){
+            return any(this.properties, compressor);
+        });
+        def(AST_ObjectProperty, function(compressor){
+            return this.value.may_throw(compressor);
+        });
+        def(AST_Sequence, function(compressor){
+            return any(this.expressions, compressor);
+        });
+        def(AST_SimpleStatement, function(compressor){
+            return this.body.may_throw(compressor);
+        });
+        def(AST_Sub, function(compressor){
+            return this.expression.may_throw_on_access(compressor)
+                || this.expression.may_throw(compressor)
+                || this.property.may_throw(compressor);
+        });
+        def(AST_Switch, function(compressor){
+            return this.expression.may_throw(compressor)
+                || any(this.body, compressor);
+        });
+        def(AST_SymbolRef, function(compressor){
+            return !this.is_declared(compressor);
+        });
+        def(AST_Try, function(compressor){
+            return any(this.body, compressor)
+                || this.bcatch && this.bcatch.may_throw(compressor)
+                || this.bfinally && this.bfinally.may_throw(compressor);
+        });
+        def(AST_Unary, function(compressor){
+            if (this.operator == "typeof" && this.expression instanceof AST_SymbolRef)
+                return false;
+            return this.expression.may_throw(compressor);
+        });
+        def(AST_VarDef, function(compressor){
+            return this.value.may_throw(compressor);
+        });
+    })(function(node, func){
+        node.DEFMETHOD("may_throw", func);
+    });
+
     // determine if expression is constant
     (function(def){
         function all(list) {

test/compress/collapse_vars.js

@@ -69,10 +69,11 @@ collapse_vars_side_effects_1: {
             log(x, s.charAt(i++), y, 7);
         }
         function f4() {
-            var i = 10,
+            var log = console.log.bind(console),
+                i = 10,
                 x = i += 2,
                 y = i += 3;
-            console.log.bind(console)(x, i += 4, y, i);
+            log(x, i += 4, y, i);
         }
         f1(), f2(), f3(), f4();
     }
@@ -152,7 +153,7 @@ collapse_vars_issue_721: {
         collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true,
         comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true,
         keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true,
-        reduce_funcs: true, reduce_vars:true
+        reduce_funcs: true, reduce_vars:true, passes:2
     }
     input: {
         define(["require", "exports", 'handlebars'], function (require, exports, hb) {
@@ -675,8 +676,8 @@ collapse_vars_lvalues: {
         function f4(x) { var a = (x -= 3); return x + a; }
         function f5(x) { var w = e1(), v = e2(), c = v = --x; return (w = x) - c; }
         function f6(x) { var w = e1(), v = e2(); return (v = --x) - (w = x); }
-        function f7(x) { var w = e1(); return (w = x) - (e2() - x); }
+        function f7(x) { var w = e1(), v = e2(); return (w = x) - (v - x); }
-        function f8(x) { var w = e1(); return (w = x) - (e2() - x); }
+        function f8(x) { var w = e1(), v = e2(); return (w = x) - (v - x); }
         function f9(x) { var w = e1(); return e2() - x - (w = x); }
     }
 }
@@ -685,7 +686,7 @@ collapse_vars_lvalues_drop_assign: {
     options = {
         collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true,
         comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true,
-        keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true
+        keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, passes:3
     }
     input: {
         function f0(x) { var i = ++x; return x += i; }
@@ -706,10 +707,10 @@ collapse_vars_lvalues_drop_assign: {
         function f3(x) { var a = (x -= 3); return x + a; }
         function f4(x) { var a = (x -= 3); return x + a; }
         function f5(x) { e1(), e2(); var c = --x; return x - c; }
-        function f6(x) { e1(), e2(); return --x - x; }
+        function f6(x) { return e1(), e2(), --x - x; }
-        function f7(x) { e1(); return x - (e2() - x); }
+        function f7(x) { return e1(), x - (e2() - x); }
-        function f8(x) { e1(); return x - (e2() - x); }
+        function f8(x) { return e1(), x - (e2() - x); }
-        function f9(x) { e1(); return e2() - x - x; }
+        function f9(x) { return e1(), e2() - x - x; }
     }
 }
 
@@ -1767,11 +1768,10 @@ switch_case: {
     }
     expect: {
         function f(x, y, z) {
-            var c = z;
             switch (x()) {
               default: d();
               case y(): e();
-              case c: f();
+              case z: f();
             }
         }
     }
@@ -2195,7 +2195,7 @@ issue_315: {
     expect: {
         console.log(function() {
             var w, _i, _len, _ref, _results;
-            for (_results = [], _i = 0, _len = (_ref = "test".trim().split(" ")).length; _i < _len ; _i++)
+            for (_ref = "test".trim().split(" "), _results = [], _i = 0, _len = _ref.length; _i < _len ; _i++)
                 w = _ref[_i], _results.push(w.toLowerCase());
             return _results;
         }());
@@ -3108,8 +3108,8 @@ issue_2437: {
                 return Object.defineProperty(XMLHttpRequest.prototype, "onreadystatechange", xhrDesc || {}),
                     result;
             }
-            var req, detectFunc = function() {};
+            var req = new XMLHttpRequest(), detectFunc = function() {};
-            (req = new XMLHttpRequest()).onreadystatechange = detectFunc;
+            req.onreadystatechange = detectFunc;
             result = req[SYMBOL_FAKE_ONREADYSTATECHANGE_1] === detectFunc;
             req.onreadystatechange = null;
         }();
@@ -3675,3 +3675,57 @@ issue_2506: {
     }
     expect_stdout: "1"
 }
+
+issue_2571_1: {
+    options = {
+        collapse_vars: true,
+        toplevel: true,
+    }
+    input: {
+        var b = 1;
+        try {
+            var a = function f0(c) {
+                throw c;
+            }(2);
+            var d = --b + a;
+        } catch (e) {
+        }
+        console.log(b);
+    }
+    expect: {
+        var b = 1;
+        try {
+            var a = function f0(c) {
+                throw c;
+            }(2);
+            var d = --b + a;
+        } catch (e) {
+        }
+        console.log(b);
+    }
+    expect_stdout: "1"
+}
+
+issue_2571_2: {
+    options = {
+        collapse_vars: true,
+        toplevel: true,
+    }
+    input: {
+        try {
+            var a = A, b = 1;
+            throw a;
+        } catch (e) {
+            console.log(b);
+        }
+    }
+    expect: {
+        try {
+            var a = A, b = 1;
+            throw a;
+        } catch (e) {
+            console.log(b);
+        }
+    }
+    expect_stdout: "undefined"
+}