Make sure 'catch' and 'finally' are not children of 'try'. Fixes #1107

terser · Feb 3, 2023 · 627a433 · 627a433
1 parent 45dcfea
commit 627a433
Show file tree

Hide file tree

Showing 13 changed files with 92 additions and 41 deletions.
diff --git a/lib/ast.js b/lib/ast.js
@@ -111,6 +111,14 @@ class AST_Token {
         Object.seal(this);
     }
 
+    // Return a string summary of the token for node.js console.log
+    [Symbol.for("nodejs.util.inspect.custom")](_depth, options) {
+        const special = str => options.stylize(str, "special");
+        const quote = this.value.includes("`") ? "'" : "`";
+        const value = `${quote}${this.value}${quote}`;
+        return `${special("[AST_Token")} ${value} at ${this.line}:${this.col}${special("]")}`;
+    }
+
     get nlb() {
         return has_tok_flag(this, TOK_FLAG_NLB);
     }
@@ -1257,12 +1265,11 @@ var AST_Case = DEFNODE("Case", "expression", function AST_Case(props) {
 
 /* -----[ EXCEPTIONS ]----- */
 
-var AST_Try = DEFNODE("Try", "bcatch bfinally", function AST_Try(props) {
+var AST_Try = DEFNODE("Try", "body bcatch bfinally", function AST_Try(props) {
     if (props) {
+        this.body = props.body;
         this.bcatch = props.bcatch;
         this.bfinally = props.bfinally;
-        this.body = props.body;
-        this.block_scope = props.block_scope;
         this.start = props.start;
         this.end = props.end;
     }
@@ -1271,22 +1278,35 @@ var AST_Try = DEFNODE("Try", "bcatch bfinally", function AST_Try(props) {
 }, {
     $documentation: "A `try` statement",
     $propdoc: {
+        body: "[AST_TryBlock] the try block",
         bcatch: "[AST_Catch?] the catch block, or null if not present",
         bfinally: "[AST_Finally?] the finally block, or null if not present"
     },
     _walk: function(visitor) {
         return visitor._visit(this, function() {
-            walk_body(this, visitor);
+            this.body._walk(visitor);
             if (this.bcatch) this.bcatch._walk(visitor);
             if (this.bfinally) this.bfinally._walk(visitor);
         });
     },
     _children_backwards(push) {
         if (this.bfinally) push(this.bfinally);
         if (this.bcatch) push(this.bcatch);
-        let i = this.body.length;
-        while (i--) push(this.body[i]);
+        push(this.body);
     },
+}, AST_Statement);
+
+var AST_TryBlock = DEFNODE("TryBlock", null, function AST_TryBlock(props) {
+    if (props) {
+        this.body = props.body;
+        this.block_scope = props.block_scope;
+        this.start = props.start;
+        this.end = props.end;
+    }
+
+    this.flags = 0;
+}, {
+    $documentation: "The `try` block of a try statement"
 }, AST_Block);
 
 var AST_Catch = DEFNODE("Catch", "argname", function AST_Catch(props) {
@@ -3235,6 +3255,7 @@ export {
     AST_Toplevel,
     AST_True,
     AST_Try,
+    AST_TryBlock,
     AST_Unary,
     AST_UnaryPostfix,
     AST_UnaryPrefix,

diff --git a/lib/compress/index.js b/lib/compress/index.js
@@ -1529,9 +1529,9 @@ def_optimize(AST_Switch, function(self, compressor) {
 });
 
 def_optimize(AST_Try, function(self, compressor) {
-    tighten_body(self.body, compressor);
     if (self.bcatch && self.bfinally && self.bfinally.body.every(is_empty)) self.bfinally = null;
-    if (compressor.option("dead_code") && self.body.every(is_empty)) {
+
+    if (compressor.option("dead_code") && self.body.body.every(is_empty)) {
         var body = [];
         if (self.bcatch) {
             trim_unreachable_code(compressor, self.bcatch, body);
@@ -2737,16 +2737,21 @@ def_optimize(AST_Assign, function(self, compressor) {
     return self;
 
     function in_try(level, node) {
-        var right = self.right;
-        self.right = make_node(AST_Null, right);
-        var may_throw = node.may_throw(compressor);
-        self.right = right;
-        var scope = self.left.definition().scope;
+        function may_assignment_throw() {
+            const right = self.right;
+            self.right = make_node(AST_Null, right);
+            const may_throw = node.may_throw(compressor);
+            self.right = right;
+
+            return may_throw;
+        }
+
+        var stop_at = self.left.definition().scope.get_defun_scope();
         var parent;
-        while ((parent = compressor.parent(level++)) !== scope) {
+        while ((parent = compressor.parent(level++)) !== stop_at) {
             if (parent instanceof AST_Try) {
                 if (parent.bfinally) return true;
-                if (may_throw && parent.bcatch) return true;
+                if (parent.bcatch && may_assignment_throw()) return true;
             }
         }
     }

diff --git a/lib/compress/inference.js b/lib/compress/inference.js
@@ -300,7 +300,7 @@ export function is_nullish(node, compressor) {
             || any(this.body, compressor);
     });
     def_has_side_effects(AST_Try, function(compressor) {
-        return any(this.body, compressor)
+        return this.body.has_side_effects(compressor)
             || this.bcatch && this.bcatch.has_side_effects(compressor)
             || this.bfinally && this.bfinally.has_side_effects(compressor);
     });
@@ -529,7 +529,7 @@ export function is_nullish(node, compressor) {
     });
     def_may_throw(AST_SymbolClassProperty, return_false);
     def_may_throw(AST_Try, function(compressor) {
-        return this.bcatch ? this.bcatch.may_throw(compressor) : any(this.body, compressor)
+        return this.bcatch ? this.bcatch.may_throw(compressor) : this.body.may_throw(compressor)
             || this.bfinally && this.bfinally.may_throw(compressor);
     });
     def_may_throw(AST_Unary, function(compressor) {

diff --git a/lib/compress/reduce-vars.js b/lib/compress/reduce-vars.js
@@ -624,7 +624,7 @@ def_reduce_vars(AST_Toplevel, function(tw, descend, compressor) {
 def_reduce_vars(AST_Try, function(tw, descend, compressor) {
     reset_block_variables(compressor, this);
     push(tw);
-    walk_body(this, tw);
+    this.body.walk(tw);
     pop(tw);
     if (this.bcatch) {
         push(tw);

diff --git a/lib/compress/tighten-body.js b/lib/compress/tighten-body.js
@@ -52,7 +52,6 @@ import {
     AST_Break,
     AST_Call,
     AST_Case,
-    AST_Catch,
     AST_Chain,
     AST_Class,
     AST_Conditional,
@@ -71,7 +70,6 @@ import {
     AST_Exit,
     AST_Expansion,
     AST_Export,
-    AST_Finally,
     AST_For,
     AST_ForIn,
     AST_If,
@@ -103,6 +101,7 @@ import {
     AST_SymbolVar,
     AST_This,
     AST_Try,
+    AST_TryBlock,
     AST_Unary,
     AST_UnaryPostfix,
     AST_UnaryPrefix,
@@ -234,13 +233,11 @@ export function tighten_body(statements, compressor) {
     function find_loop_scope_try() {
         var node = compressor.self(), level = 0, in_loop = false, in_try = false;
         do {
-            if (node instanceof AST_Catch || node instanceof AST_Finally) {
-                level++;
-            } else if (node instanceof AST_IterationStatement) {
+            if (node instanceof AST_IterationStatement) {
                 in_loop = true;
             } else if (node instanceof AST_Scope) {
                 break;
-            } else if (node instanceof AST_Try) {
+            } else if (node instanceof AST_TryBlock) {
                 in_try = true;
             }
         } while (node = compressor.parent(level++));

diff --git a/lib/equivalent-to.js b/lib/equivalent-to.js
@@ -172,7 +172,7 @@ AST_Switch.prototype.shallow_cmp = pass_through;
 AST_SwitchBranch.prototype.shallow_cmp = pass_through;
 
 AST_Try.prototype.shallow_cmp = function(other) {
-    return (this.bcatch == null ? other.bcatch == null : this.bcatch === other.bcatch) && (this.bfinally == null ? other.bfinally == null : this.bfinally === other.bfinally);
+    return (this.body === other.body) && (this.bcatch == null ? other.bcatch == null : this.bcatch === other.bcatch) && (this.bfinally == null ? other.bfinally == null : this.bfinally === other.bfinally);
 };
 
 AST_Catch.prototype.shallow_cmp = function(other) {

diff --git a/lib/mozilla-ast.js b/lib/mozilla-ast.js
@@ -151,6 +151,7 @@ import {
     AST_Toplevel,
     AST_True,
     AST_Try,
+    AST_TryBlock,
     AST_Unary,
     AST_UnaryPostfix,
     AST_UnaryPrefix,
@@ -344,7 +345,7 @@ import { is_basic_identifier_string } from "./parse.js";
             return new AST_Try({
                 start    : my_start_token(M),
                 end      : my_end_token(M),
-                body     : from_moz(M.block).body,
+                body     : new AST_TryBlock(from_moz(M.block)),
                 bcatch   : from_moz(handlers[0]),
                 bfinally : M.finalizer ? new AST_Finally(from_moz(M.finalizer)) : null
             });
@@ -1315,7 +1316,7 @@ import { is_basic_identifier_string } from "./parse.js";
     def_to_moz(AST_Try, function To_Moz_TryStatement(M) {
         return {
             type: "TryStatement",
-            block: to_moz_block(M),
+            block: to_moz_block(M.body),
             handler: to_moz(M.bcatch),
             guardedHandlers: [],
             finalizer: to_moz(M.bfinally)

diff --git a/lib/output.js b/lib/output.js
@@ -143,6 +143,7 @@ import {
     AST_Throw,
     AST_Toplevel,
     AST_Try,
+    AST_TryBlock,
     AST_Unary,
     AST_UnaryPostfix,
     AST_UnaryPrefix,
@@ -1620,7 +1621,7 @@ function OutputStream(options) {
     DEFPRINT(AST_Try, function(self, output) {
         output.print("try");
         output.space();
-        print_braced(self, output);
+        self.body.print(output);
         if (self.bcatch) {
             output.space();
             self.bcatch.print(output);
@@ -1630,6 +1631,9 @@ function OutputStream(options) {
             self.bfinally.print(output);
         }
     });
+    DEFPRINT(AST_TryBlock, function(self, output) {
+        print_braced(self, output);
+    });
     DEFPRINT(AST_Catch, function(self, output) {
         output.print("catch");
         if (self.argname) {

diff --git a/lib/parse.js b/lib/parse.js
@@ -152,6 +152,7 @@ import {
     AST_Toplevel,
     AST_True,
     AST_Try,
+    AST_TryBlock,
     AST_UnaryPostfix,
     AST_UnaryPrefix,
     AST_Var,
@@ -2066,7 +2067,12 @@ function parse($TEXT, options) {
     }
 
     function try_() {
-        var body = block_(), bcatch = null, bfinally = null;
+        var body, bcatch = null, bfinally = null;
+        body = new AST_TryBlock({
+            start : S.token,
+            body  : block_(),
+            end   : prev(),
+        });
         if (is("keyword", "catch")) {
             var start = S.token;
             next();

diff --git a/lib/scope.js b/lib/scope.js
@@ -58,7 +58,6 @@ import {
     AST_Arrow,
     AST_Block,
     AST_Call,
-    AST_Catch,
     AST_Class,
     AST_Conditional,
     AST_DefClass,
@@ -223,12 +222,7 @@ AST_Scope.DEFMETHOD("figure_out_scope", function(options, { parent_scope = null,
             const save_scope = scope;
             node.block_scope = scope = new AST_Scope(node);
             scope._block_scope = true;
-            // AST_Try in the AST sadly *is* (not has) a body itself,
-            // and its catch and finally branches are children of the AST_Try itself
-            const parent_scope = node instanceof AST_Catch
-                ? save_scope.parent_scope
-                : save_scope;
-            scope.init_scope_vars(parent_scope);
+            scope.init_scope_vars(save_scope);
             scope.uses_with = save_scope.uses_with;
             scope.uses_eval = save_scope.uses_eval;
 
@@ -243,7 +237,7 @@ AST_Scope.DEFMETHOD("figure_out_scope", function(options, { parent_scope = null,
                 // AST_Switch has a scope within the body, but it itself "is a block scope"
                 // This means the switched expression has to belong to the outer scope
                 // while the body inside belongs to the switch itself.
-                // This is pretty nasty and warrants an AST change similar to AST_Try (read above)
+                // This is pretty nasty and warrants an AST change
                 const the_block_scope = scope;
                 scope = save_scope;
                 node.expression.walk(tw);

diff --git a/lib/size.js b/lib/size.js
@@ -216,9 +216,7 @@ AST_Default.prototype._size = function () {
     return 8 + list_overhead(this.body);
 };
 
-AST_Try.prototype._size = function () {
-    return 3 + list_overhead(this.body);
-};
+AST_Try.prototype._size = () => 3;
 
 AST_Catch.prototype._size = function () {
     let size = 7 + list_overhead(this.body);

diff --git a/lib/transform.js b/lib/transform.js
@@ -185,7 +185,7 @@ def_transform(AST_Case, function(self, tw) {
 });
 
 def_transform(AST_Try, function(self, tw) {
-    self.body = do_list(self.body, tw);
+    self.body = self.body.transform(tw);
     if (self.bcatch) self.bcatch = self.bcatch.transform(tw);
     if (self.bfinally) self.bfinally = self.bfinally.transform(tw);
 });

diff --git a/test/compress/reduce_vars.js b/test/compress/reduce_vars.js
@@ -2426,6 +2426,31 @@ catch_var: {
     expect_stdout: "true"
 }
 
+issue_1107: {
+    options = {
+        reduce_vars: true,
+        unused: true,
+    }
+    input: {
+        function foo() {
+            let bar = "PASS 2";
+
+            try {
+                const bar = "PASS 1";
+                console.log(bar);
+            } finally {
+                console.log(bar);
+            }
+        }
+
+        foo();
+    }
+    expect_stdout: [
+        "PASS 1",
+        "PASS 2"
+    ]
+}
+
 var_assign_1: {
     options = {
         evaluate: true,