cleaned up usage of AST_BlockStatement
authorMihai Bazon <mihai@bazon.net>
Wed, 5 Sep 2012 08:31:02 +0000 (11:31 +0300)
committerMihai Bazon <mihai@bazon.net>
Wed, 5 Sep 2012 08:39:43 +0000 (11:39 +0300)
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).

lib/ast.js
lib/compress.js
lib/output.js
lib/parse.js
lib/scope.js
tmp/test-node.js

index 48904ec..7b2a327 100644 (file)
@@ -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 ]----- */
 
index 8db0533..71b20ed 100644 (file)
@@ -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;
     });
 
index e5c1c26..5df1c2c 100644 (file)
@@ -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 ]----- */
index 2eec335..1c9e37f 100644 (file)
@@ -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
         });
index 0f9969d..0518a49 100644 (file)
@@ -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,
index d737afd..01ae35a 100755 (executable)
@@ -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);
 });