From: Mihai Bazon Date: Tue, 4 Sep 2012 10:20:28 +0000 (+0300) Subject: more fiddling with boolean expressions, etc. X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=376667a8188083e4e64ef56bf10f38e4f831447f;p=UglifyJS.git more fiddling with boolean expressions, etc. optimize away while(false), and transform while(true) ==> for(;;). UNSAFE: some expressions are optimized away when we're in boolean context and can determine that the value will always be true or false. For example: x() || true ==> always `true` in boolean context x() && false ==> always `false` in boolean context It's not technically correct to drop these expressions since we drop the function call too (that might have side effects); on the other hand, I can't see any legitimate use for such expressions and they might simply indicate a bug (we do warn about it). --- diff --git a/lib/compress.js b/lib/compress.js index 20af4707..8b826bdd 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -63,10 +63,28 @@ function Compressor(options, false_by_default) { comparations : !false_by_default, evaluate : !false_by_default, booleans : !false_by_default, + dwloops : !false_by_default, warnings : true }); var stack = []; + function in_boolean_context() { + var i = stack.length, self = stack[--i]; + while (i > 0) { + var p = stack[--i]; + if ((p instanceof AST_If && p.condition === self) || + (p instanceof AST_Conditional && p.condition === self) || + (p instanceof AST_DWLoop && p.condition === self) || + (p instanceof AST_For && p.condition === self) || + (p instanceof AST_UnaryPrefix && p.operator == "!" && p.expression === self)) + { + return true; + } + if (!(p instanceof AST_Binary && (p.operator == "&&" || p.operator == "||"))) + return false; + self = p; + } + }; return { option : function(key) { return options[key] }, push_node : function(node) { stack.push(node) }, @@ -78,7 +96,8 @@ function Compressor(options, false_by_default) { warn : function() { if (options.warnings) AST_Node.warn.apply(AST_Node, arguments); - } + }, + in_boolean_context: in_boolean_context }; }; @@ -274,7 +293,7 @@ function Compressor(options, false_by_default) { // evaluation was successful it's a node that represents the // constant; otherwise it's the original node. AST_Node.DEFMETHOD("evaluate", function(compressor){ - if (!compressor.option("evaluate")) return this; + if (!compressor.option("evaluate")) return [ this ]; try { var val = this._eval(), ast; switch (typeof val) { @@ -468,6 +487,23 @@ function Compressor(options, false_by_default) { self = self.clone(); self.condition = self.condition.squeeze(compressor); self.body = self.body.squeeze(compressor); + return self.optimize(compressor); + }); + + AST_DWLoop.DEFMETHOD("optimize", function(compressor){ + var self = this; + if (!compressor.option("dwloops")) return self; + var cond = self.condition.evaluate(compressor); + if (cond.length == 2) { + if (cond[1]) { + return make_node(AST_For, self, { + body: self.body + }); + } else if (self instanceof AST_While) { + AST_Node.warn("Unreachable code [{line},{col}]", self.start); + return make_node(AST_EmptyStatement, self); + } + } return self; }); @@ -512,15 +548,16 @@ function Compressor(options, false_by_default) { self.body = self.body.squeeze(compressor); if (self.alternative) self.alternative = self.alternative.squeeze(compressor); - return compressor.option("conditionals") ? self.optimize(compressor) : self; + return self.optimize(compressor); }); AST_If.DEFMETHOD("optimize", function(compressor){ + var self = this; + if (!compressor.option("conditionals")) return self; // if condition can be statically determined, warn and drop // one of the blocks. note, statically determined implies // “has no side effects”; also it doesn't work for cases like // `x && true`, though it probably should. - var self = this; var cond = self.condition.evaluate(compressor); if (cond.length == 2) { if (cond[1]) { @@ -528,7 +565,7 @@ function Compressor(options, false_by_default) { return self.body; } else { AST_Node.warn("Condition always false [{line},{col}]", self.condition.start); - return self.alternative || new AST_EmptyStatement(self); + return self.alternative || make_node(AST_EmptyStatement, self); } } if (self.body instanceof AST_SimpleStatement @@ -541,7 +578,7 @@ function Compressor(options, false_by_default) { }).optimize(compressor) }); } - if (!self.alternative && self.body instanceof AST_SimpleStatement) { + if ((!self.alternative || self.alternative instanceof AST_EmptyStatement) && self.body instanceof AST_SimpleStatement) { return make_node(AST_SimpleStatement, self, { body: make_node(AST_Binary, self, { operator : "&&", @@ -666,8 +703,25 @@ function Compressor(options, false_by_default) { }); SQUEEZE(AST_UnaryPrefix, function(self, compressor){ + // need to determine the context before cloning the node + var bool = compressor.in_boolean_context(); self = self.clone(); - self.expression = self.expression.squeeze(compressor); + var e = self.expression = self.expression.squeeze(compressor); + if (compressor.option("booleans") && bool) { + switch (self.operator) { + case "!": + if (e instanceof AST_UnaryPrefix && e.operator == "!") { + // !!foo ==> foo, if we're in boolean context + return e.expression; + } + break; + case "typeof": + // typeof always returns a non-empty string, thus it's + // always true in booleans + AST_Node.warn("Boolean expression always true [{line},{col}]", self.start); + return make_node(AST_True, self).optimize(compressor); + } + } return self.evaluate(compressor)[0]; }); @@ -675,7 +729,50 @@ function Compressor(options, false_by_default) { self = self.clone(); self.left = self.left.squeeze(compressor); self.right = self.right.squeeze(compressor); - return self.evaluate(compressor)[0]; + return self.optimize(compressor); + }); + + AST_Binary.DEFMETHOD("optimize", function(compressor){ + if (compressor.option("booleans") && compressor.in_boolean_context()) switch (this.operator) { + case "&&": + var ll = this.left.evaluate(compressor), left = ll[0]; + var rr = this.right.evaluate(compressor), right = rr[0]; + if ((ll.length == 2 && !ll[1]) || (rr.length == 2 && !rr[1])) { + AST_Node.warn("Boolean && always false [{line},{col}]", this.start); + return make_node(AST_False, this).optimize(compressor); + } + if (ll.length == 2 && ll[1]) { + return rr[0]; + } + if (rr.length == 2 && rr[1]) { + return ll[0]; + } + break; + case "||": + var ll = this.left.evaluate(compressor), left = ll[0]; + var rr = this.right.evaluate(compressor), right = rr[0]; + if ((ll.length == 2 && ll[1]) || (rr.length == 2 && rr[1])) { + AST_Node.warn("Boolean || always true [{line},{col}]", this.start); + return make_node(AST_True, this).optimize(compressor); + } + if (ll.length == 2 && !ll[1]) { + return rr[0]; + } + if (rr.length == 2 && !rr[1]) { + return ll[0]; + } + break; + case "+": + var ll = this.left.evaluate(compressor), left = ll[0]; + var rr = this.right.evaluate(compressor), right = rr[0]; + if ((ll.length == 2 && ll[0] instanceof AST_String && ll[1]) || + (rr.length == 2 && rr[0] instanceof AST_String && rr[1])) { + AST_Node.warn("+ in boolean context always true [{line},{col}]", this.start); + return make_node(AST_True, this).optimize(compressor); + } + break; + } + return this.evaluate(compressor)[0]; }); SQUEEZE(AST_Assign, function(self, compressor){ @@ -690,13 +787,24 @@ function Compressor(options, false_by_default) { self.condition = self.condition.squeeze(compressor); self.consequent = self.consequent.squeeze(compressor); self.alternative = self.alternative.squeeze(compressor); - return compressor.option("conditionals") ? self.optimize(compressor) : self; + return self.optimize(compressor); }); AST_Conditional.DEFMETHOD("optimize", function(compressor){ var self = this; + if (!compressor.option("conditionals")) return self; + var cond = self.condition.evaluate(compressor); + if (cond.length == 2) { + if (cond[1]) { + AST_Node.warn("Condition always true [{line},{col}]", self.start); + return self.consequent; + } else { + AST_Node.warn("Condition always false [{line},{col}]", self.start); + return self.alternative; + } + } var rev = self.clone(); - rev.condition = rev.condition.negate(compressor); + rev.condition = cond[0].negate(compressor); var tmp = rev.consequent; rev.consequent = rev.alternative; rev.alternative = tmp; @@ -722,21 +830,31 @@ function Compressor(options, false_by_default) { }); SQUEEZE(AST_True, function(self, compressor){ - if (compressor.option("booleans")) return make_node(AST_UnaryPrefix, self, { + return self.optimize(compressor); + }); + + AST_True.DEFMETHOD("optimize", function(compressor){ + if (compressor.option("booleans")) return make_node(AST_UnaryPrefix, this, { operator: "!", - expression: make_node(AST_Number, self, { + expression: make_node(AST_Number, this, { value: 0 }) }); + return this; }); SQUEEZE(AST_False, function(self, compressor){ - if (compressor.option("booleans")) return make_node(AST_UnaryPrefix, self, { + return self.optimize(compressor); + }); + + AST_False.DEFMETHOD("optimize", function(compressor){ + if (compressor.option("booleans")) return make_node(AST_UnaryPrefix, this, { operator: "!", - expression: make_node(AST_Number, self, { + expression: make_node(AST_Number, this, { value: 1 }) }); + return this; }); })();