retrofit `catch` as block-scoped (#4165)

3 files changed, 36 insertions, 38 deletions

diff --git a/lib/ast.js b/lib/ast.js
index a0d79a8b..3d972117 100644
--- a/lib/ast.js
+++ b/lib/ast.js
@@ -412,34 +412,47 @@ var AST_With = DEFNODE("With", "expression", {
 
 /* -----[ scope and functions ]----- */
 
-var AST_Scope = DEFNODE("Scope", "cname enclosed uses_eval uses_with parent_scope functions variables make_def", {
+var AST_BlockScope = DEFNODE("BlockScope", "enclosed functions make_def parent_scope variables", {
     $documentation: "Base class for all statements introducing a lexical scope",
     $propdoc: {
-        cname: "[integer/S] current index for mangling variables (used internally by the mangler)",
         enclosed: "[SymbolDef*/S] a list of all symbol definitions that are accessed from this scope or any subscopes",
-        uses_eval: "[boolean/S] tells whether this scope contains a direct call to the global `eval`",
-        uses_with: "[boolean/S] tells whether this scope uses the `with` statement",
-        parent_scope: "[AST_Scope?/S] link to the parent scope",
         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.variables) node.variables = this.variables.clone();
-        if (this.functions) node.functions = this.functions.clone();
         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.uses_eval || this.uses_with;
+        return this.resolve().pinned();
+    },
+    resolve: function() {
+        return this.parent_scope.resolve();
     },
     _validate: function() {
-        if (this.parent_scope != null) {
-            if (!(this.parent_scope instanceof AST_Scope)) throw new Error("parent_scope must be AST_Scope");
-        }
+        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: {
+        cname: "[integer/S] current index for mangling variables (used internally by the mangler)",
+        uses_eval: "[boolean/S] tells whether this scope contains a direct call to the global `eval`",
+        uses_with: "[boolean/S] tells whether this scope uses the `with` statement",
+    },
+    pinned: function() {
+        return this.uses_eval || this.uses_with;
+    },
+    resolve: return_this,
+}, AST_BlockScope);
+
 var AST_Toplevel = DEFNODE("Toplevel", "globals", {
     $documentation: "The toplevel scope",
     $propdoc: {
@@ -703,7 +716,7 @@ var AST_Catch = DEFNODE("Catch", "argname", {
     _validate: function() {
         if (!(this.argname instanceof AST_SymbolCatch)) throw new Error("argname must be AST_SymbolCatch");
     },
-}, AST_Block);
+}, AST_BlockScope);
 
 var AST_Finally = DEFNODE("Finally", null, {
     $documentation: "A `finally` node; only makes sense as part of a `try` statement"
diff --git a/lib/compress.js b/lib/compress.js
index df797bf7..d60482f8 100644
--- a/lib/compress.js
+++ b/lib/compress.js
@@ -5401,7 +5401,7 @@ merge(Compressor.prototype, {
         process_boolean_returns(this, compressor);
     });
 
-    AST_Scope.DEFMETHOD("var_names", function() {
+    AST_BlockScope.DEFMETHOD("var_names", function() {
         var var_names = this._var_names;
         if (!var_names) {
             this._var_names = var_names = Object.create(null);
diff --git a/lib/scope.js b/lib/scope.js
index 405f563d..af888008 100644
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -100,19 +100,12 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) {
     var next_def_id = 0;
     var scope = self.parent_scope = null;
     var tw = new TreeWalker(function(node, descend) {
-        if (node instanceof AST_Catch) {
-            var save_scope = scope;
-            scope = new AST_Scope(node);
-            scope.init_scope_vars(save_scope);
-            descend();
-            scope = save_scope;
-            return true;
-        }
-        if (node instanceof AST_Scope) {
+        if (node instanceof AST_BlockScope) {
             node.init_scope_vars(scope);
-            var save_scope = scope;
             var save_defun = defun;
-            defun = scope = node;
+            var save_scope = scope;
+            if (node instanceof AST_Scope) defun = node;
+            scope = node;
             descend();
             scope = save_scope;
             defun = save_defun;
@@ -267,7 +260,7 @@ function init_scope_vars(scope, parent) {
     if (parent) scope.make_def = parent.make_def;   // top-level tracking of SymbolDef instances
 }
 
-AST_Scope.DEFMETHOD("init_scope_vars", function(parent_scope) {
+AST_BlockScope.DEFMETHOD("init_scope_vars", function(parent_scope) {
     init_scope_vars(this, parent_scope);
 });
 
@@ -300,20 +293,20 @@ AST_Symbol.DEFMETHOD("reference", function(options) {
     this.mark_enclosed(options);
 });
 
-AST_Scope.DEFMETHOD("find_variable", function(name) {
+AST_BlockScope.DEFMETHOD("find_variable", function(name) {
     if (name instanceof AST_Symbol) name = name.name;
     return this.variables.get(name)
         || (this.parent_scope && this.parent_scope.find_variable(name));
 });
 
-AST_Scope.DEFMETHOD("def_function", function(symbol, init) {
+AST_BlockScope.DEFMETHOD("def_function", function(symbol, init) {
     var def = this.def_variable(symbol, init);
     if (!def.init || def.init instanceof AST_Defun) def.init = init;
     this.functions.set(symbol.name, def);
     return def;
 });
 
-AST_Scope.DEFMETHOD("def_variable", function(symbol, init) {
+AST_BlockScope.DEFMETHOD("def_variable", function(symbol, init) {
     var def = this.variables.get(symbol.name);
     if (def) {
         def.orig.push(symbol);
@@ -326,12 +319,6 @@ 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.resolve();
-});
-AST_Toplevel.DEFMETHOD("resolve", return_this);
-
 function names_in_use(scope, options) {
     var names = scope.names_in_use;
     if (!names) {
@@ -495,8 +482,7 @@ AST_Toplevel.DEFMETHOD("find_colliding_names", function(options) {
     options.reserved.forEach(to_avoid);
     this.globals.each(add_def);
     this.walk(new TreeWalker(function(node) {
-        if (node instanceof AST_Scope) node.variables.each(add_def);
-        if (node instanceof AST_SymbolCatch) add_def(node.definition());
+        if (node instanceof AST_BlockScope) node.variables.each(add_def);
     }));
     return avoid;
 
@@ -520,8 +506,7 @@ AST_Toplevel.DEFMETHOD("expand_names", function(options) {
     var cname = 0;
     this.globals.each(rename);
     this.walk(new TreeWalker(function(node) {
-        if (node instanceof AST_Scope) node.variables.each(rename);
-        if (node instanceof AST_SymbolCatch) rename(node.definition());
+        if (node instanceof AST_BlockScope) node.variables.each(rename);
     }));
 
     function next_name() {