From 858e6c78a44f236e6e4d460a2ac187eef59823c8 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Tue, 28 Feb 2017 02:25:44 +0800 Subject: [PATCH] warn & drop `#__PURE__` iff IIFE is dropped (#1511) - consolidate `side-effects` optimisations - improve string `+` optimisation - enhance literal & `conditionals` optimisations --- lib/compress.js | 154 ++++++++++++++++++---------------- test/compress/conditionals.js | 40 +++++++-- test/compress/dead-code.js | 3 +- test/compress/evaluate.js | 42 ++++++---- test/compress/issue-1261.js | 58 +++++++++++++ test/compress/issue-1275.js | 2 +- test/compress/typeof.js | 5 +- test/mocha/minify.js | 11 +++ 8 files changed, 216 insertions(+), 99 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index 53833115..24ca1052 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -1375,9 +1375,7 @@ merge(Compressor.prototype, { && (comments = this.start.comments_before) && comments.length && /[@#]__PURE__/.test((last_comment = comments[comments.length - 1]).value)) { - compressor.warn("Dropping __PURE__ call [{file}:{line},{col}]", this.start); - last_comment.value = last_comment.value.replace(/[@#]__PURE__/g, ' '); - pure = true; + pure = last_comment; } return this.pure = pure; }); @@ -1676,8 +1674,7 @@ merge(Compressor.prototype, { line : def.name.start.line, col : def.name.start.col }; - if (def.value && def.value.has_side_effects(compressor)) { - def._unused_side_effects = true; + if (def.value && (def._unused_side_effects = def.value.drop_side_effect_free(compressor))) { compressor.warn("Side effects in initialization of unused variable {name} [{file}:{line},{col}]", w); return true; } @@ -1697,7 +1694,7 @@ merge(Compressor.prototype, { for (var i = 0; i < def.length;) { var x = def[i]; if (x._unused_side_effects) { - side_effects.push(x.value); + side_effects.push(x._unused_side_effects); def.splice(i, 1); } else { if (side_effects.length > 0) { @@ -1927,6 +1924,10 @@ merge(Compressor.prototype, { def(AST_This, return_null); def(AST_Call, function(compressor, first_in_statement){ if (!this.has_pure_annotation(compressor) && compressor.pure_funcs(this)) return this; + if (this.pure) { + compressor.warn("Dropping __PURE__ call [{file}:{line},{col}]", this.start); + this.pure.value = this.pure.value.replace(/[@#]__PURE__/g, ' '); + } var args = trim(this.args, compressor, first_in_statement); return args && AST_Seq.from_array(args); }); @@ -1978,8 +1979,17 @@ merge(Compressor.prototype, { case "typeof": if (this.expression instanceof AST_SymbolRef) return null; default: - if (first_in_statement && is_iife_call(this.expression)) return this; - return this.expression.drop_side_effect_free(compressor, first_in_statement); + var expression = this.expression.drop_side_effect_free(compressor, first_in_statement); + if (first_in_statement + && this instanceof AST_UnaryPrefix + && is_iife_call(expression)) { + if (expression === this.expression && this.operator.length === 1) return this; + return make_node(AST_UnaryPrefix, this, { + operator: this.operator.length === 1 ? this.operator : "!", + expression: expression + }); + } + return expression; } }); def(AST_SymbolRef, function() { @@ -2027,10 +2037,6 @@ merge(Compressor.prototype, { OPT(AST_SimpleStatement, function(self, compressor){ if (compressor.option("side_effects")) { var body = self.body; - if (!body.has_side_effects(compressor)) { - compressor.warn("Dropping side-effect-free statement [{file}:{line},{col}]", self.start); - return make_node(AST_EmptyStatement, self); - } var node = body.drop_side_effect_free(compressor, true); if (!node) { compressor.warn("Dropping side-effect-free statement [{file}:{line},{col}]", self.start); @@ -2647,17 +2653,10 @@ merge(Compressor.prototype, { } } if (!self.car.has_side_effects(compressor) - && !self.cdr.has_side_effects(compressor) && self.car.equivalent_to(self.cdr)) { return self.car; } } - if (self.cdr instanceof AST_UnaryPrefix - && self.cdr.operator == "void" - && !self.cdr.expression.has_side_effects(compressor)) { - self.cdr.expression = self.car; - return self.cdr; - } if (self.cdr instanceof AST_Undefined) { return make_node(AST_UnaryPrefix, self, { operator : "void", @@ -2686,8 +2685,20 @@ merge(Compressor.prototype, { }); OPT(AST_UnaryPrefix, function(self, compressor){ - self = self.lift_sequences(compressor); + var seq = self.lift_sequences(compressor); + if (seq !== self) { + return seq; + } var e = self.expression; + if (compressor.option("side_effects") && self.operator == "void") { + e = e.drop_side_effect_free(compressor); + if (e) { + self.expression = e; + return self; + } else { + return make_node(AST_Undefined, self); + } + } if (compressor.option("booleans") && compressor.in_boolean_context()) { switch (self.operator) { case "!": @@ -2704,13 +2715,10 @@ merge(Compressor.prototype, { // typeof always returns a non-empty string, thus it's // always true in booleans compressor.warn("Boolean expression always true [{file}:{line},{col}]", self.start); - if (self.expression.has_side_effects(compressor)) { - return make_node(AST_Seq, self, { - car: self.expression, - cdr: make_node(AST_True, self) - }); - } - return make_node(AST_True, self); + return make_node(AST_Seq, self, { + car: e, + cdr: make_node(AST_True, self) + }).optimize(compressor); } } return self.evaluate(compressor)[0]; @@ -2840,13 +2848,10 @@ merge(Compressor.prototype, { var rr = self.right.evaluate(compressor); if ((ll.length > 1 && !ll[1]) || (rr.length > 1 && !rr[1])) { compressor.warn("Boolean && always false [{file}:{line},{col}]", self.start); - if (self.left.has_side_effects(compressor)) { - return make_node(AST_Seq, self, { - car: self.left, - cdr: make_node(AST_False) - }).optimize(compressor); - } - return make_node(AST_False, self); + return make_node(AST_Seq, self, { + car: self.left, + cdr: make_node(AST_False) + }).optimize(compressor); } if (ll.length > 1 && ll[1]) { return rr[0]; @@ -2860,13 +2865,10 @@ merge(Compressor.prototype, { var rr = self.right.evaluate(compressor); if ((ll.length > 1 && ll[1]) || (rr.length > 1 && rr[1])) { compressor.warn("Boolean || always true [{file}:{line},{col}]", self.start); - if (self.left.has_side_effects(compressor)) { - return make_node(AST_Seq, self, { - car: self.left, - cdr: make_node(AST_True) - }).optimize(compressor); - } - return make_node(AST_True, self); + return make_node(AST_Seq, self, { + car: self.left, + cdr: make_node(AST_True, self) + }).optimize(compressor); } if (ll.length > 1 && !ll[1]) { return rr[0]; @@ -2878,10 +2880,19 @@ merge(Compressor.prototype, { case "+": var ll = self.left.evaluate(compressor); var rr = self.right.evaluate(compressor); - if ((ll.length > 1 && ll[0] instanceof AST_String && ll[1] && !self.right.has_side_effects(compressor)) || - (rr.length > 1 && rr[0] instanceof AST_String && rr[1] && !self.left.has_side_effects(compressor))) { + if (ll.length > 1 && ll[0] instanceof AST_String && ll[1]) { + compressor.warn("+ in boolean context always true [{file}:{line},{col}]", self.start); + return make_node(AST_Seq, self, { + car: self.right, + cdr: make_node(AST_True, self) + }).optimize(compressor); + } + if (rr.length > 1 && rr[0] instanceof AST_String && rr[1]) { compressor.warn("+ in boolean context always true [{file}:{line},{col}]", self.start); - return make_node(AST_True, self); + return make_node(AST_Seq, self, { + car: self.left, + cdr: make_node(AST_True, self) + }).optimize(compressor); } break; } @@ -3131,7 +3142,8 @@ merge(Compressor.prototype, { && alternative instanceof AST_Assign && consequent.operator == alternative.operator && consequent.left.equivalent_to(alternative.left) - && !consequent.left.has_side_effects(compressor) + && (!consequent.left.has_side_effects(compressor) + || !self.condition.has_side_effects(compressor)) ) { /* * Stuff like this: @@ -3149,25 +3161,19 @@ merge(Compressor.prototype, { }) }); } + // x ? y(a) : y(b) --> y(x ? a : b) if (consequent instanceof AST_Call && alternative.TYPE === consequent.TYPE - && consequent.args.length == alternative.args.length - && !consequent.expression.has_side_effects(compressor) - && consequent.expression.equivalent_to(alternative.expression)) { - if (consequent.args.length == 0) { - return make_node(AST_Seq, self, { - car: self.condition, - cdr: consequent - }); - } - if (consequent.args.length == 1) { - consequent.args[0] = make_node(AST_Conditional, self, { - condition: self.condition, - consequent: consequent.args[0], - alternative: alternative.args[0] - }); - return consequent; - } + && consequent.args.length == 1 + && alternative.args.length == 1 + && consequent.expression.equivalent_to(alternative.expression) + && !consequent.expression.has_side_effects(compressor)) { + consequent.args[0] = make_node(AST_Conditional, self, { + condition: self.condition, + consequent: consequent.args[0], + alternative: alternative.args[0] + }); + return consequent; } // x?y?z:a:a --> x&&y?z:a if (consequent instanceof AST_Conditional @@ -3182,16 +3188,12 @@ merge(Compressor.prototype, { alternative: alternative }); } - // y?1:1 --> 1 - if (consequent.is_constant() - && alternative.is_constant() - && consequent.equivalent_to(alternative)) { - var consequent_value = consequent.evaluate(compressor)[0]; - if (self.condition.has_side_effects(compressor)) { - return AST_Seq.from_array([self.condition, consequent_value]); - } else { - return consequent_value; - } + // x ? y : y --> x, y + if (consequent.equivalent_to(alternative)) { + return make_node(AST_Seq, self, { + car: self.condition, + cdr: consequent + }).optimize(compressor); } if (is_true(self.consequent)) { @@ -3350,8 +3352,12 @@ merge(Compressor.prototype, { }); function literals_in_boolean_context(self, compressor) { - if (compressor.option("booleans") && compressor.in_boolean_context() && !self.has_side_effects(compressor)) { - return make_node(AST_True, self); + if (compressor.option("booleans") && compressor.in_boolean_context()) { + var best = first_in_statement(compressor) ? best_of_statement : best_of; + return best(self, make_node(AST_Seq, self, { + car: self, + cdr: make_node(AST_True, self) + }).optimize(compressor)); } return self; }; diff --git a/test/compress/conditionals.js b/test/compress/conditionals.js index d88c5b90..074d2a65 100644 --- a/test/compress/conditionals.js +++ b/test/compress/conditionals.js @@ -50,7 +50,8 @@ ifs_3_should_warn: { conditionals : true, dead_code : true, evaluate : true, - booleans : true + booleans : true, + side_effects : true, }; input: { var x, y; @@ -135,16 +136,28 @@ ifs_6: { comparisons: true }; input: { - var x; + var x, y; if (!foo && !bar && !baz && !boo) { x = 10; } else { x = 20; } + if (y) { + x[foo] = 10; + } else { + x[foo] = 20; + } + if (foo) { + x[bar] = 10; + } else { + x[bar] = 20; + } } expect: { - var x; + var x, y; x = foo || bar || baz || boo ? 20 : 10; + x[foo] = y ? 10 : 20; + foo ? x[bar] = 10 : x[bar] = 20; } } @@ -159,10 +172,16 @@ cond_1: { } else { do_something(y); } + if (some_condition()) { + side_effects(x); + } else { + side_effects(y); + } } expect: { var do_something; do_something(some_condition() ? x : y); + some_condition() ? side_effects(x) : side_effects(y); } } @@ -213,10 +232,16 @@ cond_4: { } else { do_something(); } + if (some_condition()) { + side_effects(); + } else { + side_effects(); + } } expect: { var do_something; some_condition(), do_something(); + some_condition(), side_effects(); } } @@ -250,7 +275,8 @@ cond_5: { cond_7: { options = { conditionals: true, - evaluate : true + evaluate : true, + side_effects: true, }; input: { var x, y, z, a, b; @@ -714,6 +740,7 @@ issue_1154: { conditionals: true, evaluate : true, booleans : true, + side_effects: true, }; input: { function f1(x) { return x ? -1 : -1; } @@ -742,7 +769,7 @@ issue_1154: { function g2() { return g(), 2; } function g3() { return g(), -4; } function g4() { return g(), !1; } - function g5() { return g(), void 0; } + function g5() { return void g(); } function g6() { return g(), "number"; } } } @@ -750,7 +777,8 @@ issue_1154: { no_evaluate: { options = { conditionals: true, - evaluate : false + evaluate : false, + side_effects: true, } input: { function f(b) { diff --git a/test/compress/dead-code.js b/test/compress/dead-code.js index 2596e80e..cd96d02d 100644 --- a/test/compress/dead-code.js +++ b/test/compress/dead-code.js @@ -64,7 +64,8 @@ dead_code_constant_boolean_should_warn_more: { loops : true, booleans : true, conditionals : true, - evaluate : true + evaluate : true, + side_effects : true, }; input: { while (!((foo && bar) || (x + "0"))) { diff --git a/test/compress/evaluate.js b/test/compress/evaluate.js index 5cefadc8..6bed73fb 100644 --- a/test/compress/evaluate.js +++ b/test/compress/evaluate.js @@ -624,25 +624,35 @@ in_boolean_context: { options = { booleans: true, evaluate: true, + sequences: true, + side_effects: true, } input: { - !42; - !"foo"; - ![1, 2]; - !/foo/; - !b(42); - !b("foo"); - !b([1, 2]); - !b(/foo/); + console.log( + !42, + !"foo", + ![1, 2], + !/foo/, + !b(42), + !b("foo"), + !b([1, 2]), + !b(/foo/), + ![1, foo()], + ![1, foo(), 2] + ); } expect: { - !1; - !1; - !1; - !1; - !b(42); - !b("foo"); - !b([1, 2]); - !b(/foo/); + console.log( + !1, + !1, + !1, + !1, + !b(42), + !b("foo"), + !b([1, 2]), + !b(/foo/), + ![1, foo()], + (foo(), !1) + ); } } diff --git a/test/compress/issue-1261.js b/test/compress/issue-1261.js index dfbe2100..a872c578 100644 --- a/test/compress/issue-1261.js +++ b/test/compress/issue-1261.js @@ -116,3 +116,61 @@ pure_function_calls_toplevel: { "WARN: Dropping unused variable iife1 [test/compress/issue-1261.js:84,12]", ] } + +should_warn: { + options = { + booleans: true, + conditionals: true, + evaluate: true, + side_effects: true, + } + input: { + /* @__PURE__ */(function(){x})(), void/* @__PURE__ */(function(){y})(); + /* @__PURE__ */(function(){x})() || true ? foo() : bar(); + true || /* @__PURE__ */(function(){y})() ? foo() : bar(); + /* @__PURE__ */(function(){x})() && false ? foo() : bar(); + false && /* @__PURE__ */(function(){y})() ? foo() : bar(); + /* @__PURE__ */(function(){x})() + "foo" ? bar() : baz(); + "foo" + /* @__PURE__ */(function(){y})() ? bar() : baz(); + /* @__PURE__ */(function(){x})() ? foo() : foo(); + [/* @__PURE__ */(function(){x})()] ? foo() : bar(); + !{ foo: /* @__PURE__ */(function(){x})() } ? bar() : baz(); + } + expect: { + foo(); + foo(); + bar(); + bar(); + bar(); + bar(); + foo(); + foo(); + baz(); + } + expect_warnings: [ + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:128,61]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:128,23]", + "WARN: Dropping side-effect-free statement [test/compress/issue-1261.js:128,23]", + "WARN: Boolean || always true [test/compress/issue-1261.js:129,23]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:129,23]", + "WARN: Condition always true [test/compress/issue-1261.js:129,23]", + "WARN: Boolean || always true [test/compress/issue-1261.js:130,8]", + "WARN: Condition always true [test/compress/issue-1261.js:130,8]", + "WARN: Boolean && always false [test/compress/issue-1261.js:131,23]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:131,23]", + "WARN: Condition always false [test/compress/issue-1261.js:131,23]", + "WARN: Boolean && always false [test/compress/issue-1261.js:132,8]", + "WARN: Condition always false [test/compress/issue-1261.js:132,8]", + "WARN: + in boolean context always true [test/compress/issue-1261.js:133,23]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:133,23]", + "WARN: Condition always true [test/compress/issue-1261.js:133,23]", + "WARN: + in boolean context always true [test/compress/issue-1261.js:134,8]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:134,31]", + "WARN: Condition always true [test/compress/issue-1261.js:134,8]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:135,23]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:136,24]", + "WARN: Condition always true [test/compress/issue-1261.js:136,8]", + "WARN: Dropping __PURE__ call [test/compress/issue-1261.js:137,31]", + "WARN: Condition always false [test/compress/issue-1261.js:137,8]", + ] +} diff --git a/test/compress/issue-1275.js b/test/compress/issue-1275.js index e88e284c..51f696af 100644 --- a/test/compress/issue-1275.js +++ b/test/compress/issue-1275.js @@ -35,7 +35,7 @@ string_plus_optimization: { throw "nope"; } try { - console.log('0' + throwing_function() ? "yes" : "no"); + console.log((throwing_function(), "yes")); } catch (ex) { console.log(ex); } diff --git a/test/compress/typeof.js b/test/compress/typeof.js index fb391573..7bf8e5e3 100644 --- a/test/compress/typeof.js +++ b/test/compress/typeof.js @@ -29,6 +29,7 @@ typeof_in_boolean_context: { booleans : true, evaluate : true, conditionals : true, + side_effects : true, }; input: { function f1(x) { return typeof x ? "yes" : "no"; } @@ -36,12 +37,14 @@ typeof_in_boolean_context: { typeof 0 ? foo() : bar(); !typeof console.log(1); var a = !typeof console.log(2); + if (typeof (1 + foo())); } expect: { function f1(x) { return "yes"; } function f2() { return g(), "Yes"; } foo(); - !(console.log(1), !0); + console.log(1); var a = !(console.log(2), !0); + foo(); } } diff --git a/test/mocha/minify.js b/test/mocha/minify.js index 0cf8c5c1..baac2a41 100644 --- a/test/mocha/minify.js +++ b/test/mocha/minify.js @@ -154,6 +154,17 @@ describe("minify", function() { var code = result.code; assert.strictEqual(code, "// comment1 comment2\nbar();"); }); + it("should not drop #__PURE__ hint if function is retained", function() { + var result = Uglify.minify("var a = /*#__PURE__*/(function(){return 1})();", { + fromString: true, + output: { + comments: "all", + beautify: false, + } + }); + var code = result.code; + assert.strictEqual(code, "var a=/*#__PURE__*/function(){return 1}();"); + }) }); describe("JS_Parse_Error", function() { -- 2.34.1