From 7b61644f54fc70db405790c9acd1ec34a68b9ba5 Mon Sep 17 00:00:00 2001 From: alexlamsl Date: Mon, 30 Jan 2017 18:50:17 +0800 Subject: [PATCH] clean up `negate_iife` let other (safe) optimisations make proper decisions --- lib/compress.js | 128 ++++++++++++--------------- lib/output.js | 25 ------ lib/utils.js | 23 +++++ test/compress/negate-iife.js | 165 +++++++++++++++++++++++++++++++---- 4 files changed, 229 insertions(+), 112 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index 4e45df929fd..ec6ecbfd768 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -286,10 +286,6 @@ merge(Compressor.prototype, { } } while (CHANGED && max_iter-- > 0); - if (compressor.option("negate_iife")) { - negate_iifes(statements, compressor); - } - return statements; function collapse_single_use_vars(statements, compressor) { @@ -731,7 +727,11 @@ merge(Compressor.prototype, { }; statements.forEach(function(stat){ if (stat instanceof AST_SimpleStatement && seqLength(seq) < compressor.sequences_limit) { - seq.push(stat.body); + if (seq.length > 0 && stat.body instanceof AST_UnaryPrefix && stat.body.operator.length == 1) { + seq.push(stat.body.expression); + } else { + seq.push(stat.body); + } } else { push_seq(); ret.push(stat); @@ -837,50 +837,6 @@ merge(Compressor.prototype, { }, []); }; - function negate_iifes(statements, compressor) { - function is_iife_call(node) { - if (node instanceof AST_Call) { - return node.expression instanceof AST_Function || is_iife_call(node.expression); - } - return false; - } - - statements.forEach(function(stat){ - if (stat instanceof AST_SimpleStatement) { - stat.body = (function transform(thing) { - return thing.transform(new TreeTransformer(function(node){ - if (node instanceof AST_New) { - return node; - } - if (is_iife_call(node)) { - return make_node(AST_UnaryPrefix, node, { - operator: "!", - expression: node - }); - } - else if (node instanceof AST_Call) { - node.expression = transform(node.expression); - } - else if (node instanceof AST_Seq) { - node.car = transform(node.car); - } - else if (node instanceof AST_Conditional) { - var expr = transform(node.condition); - if (expr !== node.condition) { - // it has been negated, reverse - node.condition = expr; - var tmp = node.consequent; - node.consequent = node.alternative; - node.alternative = tmp; - } - } - return node; - })); - })(stat.body); - } - }); - }; - }; function extract_functions_from_statement_array(statements) { @@ -985,7 +941,15 @@ merge(Compressor.prototype, { return ast1.print_to_string().length > ast2.print_to_string().length ? ast2 : ast1; - }; + } + + function best_of_statement(ast1, ast2) { + return best_of(make_node(AST_SimpleStatement, ast1, { + body: ast1 + }), make_node(AST_SimpleStatement, ast2, { + body: ast2 + })).body; + } // methods to evaluate a constant expression (function (def){ @@ -1205,7 +1169,17 @@ merge(Compressor.prototype, { operator: "!", expression: exp }); - }; + } + function best(orig, alt, first_in_statement) { + var negated = basic_negation(orig); + if (first_in_statement) { + var stat = make_node(AST_SimpleStatement, alt, { + body: alt + }); + return best_of(negated, stat) === stat ? alt : negated; + } + return best_of(negated, alt); + } def(AST_Node, function(){ return basic_negation(this); }); @@ -1220,18 +1194,18 @@ merge(Compressor.prototype, { return this.expression; return basic_negation(this); }); - def(AST_Seq, function(compressor){ + def(AST_Seq, function(compressor, first_in_statement){ var self = this.clone(); self.cdr = self.cdr.negate(compressor); - return self; + return best(this, self, first_in_statement); }); - def(AST_Conditional, function(compressor){ + def(AST_Conditional, function(compressor, first_in_statement){ var self = this.clone(); self.consequent = self.consequent.negate(compressor); self.alternative = self.alternative.negate(compressor); - return best_of(basic_negation(this), self); + return best(this, self, first_in_statement); }); - def(AST_Binary, function(compressor){ + def(AST_Binary, function(compressor, first_in_statement){ var self = this.clone(), op = this.operator; if (compressor.option("unsafe_comps")) { switch (op) { @@ -1248,20 +1222,20 @@ merge(Compressor.prototype, { case "!==": self.operator = "==="; return self; case "&&": self.operator = "||"; - self.left = self.left.negate(compressor); + self.left = self.left.negate(compressor, first_in_statement); self.right = self.right.negate(compressor); - return best_of(basic_negation(this), self); + return best(this, self, first_in_statement); case "||": self.operator = "&&"; - self.left = self.left.negate(compressor); + self.left = self.left.negate(compressor, first_in_statement); self.right = self.right.negate(compressor); - return best_of(basic_negation(this), self); + return best(this, self, first_in_statement); } return basic_negation(this); }); })(function(node, func){ - node.DEFMETHOD("negate", function(compressor){ - return func.call(this, compressor); + node.DEFMETHOD("negate", function(compressor, first_in_statement){ + return func.call(this, compressor, first_in_statement); }); }); @@ -2292,7 +2266,18 @@ merge(Compressor.prototype, { } } } - return self.evaluate(compressor)[0]; + function is_iife_call(node) { + if (node instanceof AST_Call && !(node instanceof AST_New)) { + return node.expression instanceof AST_Function || is_iife_call(node.expression); + } + return false; + } + if (compressor.option("negate_iife") + && compressor.parent() instanceof AST_SimpleStatement + && is_iife_call(self)) { + return self.negate(compressor, true); + } + return self; }); OPT(AST_New, function(self, compressor){ @@ -2379,6 +2364,10 @@ merge(Compressor.prototype, { // !!foo ==> foo, if we're in boolean context return e.expression; } + if (e instanceof AST_Binary) { + var statement = first_in_statement(compressor); + self = (statement ? best_of_statement : best_of)(self, e.negate(compressor, statement)); + } break; case "typeof": // typeof always returns a non-empty string, thus it's @@ -2392,9 +2381,6 @@ merge(Compressor.prototype, { } return make_node(AST_True, self); } - if (e instanceof AST_Binary && self.operator == "!") { - self = best_of(self, e.negate(compressor)); - } } return self.evaluate(compressor)[0]; }); @@ -2570,11 +2556,12 @@ merge(Compressor.prototype, { if (compressor.option("comparisons") && self.is_boolean()) { if (!(compressor.parent() instanceof AST_Binary) || compressor.parent() instanceof AST_Assign) { + var statement = first_in_statement(compressor); var negated = make_node(AST_UnaryPrefix, self, { operator: "!", - expression: self.negate(compressor) + expression: self.negate(compressor, statement) }); - self = best_of(self, negated); + self = (statement ? best_of_statement : best_of)(self, negated); } if (compressor.option("unsafe_comps")) { switch (self.operator) { @@ -2773,8 +2760,9 @@ merge(Compressor.prototype, { return maintain_this_binding(compressor.parent(), self, self.alternative); } } - var negated = cond[0].negate(compressor); - if (best_of(cond[0], negated) === negated) { + var statement = first_in_statement(compressor); + var negated = cond[0].negate(compressor, statement); + if ((statement ? best_of_statement : best_of)(cond[0], negated) === negated) { self = make_node(AST_Conditional, self, { condition: negated, consequent: self.alternative, diff --git a/lib/output.js b/lib/output.js index 50e5aa43e6f..b6f0a873166 100644 --- a/lib/output.js +++ b/lib/output.js @@ -425,7 +425,6 @@ function OutputStream(options) { pos : function() { return current_pos }, push_node : function(node) { stack.push(node) }, pop_node : function() { return stack.pop() }, - stack : function() { return stack }, parent : function(n) { return stack[stack.length - 2 - (n || 0)]; } @@ -1334,30 +1333,6 @@ function OutputStream(options) { } }; - // return true if the node at the top of the stack (that means the - // innermost node in the current output) is lexically the first in - // a statement. - function first_in_statement(output) { - var a = output.stack(), i = a.length, node = a[--i], p = a[--i]; - while (i > 0) { - if (p instanceof AST_Statement && p.body === node) - return true; - if ((p instanceof AST_Seq && p.car === node ) || - (p instanceof AST_Call && p.expression === node && !(p instanceof AST_New) ) || - (p instanceof AST_Dot && p.expression === node ) || - (p instanceof AST_Sub && p.expression === node ) || - (p instanceof AST_Conditional && p.condition === node ) || - (p instanceof AST_Binary && p.left === node ) || - (p instanceof AST_UnaryPostfix && p.expression === node )) - { - node = p; - p = a[--i]; - } else { - return false; - } - } - }; - // self should be AST_New. decide if we want to show parens or not. function need_constructor_parens(self, output) { // Always print parentheses with arguments diff --git a/lib/utils.js b/lib/utils.js index d0a21ac9e69..a0571d65326 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -320,3 +320,26 @@ Dictionary.fromObject = function(obj) { function HOP(obj, prop) { return Object.prototype.hasOwnProperty.call(obj, prop); } + +// return true if the node at the top of the stack (that means the +// innermost node in the current output) is lexically the first in +// a statement. +function first_in_statement(stack) { + var node = stack.parent(-1); + for (var i = 0, p; p = stack.parent(i); i++) { + if (p instanceof AST_Statement && p.body === node) + return true; + if ((p instanceof AST_Seq && p.car === node ) || + (p instanceof AST_Call && p.expression === node && !(p instanceof AST_New) ) || + (p instanceof AST_Dot && p.expression === node ) || + (p instanceof AST_Sub && p.expression === node ) || + (p instanceof AST_Conditional && p.condition === node ) || + (p instanceof AST_Binary && p.left === node ) || + (p instanceof AST_UnaryPostfix && p.expression === node )) + { + node = p; + } else { + return false; + } + } +} diff --git a/test/compress/negate-iife.js b/test/compress/negate-iife.js index 0c11160448a..08fc9fbc3ca 100644 --- a/test/compress/negate-iife.js +++ b/test/compress/negate-iife.js @@ -1,7 +1,7 @@ -negate_iife_1: { +on: { options = { negate_iife: true - }; + } input: { (function(){ stuff() })(); } @@ -10,10 +10,20 @@ negate_iife_1: { } } -negate_iife_2: { +off: { + options = { + negate_iife: false + } + input: { + (function(){ stuff() })(); + } + expect_exact: '(function(){stuff()})();' +} + +assignment: { options = { negate_iife: true - }; + } input: { (function(){ return {} })().x = 10; // should not transform this one } @@ -22,10 +32,11 @@ negate_iife_2: { } } -negate_iife_3: { +conditional_on: { options = { negate_iife: true, - }; + conditionals: true + } input: { (function(){ return true })() ? console.log(true) : console.log(false); } @@ -34,30 +45,118 @@ negate_iife_3: { } } -negate_iife_3: { +conditional_off: { + options = { + negate_iife: false, + conditionals: true + } + input: { + (function(){ return true })() ? console.log(true) : console.log(false); + } + expect: { + !function(){ return true }() ? console.log(false) : console.log(true); + } +} + +sequence_on: { options = { negate_iife: true, + conditionals: true, + sequences: true, + passes: 2 + } + input: { + function f() { + (function(){ return true })() ? console.log(true) : console.log(false); + (function(){ + console.log("something"); + })(); + } + function g() { + (function(){ + console.log("something"); + })(); + (function(){ return true })() ? console.log(true) : console.log(false); + } + } + expect: { + function f() { + !function(){ return true }() ? console.log(false) : console.log(true), function(){ + console.log("something"); + }(); + } + function g() { + !function(){ + console.log("something"); + }(), function(){ return true }() ? console.log(true) : console.log(false); + } + } +} + +sequence_off: { + options = { + negate_iife: false, + conditionals: true, + sequences: true, + passes: 2 + } + input: { + function f() { + (function(){ return true })() ? console.log(true) : console.log(false); + (function(){ + console.log("something"); + })(); + } + function g() { + (function(){ + console.log("something"); + })(); + (function(){ return true })() ? console.log(true) : console.log(false); + } + } + expect: { + function f() { + !function(){ return true }() ? console.log(false) : console.log(true), function(){ + console.log("something"); + }(); + } + function g() { + (function(){ + console.log("something"); + })(), function(){ return true }() ? console.log(true) : console.log(false); + } + } +} + +conditional_sequence_on: { + options = { + negate_iife: true, + conditionals: true, sequences: true - }; + } input: { - (function(){ return true })() ? console.log(true) : console.log(false); + if ((function(){ return true })()) { + foo(true); + } else { + bar(false); + } (function(){ console.log("something"); })(); } expect: { - !function(){ return true }() ? console.log(false) : console.log(true), function(){ + !function(){ return true }() ? bar(false) : foo(true), function(){ console.log("something"); }(); } } -negate_iife_4: { +conditional_sequence_off: { options = { - negate_iife: true, - sequences: true, + negate_iife: false, conditionals: true, - }; + sequences: true + } input: { if ((function(){ return true })()) { foo(true); @@ -75,12 +174,12 @@ negate_iife_4: { } } -negate_iife_nested: { +negate_iife_nested_on: { options = { negate_iife: true, - sequences: true, conditionals: true, - }; + sequences: true + } input: { function Foo(f) { this.f = f; @@ -107,6 +206,38 @@ negate_iife_nested: { } } +negate_iife_nested_off: { + options = { + negate_iife: false, + conditionals: true, + sequences: true + } + input: { + function Foo(f) { + this.f = f; + } + new Foo(function() { + (function(x) { + (function(y) { + console.log(y); + })(x); + })(7); + }).f(); + } + expect: { + function Foo(f) { + this.f = f; + } + new Foo(function() { + (function(x) { + (function(y) { + console.log(y); + })(x); + })(7); + }).f(); + } +} + negate_iife_issue_1073: { options = { negate_iife: true,