From: Mihai Bazon Date: Wed, 5 Sep 2012 08:31:02 +0000 (+0300) Subject: cleaned up usage of AST_BlockStatement X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=8633b0073f016fddd655ae5e1f1726287a24828d;p=UglifyJS.git 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). --- diff --git a/lib/ast.js b/lib/ast.js index 48904ec9..7b2a3271 100644 --- 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 ]----- */ diff --git a/lib/compress.js b/lib/compress.js index 8db05333..71b20edb 100644 --- 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; }); diff --git a/lib/output.js b/lib/output.js index e5c1c26b..5df1c2c4 100644 --- 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 ]----- */ diff --git a/lib/parse.js b/lib/parse.js index 2eec3356..1c9e37fd 100644 --- 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 }); diff --git a/lib/scope.js b/lib/scope.js index 0f9969d2..0518a49a 100644 --- 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, diff --git a/tmp/test-node.js b/tmp/test-node.js index d737afde..01ae35a7 100755 --- 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); });