fix corner case in `ie8` (#3216)

fixes #3215

3 files changed, 93 insertions, 91 deletions

diff --git a/lib/compress.js b/lib/compress.js
index c28305a4..364bd362 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -1264,6 +1264,9 @@ merge(Compressor.prototype, {
                 if (node instanceof AST_Exit) {
                     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;
+                }
                 if (node instanceof AST_PropAccess) {
                     return side_effects || node.expression.may_throw_on_access(compressor);
                 }
@@ -1610,10 +1613,7 @@ merge(Compressor.prototype, {
                 if (def.orig.length == 1 && def.orig[0] instanceof AST_SymbolDefun) return false;
                 if (def.scope !== scope) return true;
                 return !all(def.references, function(ref) {
-                    var s = ref.scope;
-                    // "block" scope within AST_Catch
-                    if (s.TYPE == "Scope") s = s.parent_scope;
-                    return s === scope;
+                    return ref.scope.resolve() === scope;
                 });
             }
 
@@ -2704,35 +2704,30 @@ merge(Compressor.prototype, {
             if (right === this.right) return this;
             var result;
             switch (this.operator) {
-              case "&&"  : result = left &&  right; break;
-              case "||"  : result = left ||  right; break;
-              case "|"   : result = left |   right; break;
-              case "&"   : result = left &   right; break;
-              case "^"   : result = left ^   right; break;
-              case "+"   : result = left +   right; break;
-              case "*"   : result = left *   right; break;
-              case "/"   : result = left /   right; break;
-              case "%"   : result = left %   right; break;
-              case "-"   : result = left -   right; break;
-              case "<<"  : result = left <<  right; break;
-              case ">>"  : result = left >>  right; break;
-              case ">>>" : result = left >>> right; break;
-              case "=="  : result = left ==  right; break;
-              case "===" : result = left === right; break;
-              case "!="  : result = left !=  right; break;
-              case "!==" : result = left !== right; break;
-              case "<"   : result = left <   right; break;
-              case "<="  : result = left <=  right; break;
-              case ">"   : result = left >   right; break;
-              case ">="  : result = left >=  right; break;
-              default:
-                  return this;
-            }
-            if (isNaN(result) && compressor.find_parent(AST_With)) {
-                // leave original expression as is
-                return this;
-            }
-            return result;
+              case "&&" : result = left &&  right; break;
+              case "||" : result = left ||  right; break;
+              case "|"  : result = left |   right; break;
+              case "&"  : result = left &   right; break;
+              case "^"  : result = left ^   right; break;
+              case "+"  : result = left +   right; break;
+              case "*"  : result = left *   right; break;
+              case "/"  : result = left /   right; break;
+              case "%"  : result = left %   right; break;
+              case "-"  : result = left -   right; break;
+              case "<<" : result = left <<  right; break;
+              case ">>" : result = left >>  right; break;
+              case ">>>": result = left >>> right; break;
+              case "==" : result = left ==  right; break;
+              case "===": result = left === right; break;
+              case "!=" : result = left !=  right; break;
+              case "!==": result = left !== right; break;
+              case "<"  : result = left <   right; break;
+              case "<=" : result = left <=  right; break;
+              case ">"  : result = left >   right; break;
+              case ">=" : result = left >=  right; break;
+              default   : return this;
+            }
+            return isNaN(result) && compressor.find_parent(AST_With) ? this : result;
         });
         def(AST_Conditional, function(compressor, cached, depth) {
             var condition = this.condition._eval(compressor, cached, depth);
@@ -3412,6 +3407,14 @@ merge(Compressor.prototype, {
                 init.walk(tw);
             });
         }
+        var drop_fn_name = compressor.option("keep_fnames") ? return_false : compressor.option("ie8") ? function(def) {
+            return !compressor.exposed(def) && !def.references.length;
+        } : function(def) {
+            // any declarations with same name will overshadow
+            // name of this anonymous function and can therefore
+            // never be used anywhere
+            return !(def.id in in_use_ids) || def.orig.length > 1;
+        };
         // pass 3: we should drop declarations not in_use
         var tt = new TreeTransformer(function(node, descend, in_list) {
             var parent = tt.parent();
@@ -3439,15 +3442,8 @@ merge(Compressor.prototype, {
                 }
             }
             if (scope !== self) return;
-            if (node instanceof AST_Function
-                && node.name
-                && !compressor.option("ie8")
-                && !compressor.option("keep_fnames")) {
-                var def = node.name.definition();
-                // any declarations with same name will overshadow
-                // name of this anonymous function and can therefore
-                // never be used anywhere
-                if (!(def.id in in_use_ids) || def.orig.length > 1) node.name = null;
+            if (node instanceof AST_Function && node.name && drop_fn_name(node.name.definition())) {
+                node.name = null;
             }
             if (node instanceof AST_Lambda && !(node instanceof AST_Accessor)) {
                 var trim = !compressor.option("keep_fargs");
@@ -5726,10 +5722,10 @@ merge(Compressor.prototype, {
         if (def) {
             return def.optimize(compressor);
         }
-        // testing against !self.scope.uses_with first is an optimization
         if (!compressor.option("ie8")
             && is_undeclared_ref(self)
-            && (!self.scope.uses_with || !compressor.find_parent(AST_With))) {
+            // testing against `self.scope.uses_with` is an optimization
+            && !(self.scope.uses_with && compressor.find_parent(AST_With))) {
             switch (self.name) {
               case "undefined":
                 return make_node(AST_Undefined, self).optimize(compressor);
diff --git a/lib/scope.js b/lib/scope.js
index 8ce3e4da..ab207d60 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -118,11 +118,10 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
             descend();
             scope = save_scope;
             defun = save_defun;
-            return true;        // don't descend again in TreeWalker
+            return true;
         }
         if (node instanceof AST_With) {
-            for (var s = scope; s; s = s.parent_scope)
-                s.uses_with = true;
+            for (var s = scope; s; s = s.parent_scope) s.uses_with = true;
             return;
         }
         if (node instanceof AST_Symbol) {
@@ -132,18 +131,13 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
             node.thedef = node;
             node.references = [];
         }
-        if (node instanceof AST_SymbolDefun || options.ie8 && node instanceof AST_SymbolLambda) {
-            // Careful here, the scope where this should be defined is
-            // the parent scope.  The reason is that we enter a new
-            // scope when we encounter the AST_Defun node (which is
-            // instanceof AST_Scope) but we get to the symbol a bit
-            // later.
-            (node.scope = defun.parent_scope).def_function(node, defun);
-        }
-        else if (node instanceof AST_SymbolLambda) {
+        if (node instanceof AST_SymbolDefun) {
+            // This should be defined in the parent scope, as we encounter the
+            // AST_Defun node before getting to its AST_Symbol.
+            (node.scope = defun.parent_scope.resolve()).def_function(node, defun);
+        } else if (node instanceof AST_SymbolLambda) {
             defun.def_function(node, node.name == "arguments" ? undefined : defun);
-        }
-        else if (node instanceof AST_SymbolVar) {
+        } else if (node instanceof AST_SymbolVar) {
             defun.def_variable(node, node.TYPE == "SymbolVar" ? null : undefined);
             if (defun !== scope) {
                 node.mark_enclosed(options);
@@ -153,8 +147,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
                 }
                 node.reference(options);
             }
-        }
-        else if (node instanceof AST_SymbolCatch) {
+        } else if (node instanceof AST_SymbolCatch) {
             scope.def_variable(node).defun = defun;
         }
     });
@@ -162,9 +155,9 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
 
     // pass 2: find back references and eval
     self.globals = new Dictionary();
-    var tw = new TreeWalker(function(node, descend) {
-        if (node instanceof AST_LoopControl && node.label) {
-            node.label.thedef.references.push(node);
+    var tw = new TreeWalker(function(node) {
+        if (node instanceof AST_LoopControl) {
+            if (node.label) node.label.thedef.references.push(node);
             return true;
         }
         if (node instanceof AST_SymbolRef) {
@@ -185,35 +178,43 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
             return true;
         }
         // ensure mangling works if catch reuses a scope variable
-        var def;
-        if (node instanceof AST_SymbolCatch && (def = node.definition().redefined())) {
-            var s = node.scope;
-            while (s) {
+        if (node instanceof AST_SymbolCatch) {
+            var def = node.definition().redefined();
+            if (def) for (var s = node.scope; s; s = s.parent_scope) {
                 push_uniq(s.enclosed, def);
                 if (s === def.scope) break;
-                s = s.parent_scope;
             }
+            return true;
         }
     });
     self.walk(tw);
 
     // pass 3: fix up any scoping issue with IE8
-    if (options.ie8) {
-        self.walk(new TreeWalker(function(node, descend) {
-            if (node instanceof AST_SymbolCatch) {
-                var name = node.name;
-                var refs = node.thedef.references;
-                var scope = node.thedef.defun;
-                var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node);
-                refs.forEach(function(ref) {
-                    ref.thedef = def;
-                    ref.reference(options);
-                });
-                node.thedef = def;
-                node.reference(options);
-                return true;
+    if (options.ie8) self.walk(new TreeWalker(function(node) {
+        if (node instanceof AST_SymbolCatch) {
+            redefine(node, node.thedef.defun);
+            return true;
+        }
+        if (node instanceof AST_SymbolLambda) {
+            var def = node.thedef;
+            if (def.orig.length == 1) {
+                redefine(node, node.scope.parent_scope);
+                node.thedef.init = def.init;
             }
-        }));
+            return true;
+        }
+    }));
+
+    function redefine(node, scope) {
+        var name = node.name;
+        var refs = node.thedef.references;
+        var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node);
+        refs.forEach(function(ref) {
+            ref.thedef = def;
+            ref.reference(options);
+        });
+        node.thedef = def;
+        node.reference(options);
     }
 });
 
@@ -252,8 +253,7 @@ AST_Lambda.DEFMETHOD("init_scope_vars", function() {
 
 AST_Symbol.DEFMETHOD("mark_enclosed", function(options) {
     var def = this.definition();
-    var s = this.scope;
-    while (s) {
+    for (var s = this.scope; s; s = s.parent_scope) {
         push_uniq(s.enclosed, def);
         if (options.keep_fnames) {
             s.functions.each(function(d) {
@@ -261,7 +261,6 @@ AST_Symbol.DEFMETHOD("mark_enclosed", function(options) {
             });
         }
         if (s === def.scope) break;
-        s = s.parent_scope;
     }
 });
 
@@ -298,6 +297,12 @@ AST_Scope.DEFMETHOD("def_variable", function(symbol, init) {
     return symbol.thedef = def;
 });
 
+AST_Lambda.DEFMETHOD("resolve", return_this);
+AST_Scope.DEFMETHOD("resolve", function() {
+    return this.parent_scope;
+});
+AST_Toplevel.DEFMETHOD("resolve", return_this);
+
 function names_in_use(scope, options) {
     var names = scope.names_in_use;
     if (!names) {
@@ -410,7 +415,7 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options) {
             var save_nesting = lname;
             descend();
             lname = save_nesting;
-            return true;        // don't descend again in TreeWalker
+            return true;
         }
         if (node instanceof AST_Scope) {
             descend();
@@ -422,7 +427,9 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options) {
         }
         if (node instanceof AST_Label) {
             var name;
-            do name = base54(++lname); while (!is_identifier(name));
+            do {
+                name = base54(++lname);
+            } while (!is_identifier(name));
             node.mangled_name = name;
             return true;
         }
diff --git a/lib/utils.js b/lib/utils.js
index 4a61623a..40b65e2a 100644
--- a/lib/utils.js
+++ b/lib/utils.js
@@ -172,9 +172,8 @@ function string_template(text, props) {
 }
 
 function remove(array, el) {
-    for (var i = array.length; --i >= 0;) {
-        if (array[i] === el) array.splice(i, 1);
-    }
+    var index = array.indexOf(el);
+    if (index >= 0) array.splice(index, 1);
 }
 
 function makePredicate(words) {