retrofit `AST_BlockStatement` as block-scoped (#4177)

6 files changed, 104 insertions, 59 deletions

diff --git a/lib/ast.js b/lib/ast.js
index 3d972117..7e691c2c 100644
--- a/lib/ast.js
+++ b/lib/ast.js
@@ -251,9 +251,40 @@ var AST_Block = DEFNODE("Block", "body", {
     },
 }, AST_Statement);
 
+var AST_BlockScope = DEFNODE("BlockScope", "enclosed functions make_def parent_scope variables", {
+    $documentation: "Base class for all statements introducing a lexical scope",
+    $propdoc: {
+        enclosed: "[SymbolDef*/S] a list of all symbol definitions that are accessed from this scope or any subscopes",
+        functions: "[Object/S] like `variables`, but only lists function declarations",
+        parent_scope: "[AST_Scope?/S] link to the parent scope",
+        variables: "[Object/S] a map of name -> SymbolDef for all variables/functions defined in this scope",
+    },
+    clone: function(deep) {
+        var node = this._clone(deep);
+        if (this.enclosed) node.enclosed = this.enclosed.slice();
+        if (this.functions) node.functions = this.functions.clone();
+        if (this.variables) node.variables = this.variables.clone();
+        return node;
+    },
+    pinned: function() {
+        return this.resolve().pinned();
+    },
+    resolve: function() {
+        return this.parent_scope.resolve();
+    },
+    _validate: function() {
+        if (this.parent_scope == null) return;
+        if (!(this.parent_scope instanceof AST_BlockScope)) throw new Error("parent_scope must be AST_BlockScope");
+        if (!(this.resolve() instanceof AST_Scope)) throw new Error("must be contained within AST_Scope");
+    },
+}, AST_Block);
+
 var AST_BlockStatement = DEFNODE("BlockStatement", null, {
     $documentation: "A block statement",
-}, AST_Block);
+    initialize: function() {
+        this.variables = new Dictionary();
+    },
+}, AST_BlockScope);
 
 var AST_EmptyStatement = DEFNODE("EmptyStatement", null, {
     $documentation: "The empty statement (empty block or simply a semicolon)"
@@ -412,34 +443,6 @@ var AST_With = DEFNODE("With", "expression", {
 
 /* -----[ scope and functions ]----- */
 
-var AST_BlockScope = DEFNODE("BlockScope", "enclosed functions make_def parent_scope variables", {
-    $documentation: "Base class for all statements introducing a lexical scope",
-    $propdoc: {
-        enclosed: "[SymbolDef*/S] a list of all symbol definitions that are accessed from this scope or any subscopes",
-        functions: "[Object/S] like `variables`, but only lists function declarations",
-        parent_scope: "[AST_Scope?/S] link to the parent scope",
-        variables: "[Object/S] a map of name -> SymbolDef for all variables/functions defined in this scope",
-    },
-    clone: function(deep) {
-        var node = this._clone(deep);
-        if (this.enclosed) node.enclosed = this.enclosed.slice();
-        if (this.functions) node.functions = this.functions.clone();
-        if (this.variables) node.variables = this.variables.clone();
-        return node;
-    },
-    pinned: function() {
-        return this.resolve().pinned();
-    },
-    resolve: function() {
-        return this.parent_scope.resolve();
-    },
-    _validate: function() {
-        if (this.parent_scope == null) return;
-        if (!(this.parent_scope instanceof AST_BlockScope)) throw new Error("parent_scope must be AST_BlockScope");
-        if (!(this.resolve() instanceof AST_Scope)) throw new Error("must be contained within AST_Scope");
-    },
-}, AST_Block);
-
 var AST_Scope = DEFNODE("Scope", "cname uses_eval uses_with", {
     $documentation: "Base class for all statements introducing a lexical scope",
     $propdoc: {
diff --git a/lib/compress.js b/lib/compress.js
index 30757c44..7535d2bc 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -368,10 +368,10 @@ merge(Compressor.prototype, {
                 && !(def.init instanceof AST_Function && def.init !== def.scope)
                 && def.init;
             if (def.fixed instanceof AST_Defun && !all(def.references, function(ref) {
-                var scope = ref.scope;
+                var scope = ref.scope.resolve();
                 do {
                     if (def.scope === scope) return true;
-                } while (scope instanceof AST_Function && (scope = scope.parent_scope));
+                } while (scope instanceof AST_Function && (scope = scope.parent_scope.resolve()));
             })) {
                 tw.defun_ids[def.id] = false;
             }
@@ -854,7 +854,7 @@ merge(Compressor.prototype, {
                             && !value.pinned()
                             && (!d.in_loop || tw.parent() instanceof AST_Call)
                         || !d.in_loop
-                            && d.scope === this.scope
+                            && d.scope === this.scope.resolve()
                             && value.is_constant_expression();
                 } else {
                     d.single_use = false;
@@ -2585,7 +2585,7 @@ merge(Compressor.prototype, {
                 var lhs = expr.left;
                 if (!(lhs instanceof AST_SymbolRef)) break;
                 if (is_undeclared_ref(lhs)) break;
-                if (lhs.scope !== scope) break;
+                if (lhs.scope.resolve() !== scope) break;
                 var def = lhs.definition();
                 if (def.scope !== scope) break;
                 if (def.orig.length > def.eliminated + 1) break;
@@ -5179,12 +5179,12 @@ merge(Compressor.prototype, {
                 if (!(node_def.id in in_use_ids)) {
                     in_use_ids[node_def.id] = true;
                     in_use.push(node_def);
-                    if (node.scope !== node_def.scope) {
-                        var redef = node_def.redefined();
-                        if (redef && !(redef.id in in_use_ids)) {
-                            in_use_ids[redef.id] = true;
-                            in_use.push(redef);
-                        }
+                }
+                if (node.scope !== node_def.scope) {
+                    var redef = node_def.redefined();
+                    if (redef && !(redef.id in in_use_ids)) {
+                        in_use_ids[redef.id] = true;
+                        in_use.push(redef);
                     }
                 }
                 if (track_assigns(node_def, node)) add_assigns(node_def, node);
@@ -7031,6 +7031,9 @@ merge(Compressor.prototype, {
                 var stat = fn.body[i];
                 if (stat instanceof AST_Defun) {
                     if (!safe_to_inject || var_exists(used, stat.name.name)) return false;
+                    if (!all(stat.enclosed, function(def) {
+                        return def.scope === stat || !catches[def.name];
+                    })) return false;
                     continue;
                 }
                 if (!(stat instanceof AST_Var)) continue;
@@ -7067,7 +7070,7 @@ merge(Compressor.prototype, {
                 }
             } while (!(scope instanceof AST_Scope));
             var safe_to_inject = (!(scope instanceof AST_Toplevel) || compressor.toplevel.vars)
-                && (exp !== fn || fn.parent_scope === compressor.find_parent(AST_Scope));
+                && (exp !== fn || fn.parent_scope.resolve() === compressor.find_parent(AST_Scope));
             var inline = compressor.option("inline");
             var used = Object.create(catches);
             if (!can_inject_args(catches, used, inline >= 2 && safe_to_inject)) return false;
@@ -8087,7 +8090,7 @@ merge(Compressor.prototype, {
         if (!compressor.option("ie8")
             && is_undeclared_ref(self)
             // testing against `self.scope.uses_with` is an optimization
-            && !(self.scope.uses_with && compressor.find_parent(AST_With))) {
+            && !(self.scope.resolve().uses_with && compressor.find_parent(AST_With))) {
             switch (self.name) {
               case "undefined":
                 return make_node(AST_Undefined, self).optimize(compressor);
@@ -8104,16 +8107,14 @@ merge(Compressor.prototype, {
             var single_use = def.single_use && !(parent instanceof AST_Call && parent.is_expr_pure(compressor));
             if (single_use) {
                 if (fixed instanceof AST_Lambda) {
-                    if ((def.scope !== self.scope || def.in_loop)
+                    if ((def.scope !== self.scope.resolve() || def.in_loop)
                         && (!compressor.option("reduce_funcs") || def.escaped.depth == 1 || fixed.inlined)) {
                         single_use = false;
                     } else if (recursive_ref(compressor, def)) {
                         single_use = false;
                     } else if (fixed.name && fixed.name.definition() !== def) {
                         single_use = false;
-                    } else if (fixed.parent_scope !== self.scope
-                        || !(self.scope instanceof AST_Scope)
-                        || def.orig[0] instanceof AST_SymbolFunarg) {
+                    } else if (fixed.parent_scope !== self.scope.resolve() || def.orig[0] instanceof AST_SymbolFunarg) {
                         single_use = fixed.is_constant_expression(self.scope);
                         if (single_use == "f") {
                             var scope = self.scope;
@@ -8894,7 +8895,7 @@ merge(Compressor.prototype, {
             && expr instanceof AST_SymbolRef
             && is_arguments(def = expr.definition())
             && prop instanceof AST_Number
-            && (fn = expr.scope) === find_lambda()) {
+            && (fn = expr.scope.resolve()) === find_lambda()) {
             var index = prop.value;
             if (parent instanceof AST_UnaryPrefix && parent.operator == "delete") {
                 if (!def.deleted) def.deleted = [];
diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js
index 160b96b4..616bcaf7 100644
--- a/test/compress/drop-unused.js
+++ b/test/compress/drop-unused.js
@@ -2993,6 +2993,38 @@ issue_4146: {
     expect_stdout: "function"
 }
 
+var_catch_redefined: {
+    options = {
+        toplevel: true,
+        unused: true,
+    }
+    input: {
+        var a = "FAIL";
+        try {
+            throw "PASS";
+        } catch (a) {
+            function f() {
+                return a;
+            }
+            console.log(a);
+        }
+        f();
+    }
+    expect: {
+        var a = "FAIL";
+        try {
+            throw "PASS";
+        } catch (a) {
+            function f() {
+                return a;
+            }
+            console.log(a);
+        }
+        f();
+    }
+    expect_stdout: "PASS"
+}
+
 single_use_catch_redefined: {
     options = {
         reduce_vars: true,
diff --git a/test/compress/issue-1105.js b/test/compress/issue-1105.js
index df6c7dca..fee6540a 100644
--- a/test/compress/issue-1105.js
+++ b/test/compress/issue-1105.js
@@ -151,15 +151,18 @@ Infinity_not_in_with_scope: {
         unused: true,
     }
     input: {
-        var o = { Infinity: 'oInfinity' };
+        var o = { Infinity: "FAIL" };
         var vInfinity = "Infinity";
         vInfinity = Infinity;
+        console.log(vInfinity);
     }
     expect: {
-        var o = { Infinity: 'oInfinity' }
-        var vInfinity = "Infinity"
-        vInfinity = 1/0
+        var o = { Infinity: "FAIL" };
+        var vInfinity = "Infinity";
+        vInfinity = 1/0;
+        console.log(vInfinity);
     }
+    expect_stdout: "Infinity"
 }
 
 Infinity_in_with_scope: {
@@ -167,15 +170,18 @@ Infinity_in_with_scope: {
         unused: true,
     }
     input: {
-        var o = { Infinity: 'oInfinity' };
+        var o = { Infinity: "PASS" };
         var vInfinity = "Infinity";
         with (o) { vInfinity = Infinity; }
+        console.log(vInfinity);
     }
     expect: {
-        var o = { Infinity: 'oInfinity' }
-        var vInfinity = "Infinity"
-        with (o) vInfinity = Infinity
+        var o = { Infinity: "PASS" };
+        var vInfinity = "Infinity";
+        with (o) vInfinity = Infinity;
+        console.log(vInfinity);
     }
+    expect_stdout: "PASS"
 }
 
 assorted_Infinity_NaN_undefined_in_with_scope: {
diff --git a/test/compress/merge_vars.js b/test/compress/merge_vars.js
index a12781d3..28a0135d 100644
--- a/test/compress/merge_vars.js
+++ b/test/compress/merge_vars.js
@@ -466,7 +466,7 @@ issue_4112: {
                 var o = e;
                 for (e in o);
                 var a = function() {};
-                console.log;
+                console.log(typeof a);
                 return a;
             }
         }());
@@ -479,12 +479,15 @@ issue_4112: {
                 var a = e;
                 for (e in a);
                 a = function() {};
-                console.log;
+                console.log(typeof a);
                 return a;
             }
         }());
     }
-    expect_stdout: "function"
+    expect_stdout: [
+        "function",
+        "function",
+    ]
 }
 
 issue_4115: {
diff --git a/test/mocha/with.js b/test/mocha/with.js
index c6283800..7c17077e 100644
--- a/test/mocha/with.js
+++ b/test/mocha/with.js
@@ -17,7 +17,7 @@ describe("With", function() {
         var ast = UglifyJS.parse("with(e) {f(1, 2)}");
         ast.figure_out_scope();
         assert.equal(ast.uses_with, true);
-        assert.equal(ast.body[0].expression.scope.uses_with, true);
-        assert.equal(ast.body[0].body.body[0].body.expression.scope.uses_with, true);
+        assert.equal(ast.body[0].expression.scope.resolve().uses_with, true);
+        assert.equal(ast.body[0].body.body[0].body.expression.scope.resolve().uses_with, true);
     });
 });