fix function inlining within loops (#2675)

fixes #2663

2 files changed, 191 insertions, 30 deletions

diff --git a/lib/compress.js b/lib/compress.js
index bd61b87f..ac5cd235 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -3982,23 +3982,30 @@ merge(Compressor.prototype, {
         return self;
 
         function can_flatten_args(fn) {
-            var catches = Object.create(null);
+            var catches = Object.create(null), defs;
             do {
                 scope = compressor.parent(++level);
-                if (scope instanceof AST_SymbolRef) {
-                    if (scope.fixed_value() instanceof AST_Scope) return false;
-                } else if (scope instanceof AST_Catch) {
+                if (scope instanceof AST_Catch) {
                     catches[scope.argname.name] = true;
+                } else if (scope instanceof AST_IterationStatement) {
+                    defs = [];
+                } else if (scope instanceof AST_SymbolRef) {
+                    if (scope.fixed_value() instanceof AST_Scope) return false;
                 }
             } while (!(scope instanceof AST_Scope));
             var safe_to_inject = compressor.toplevel.vars || !(scope instanceof AST_Toplevel);
-            return all(fn.argnames, function(arg) {
-                return arg.__unused
-                    || safe_to_inject
-                        && !catches[arg.name]
-                        && !identifier_atom(arg.name)
-                        && !scope.var_names()[arg.name];
-            });
+            for (var i = 0, len = fn.argnames.length; i < len; i++) {
+                var arg = fn.argnames[i];
+                if (arg.__unused) continue;
+                if (!safe_to_inject
+                    || catches[arg.name]
+                    || identifier_atom(arg.name)
+                    || scope.var_names()[arg.name]) {
+                    return false;
+                }
+                if (defs) defs.push(arg.definition());
+            }
+            return !defs || defs.length == 0 || !is_reachable(fn.body[0], defs);
         }
 
         function flatten_args(fn) {
@@ -4838,6 +4845,28 @@ merge(Compressor.prototype, {
         return self;
     });
 
+    function is_reachable(node, defs) {
+        var reachable = false;
+        var find_ref = new TreeWalker(function(node) {
+            if (reachable) return true;
+            if (node instanceof AST_SymbolRef && member(node.definition(), defs)) {
+                return reachable = true;
+            }
+        });
+        var scan_scope = new TreeWalker(function(node) {
+            if (reachable) return true;
+            if (node instanceof AST_Scope) {
+                var parent = scan_scope.parent();
+                if (!(parent instanceof AST_Call && parent.expression === node)) {
+                    node.walk(find_ref);
+                }
+                return true;
+            }
+        });
+        node.walk(scan_scope);
+        return reachable;
+    }
+
     var ASSIGN_OPS = [ '+', '-', '/', '*', '%', '>>', '<<', '>>>', '|', '^', '&' ];
     var ASSIGN_OPS_COMMUTATIVE = [ '*', '|', '^', '&' ];
     OPT(AST_Assign, function(self, compressor){
@@ -4851,7 +4880,7 @@ merge(Compressor.prototype, {
                 parent = compressor.parent(level++);
                 if (parent instanceof AST_Exit) {
                     if (in_try(level, parent instanceof AST_Throw)) break;
-                    if (is_reachable(def)) break;
+                    if (is_reachable(self, [ def ])) break;
                     if (self.operator == "=") return self.right;
                     return make_node(AST_Binary, self, {
                         operator: self.operator.slice(0, -1),
@@ -4893,24 +4922,6 @@ merge(Compressor.prototype, {
                 }
             }
         }
-
-        function is_reachable(def) {
-            var reachable = false;
-            var find_ref = new TreeWalker(function(node) {
-                if (reachable) return true;
-                if (node instanceof AST_SymbolRef && node.definition() === def) {
-                    return reachable = true;
-                }
-            });
-            self.right.walk(new TreeWalker(function(node) {
-                if (reachable) return true;
-                if (node instanceof AST_Scope) {
-                    node.walk(find_ref);
-                    return true;
-                }
-            }));
-            return reachable;
-        }
     });
 
     OPT(AST_Conditional, function(self, compressor){
diff --git a/test/compress/functions.js b/test/compress/functions.js
index 83a27a06..888c6e3c 100644
--- a/test/compress/functions.js
+++ b/test/compress/functions.js
@@ -1496,3 +1496,153 @@ issue_2657: {
     }
     expect_stdout: "42"
 }
+
+issue_2663_1: {
+    options = {
+        inline: true,
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function() {
+            var i, o = {};
+            function createFn(j) {
+                return function() {
+                    console.log(j);
+                };
+            }
+            for (i in { a: 1, b: 2, c: 3 })
+                o[i] = createFn(i);
+            for (i in o)
+                o[i]();
+        })();
+    }
+    expect: {
+        (function() {
+            var i, o = {};
+            function createFn(j) {
+                return function() {
+                    console.log(j);
+                };
+            }
+            for (i in { a: 1, b: 2, c: 3 })
+                o[i] = createFn(i);
+            for (i in o)
+                o[i]();
+        })();
+    }
+    expect_stdout: [
+        "a",
+        "b",
+        "c",
+    ]
+}
+
+issue_2663_2: {
+    options = {
+        inline: true,
+        reduce_vars: true,
+        side_effects: true,
+        unused: true,
+    }
+    input: {
+        (function() {
+            var i;
+            function fn(j) {
+                return function() {
+                    console.log(j);
+                }();
+            }
+            for (i in { a: 1, b: 2, c: 3 })
+                fn(i);
+        })();
+    }
+    expect: {
+        (function() {
+            var i;
+            for (i in { a: 1, b: 2, c: 3 })
+                j = i, console.log(j);
+            var j;
+        })();
+    }
+    expect_stdout: [
+        "a",
+        "b",
+        "c",
+    ]
+}
+
+issue_2663_3: {
+    options = {
+        inline: true,
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        (function () {
+            var outputs = [
+                { type: 0, target: null, eventName: "ngSubmit", propName: null },
+                { type: 0, target: null, eventName: "submit", propName: null },
+                { type: 0, target: null, eventName: "reset", propName: null },
+            ];
+            function listenToElementOutputs(outputs) {
+                var handlers = [];
+                for (var i = 0; i < outputs.length; i++) {
+                    var output = outputs[i];
+                    var handleEventClosure = renderEventHandlerClosure(output.eventName);
+                    handlers.push(handleEventClosure)
+                }
+                var target, name;
+                return handlers;
+            }
+            function renderEventHandlerClosure(eventName) {
+                return function () {
+                    return console.log(eventName);
+                };
+            }
+            listenToElementOutputs(outputs).forEach(function (handler) {
+                return handler()
+            });
+        })();
+    }
+    expect: {
+        (function() {
+            function renderEventHandlerClosure(eventName) {
+                return function() {
+                    return console.log(eventName);
+                };
+            }
+            (function(outputs) {
+                var handlers = [];
+                for (var i = 0; i < outputs.length; i++) {
+                    var output = outputs[i];
+                    var handleEventClosure = renderEventHandlerClosure(output.eventName);
+                    handlers.push(handleEventClosure);
+                }
+                return handlers;
+            })([ {
+                type: 0,
+                target: null,
+                eventName: "ngSubmit",
+                propName: null
+            }, {
+                type: 0,
+                target: null,
+                eventName: "submit",
+                propName: null
+            }, {
+                type: 0,
+                target: null,
+                eventName: "reset",
+                propName: null
+            } ]).forEach(function(handler) {
+                return handler();
+            });
+        })();
+    }
+    expect_stdout: [
+        "ngSubmit",
+        "submit",
+        "reset",
+    ]
+}