cleaned up usage of AST_BlockStatement

The following nodes were instances of AST_BlockStatement: AST_Scope, AST_SwitchBlock, AST_SwitchBranch. Also, AST_Try, AST_Catch, AST_Finally were having a body instanceof AST_BlockStatement. Overloading the meaning of AST_BlockStatement this way turned out to be a mess; we now have an AST_Block class that is the base class for things having a block of statements (might or might not be bracketed). The `this.body` of AST_Scope, AST_Try, AST_Catch, AST_Finally is now an array of statements (as they inherit from AST_Block). Avoiding calling superclass's _walk function in walkers (turns out we walked a node multiple times).
2012-09-05 11:31:02 +03:00
parent 1b5183dd5e
commit 8633b0073f
6 changed files with 61 additions and 60 deletions
--- a/lib/ast.js
+++ b/lib/ast.js
@@ -127,7 +127,7 @@ var AST_SimpleStatement = DEFNODE("SimpleStatement", null, {
    $documentation: "A statement consisting of an expression, i.e. a = 1 + 2."
 }, AST_Statement);

-var AST_BlockStatement = DEFNODE("BlockStatement", "required", {
+var AST_BlockStatement = DEFNODE("BlockStatement", null, {
    $documentation: "A block statement.",
    _walk: function(visitor) {
        return visitor._visit(this, function(){
@@ -138,6 +138,21 @@ var AST_BlockStatement = DEFNODE("BlockStatement", "required", {
    }
 }, AST_Statement);

+function walk_body(node, visitor) {
+    if (node.body instanceof Array) node.body.forEach(function(stat){
+        stat._walk(visitor);
+    }); else node.body._walk(visitor);
+};
+
+var AST_Block = DEFNODE("Block", null, {
+    $documentation: "A block of statements (usually always bracketed)",
+    _walk: function(visitor) {
+        return visitor._visit(this, function(){
+            walk_body(this, visitor);
+        });
+    }
+}, AST_Statement);
+
 var AST_EmptyStatement = DEFNODE("EmptyStatement", null, {
    $documentation: "The empty statement (empty block or simply a semicolon).",
    _walk: function(visitor) {
@@ -213,8 +228,8 @@ var AST_With = DEFNODE("With", "expression", {
 /* -----[ scope and functions ]----- */

 var AST_Scope = DEFNODE("Scope", "variables functions uses_with uses_eval parent_scope enclosed cname", {
-    $documentation: "Base class for all statements introducing a lexical scope"
-}, AST_BlockStatement);
+    $documentation: "Base class for all statements introducing a lexical scope",
+}, AST_Block);

 var AST_Toplevel = DEFNODE("Toplevel", null, {
    initialize: function() {
@@ -231,7 +246,7 @@ var AST_Lambda = DEFNODE("Lambda", "name argnames uses_arguments", {
            this.argnames.forEach(function(arg){
                arg._walk(visitor);
            });
-            this.body._walk(visitor);
+            walk_body(this, visitor);
        });
    }
 }, AST_Scope);
@@ -311,25 +326,14 @@ var AST_Switch = DEFNODE("Switch", "expression", {

 var AST_SwitchBlock = DEFNODE("SwitchBlock", null, {
    $documentation: "The switch block is somewhat special, hence a special node for it",
-    initialize: function() {
-        this.required = true;
-    },
-}, AST_BlockStatement);
+}, AST_Block);

 var AST_SwitchBranch = DEFNODE("SwitchBranch", null, {
    $documentation: "Base class for `switch` branches",
-    initialize: function() {
-        this.required = true;
-    },
-}, AST_BlockStatement);
+}, AST_Block);

 var AST_Default = DEFNODE("Default", null, {
    $documentation: "A `default` switch branch",
-    _walk: function(visitor) {
-        return visitor._visit(this, function(){
-            AST_BlockStatement.prototype._walk.call(this, visitor);
-        });
-    }
 }, AST_SwitchBranch);

 var AST_Case = DEFNODE("Case", "expression", {
@@ -337,23 +341,23 @@ var AST_Case = DEFNODE("Case", "expression", {
    _walk: function(visitor) {
        return visitor._visit(this, function(){
            this.expression._walk(visitor);
-            AST_BlockStatement.prototype._walk.call(this, visitor);
+            walk_body(this, visitor);
        });
    }
 }, AST_SwitchBranch);

 /* -----[ EXCEPTIONS ]----- */

-var AST_Try = DEFNODE("Try", "btry bcatch bfinally", {
+var AST_Try = DEFNODE("Try", "bcatch bfinally", {
    $documentation: "A `try` statement",
    _walk: function(visitor) {
        return visitor._visit(this, function(){
-            this.btry._walk(visitor);
+            walk_body(this, visitor);
            if (this.bcatch) this.bcatch._walk(visitor);
            if (this.bfinally) this.bfinally._walk(visitor);
        });
    }
-}, AST_Statement);
+}, AST_Block);

 // XXX: this is wrong according to ECMA-262 (12.4).  the catch block
 // should introduce another scope, as the argname should be visible
@@ -361,24 +365,19 @@ var AST_Try = DEFNODE("Try", "btry bcatch bfinally", {
 // IE which simply introduces the name in the surrounding scope.  If
 // we ever want to fix this then AST_Catch should inherit from
 // AST_Scope.
-var AST_Catch = DEFNODE("Catch", "argname body", {
+var AST_Catch = DEFNODE("Catch", "argname", {
    $documentation: "A `catch` node; only makes sense as part of a `try` statement",
    _walk: function(visitor) {
        return visitor._visit(this, function(){
            this.argname._walk(visitor);
-            this.body._walk(visitor);
+            walk_body(this, visitor);
        });
    }
-});
+}, AST_Block);

-var AST_Finally = DEFNODE("Finally", "body", {
-    $documentation: "A `finally` node; only makes sense as part of a `try` statement",
-    _walk: function(visitor) {
-        return visitor._visit(this, function(){
-            this.body._walk(visitor);
-        });
-    }
-});
+var AST_Finally = DEFNODE("Finally", null, {
+    $documentation: "A `finally` node; only makes sense as part of a `try` statement"
+}, AST_Block);

 /* -----[ VAR/CONST ]----- */

--- a/lib/compress.js
+++ b/lib/compress.js
@@ -480,6 +480,12 @@ function Compressor(options, false_by_default) {
        return self;
    });

+    SQUEEZE(AST_Block, function(self, compressor){
+        self = self.clone();
+        self.body = tighten_body(self.body, compressor);
+        return self;
+    });
+
    SQUEEZE(AST_Scope, function(self, compressor){
        self = self.clone();
        self.hoist_declarations(compressor);
@@ -651,7 +657,7 @@ function Compressor(options, false_by_default) {

    SQUEEZE(AST_Try, function(self, compressor){
        self = self.clone();
-        self.btry = self.btry.squeeze(compressor);
+        self.body = tighten_body(self.body, compressor);
        if (self.bcatch) self.bcatch = self.bcatch.squeeze(compressor);
        if (self.bfinally) self.bfinally = self.bfinally.squeeze(compressor);
        return self;
@@ -682,7 +688,7 @@ function Compressor(options, false_by_default) {
        self.hoist_declarations(compressor);
        if (self.name) self.name = self.name.squeeze(compressor);
        self.argnames = do_list(self.argnames, compressor);
-        self.body = self.body.squeeze(compressor);
+        self.body = tighten_body(self.body, compressor);
        return self;
    });

--- a/lib/output.js
+++ b/lib/output.js
@@ -467,12 +467,14 @@ function OutputStream(options) {
        self.body.print(output);
        output.semicolon();
    });
-    DEFPRINT(AST_BlockStatement, function(self, output){
-        var body = self.body;
+    function print_bracketed(body, output) {
        if (body.length > 0) output.with_block(function(){
            display_body(body, false, output);
        });
        else output.print("{}");
+    };
+    DEFPRINT(AST_BlockStatement, function(self, output){
+        print_bracketed(self.body, output);
    });
    DEFPRINT(AST_EmptyStatement, function(self, output){
        output.semicolon();
@@ -563,7 +565,7 @@ function OutputStream(options) {
            });
        });
        output.space();
-        self.body.print(output);
+        print_bracketed(self.body, output);
    });
    DEFPRINT(AST_Lambda, function(self, output){
        self._do_print(output);
@@ -701,7 +703,7 @@ function OutputStream(options) {
    DEFPRINT(AST_Try, function(self, output){
        output.print("try");
        output.space();
-        self.btry.print(output);
+        print_bracketed(self.body, output);
        if (self.bcatch) {
            output.space();
            self.bcatch.print(output);
@@ -718,12 +720,12 @@ function OutputStream(options) {
            self.argname.print(output);
        });
        output.space();
-        self.body.print(output);
+        print_bracketed(self.body, output);
    });
    DEFPRINT(AST_Finally, function(self, output){
        output.print("finally");
        output.space();
-        self.body.print(output);
+        print_bracketed(self.body, output);
    });

    /* -----[ var/const ]----- */
--- a/lib/parse.js
+++ b/lib/parse.js
@@ -790,7 +790,11 @@ function parse($TEXT, exigent_mode) {
          case "punc":
            switch (S.token.value) {
              case "{":
-                return block_(false);
+                return new AST_BlockStatement({
+                    start : S.token,
+                    body  : block_(),
+                    end   : prev()
+                });
              case "[":
              case "(":
                return simple_statement();
@@ -982,7 +986,7 @@ function parse($TEXT, exigent_mode) {
                S.in_directives = true;
                S.in_loop = 0;
                S.labels = [];
-                var a = block_(true);
+                var a = block_();
                --S.in_function;
                S.in_loop = loop;
                S.labels = labels;
@@ -1004,22 +1008,15 @@ function parse($TEXT, exigent_mode) {
        });
    };

-    function block_(required) {
-        var start = S.token;
+    function block_() {
        expect("{");
        var a = [];
        while (!is("punc", "}")) {
            if (is("eof")) unexpected();
            a.push(statement());
        }
-        var end = S.token;
        next();
-        return new AST_BlockStatement({
-            required : required,
-            start    : start,
-            body     : a,
-            end      : end
-        });
+        return a;
    };

    var switch_block_ = embed_tokens(curry(in_loop, function(){
@@ -1056,7 +1053,6 @@ function parse($TEXT, exigent_mode) {
        var end = S.token;
        next();
        return new AST_SwitchBlock({
-            required : true,
            start    : start,
            body     : a,
            end      : end
@@ -1064,7 +1060,7 @@ function parse($TEXT, exigent_mode) {
    }));

    function try_() {
-        var body = block_(true), bcatch = null, bfinally = null;
+        var body = block_(), bcatch = null, bfinally = null;
        if (is("keyword", "catch")) {
            var start = S.token;
            next();
@@ -1074,7 +1070,7 @@ function parse($TEXT, exigent_mode) {
            bcatch = new AST_Catch({
                start   : start,
                argname : name,
-                body    : block_(true),
+                body    : block_(),
                end     : prev()
            });
        }
@@ -1083,14 +1079,14 @@ function parse($TEXT, exigent_mode) {
            next();
            bfinally = new AST_Finally({
                start : start,
-                body  : block_(true),
+                body  : block_(),
                end   : prev()
            });
        }
        if (!bcatch && !bfinally)
            croak("Missing catch/finally blocks");
        return new AST_Try({
-            btry     : body,
+            body     : body,
            bcatch   : bcatch,
            bfinally : bfinally
        });
--- a/lib/scope.js
+++ b/lib/scope.js
@@ -228,9 +228,7 @@ AST_Toplevel.DEFMETHOD("scope_warnings", function(options){
        }
        if (options.nested_defuns
            && node instanceof AST_Defun
-            && !(tw.parent() instanceof AST_Scope
-                 || (tw.parent() instanceof AST_BlockStatement
-                     && tw.parent(1) instanceof AST_Scope))) {
+            && !(tw.parent() instanceof AST_Scope)) {
            AST_Node.warn("Function {name} declared in nested statement [{line},{col}]", {
                name: node.name.name,
                line: node.start.line,
--- a/tmp/test-node.js
+++ b/tmp/test-node.js
@@ -41,7 +41,7 @@ time_it("compress", function(){
    ast = ast.squeeze(compressor);
 });

-var stream = UglifyJS.OutputStream({ beautify: false });
+var stream = UglifyJS.OutputStream({ beautify: true });
 time_it("generate", function(){
    ast.print(stream);
 });