From f352bcec3ab48b1dafd5fdf698f99398c4cf52ff Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Wed, 29 Aug 2018 11:34:34 +0800 Subject: [PATCH] fix corner case in `collapse_vars` (#3239) fixes #3238 --- lib/compress.js | 105 ++++++++++--------- test/compress/collapse_vars.js | 180 +++++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 48 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index ad86dd84..f84409ff 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -1481,14 +1481,26 @@ merge(Compressor.prototype, { } function get_rhs(expr) { - if (!(candidate instanceof AST_Assign && candidate.operator == "=")) return; - return candidate.right; + return candidate instanceof AST_Assign && candidate.operator == "=" && candidate.right; } function get_rvalue(expr) { return expr[expr instanceof AST_Assign ? "right" : "value"]; } + function invariant(expr) { + if (expr instanceof AST_Array) return false; + if (expr instanceof AST_Binary && lazy_op[expr.operator]) { + return invariant(expr.left) && invariant(expr.right); + } + if (expr instanceof AST_Call) return false; + if (expr instanceof AST_Conditional) { + return invariant(expr.consequent) && invariant(expr.alternative); + } + if (expr instanceof AST_Object) return false; + return !expr.has_side_effects(compressor); + } + function foldable(expr) { if (expr instanceof AST_SymbolRef) { var value = expr.evaluate(compressor); @@ -1501,7 +1513,7 @@ merge(Compressor.prototype, { return rhs_fuzzy_match(expr.evaluate(compressor), rhs_exact_match); } if (!(lhs instanceof AST_SymbolRef)) return false; - if (expr.has_side_effects(compressor)) return false; + if (!invariant(expr)) return false; var circular; var def = lhs.definition(); expr.walk(new TreeWalker(function(node) { @@ -2980,19 +2992,21 @@ merge(Compressor.prototype, { // determine if expression has side effects (function(def) { - def(AST_Node, return_true); - - def(AST_EmptyStatement, return_false); - def(AST_Constant, return_false); - def(AST_This, return_false); - function any(list, compressor) { for (var i = list.length; --i >= 0;) if (list[i].has_side_effects(compressor)) return true; return false; } - + def(AST_Node, return_true); + def(AST_Array, function(compressor) { + return any(this.elements, compressor); + }); + def(AST_Assign, return_true); + def(AST_Binary, function(compressor) { + return this.left.has_side_effects(compressor) + || this.right.has_side_effects(compressor); + }); def(AST_Block, function(compressor) { return any(this.body, compressor); }); @@ -3004,19 +3018,24 @@ merge(Compressor.prototype, { } return any(this.args, compressor); }); - def(AST_Switch, function(compressor) { - return this.expression.has_side_effects(compressor) - || any(this.body, compressor); - }); def(AST_Case, function(compressor) { return this.expression.has_side_effects(compressor) || any(this.body, compressor); }); - def(AST_Try, function(compressor) { - return any(this.body, compressor) - || this.bcatch && this.bcatch.has_side_effects(compressor) - || this.bfinally && this.bfinally.has_side_effects(compressor); + def(AST_Conditional, function(compressor) { + return this.condition.has_side_effects(compressor) + || this.consequent.has_side_effects(compressor) + || this.alternative.has_side_effects(compressor); }); + def(AST_Constant, return_false); + def(AST_Definitions, function(compressor) { + return any(this.definitions, compressor); + }); + def(AST_Dot, function(compressor) { + return this.expression.may_throw_on_access(compressor) + || this.expression.has_side_effects(compressor); + }); + def(AST_EmptyStatement, return_false); def(AST_If, function(compressor) { return this.condition.has_side_effects(compressor) || this.body && this.body.has_side_effects(compressor) @@ -3025,41 +3044,13 @@ merge(Compressor.prototype, { def(AST_LabeledStatement, function(compressor) { return this.body.has_side_effects(compressor); }); - def(AST_SimpleStatement, function(compressor) { - return this.body.has_side_effects(compressor); - }); def(AST_Lambda, return_false); - def(AST_Binary, function(compressor) { - return this.left.has_side_effects(compressor) - || this.right.has_side_effects(compressor); - }); - def(AST_Assign, return_true); - def(AST_Conditional, function(compressor) { - return this.condition.has_side_effects(compressor) - || this.consequent.has_side_effects(compressor) - || this.alternative.has_side_effects(compressor); - }); - def(AST_Unary, function(compressor) { - return unary_side_effects[this.operator] - || this.expression.has_side_effects(compressor); - }); - def(AST_SymbolRef, function(compressor) { - return !this.is_declared(compressor); - }); - def(AST_SymbolDeclaration, return_false); def(AST_Object, function(compressor) { return any(this.properties, compressor); }); def(AST_ObjectProperty, function(compressor) { return this.value.has_side_effects(compressor); }); - def(AST_Array, function(compressor) { - return any(this.elements, compressor); - }); - def(AST_Dot, function(compressor) { - return this.expression.may_throw_on_access(compressor) - || this.expression.has_side_effects(compressor); - }); def(AST_Sub, function(compressor) { return this.expression.may_throw_on_access(compressor) || this.expression.has_side_effects(compressor) @@ -3068,8 +3059,26 @@ merge(Compressor.prototype, { def(AST_Sequence, function(compressor) { return any(this.expressions, compressor); }); - def(AST_Definitions, function(compressor) { - return any(this.definitions, compressor); + def(AST_SimpleStatement, function(compressor) { + return this.body.has_side_effects(compressor); + }); + def(AST_Switch, function(compressor) { + return this.expression.has_side_effects(compressor) + || any(this.body, compressor); + }); + def(AST_SymbolDeclaration, return_false); + def(AST_SymbolRef, function(compressor) { + return !this.is_declared(compressor); + }); + def(AST_This, return_false); + def(AST_Try, function(compressor) { + return any(this.body, compressor) + || this.bcatch && this.bcatch.has_side_effects(compressor) + || this.bfinally && this.bfinally.has_side_effects(compressor); + }); + def(AST_Unary, function(compressor) { + return unary_side_effects[this.operator] + || this.expression.has_side_effects(compressor); }); def(AST_VarDef, function(compressor) { return this.value; diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index 31eb2383..d1c26c68 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -5827,3 +5827,183 @@ issue_3215_4: { } expect_stdout: "number" } + +issue_3238_1: { + options = { + collapse_vars: true, + unsafe: true, + } + input: { + function f(a) { + var b, c; + if (a) { + b = Object.create(null); + c = Object.create(null); + } + return b === c; + } + console.log(f(0), f(1)); + } + expect: { + function f(a) { + var b, c; + if (a) { + b = Object.create(null); + c = Object.create(null); + } + return b === c; + } + console.log(f(0), f(1)); + } + expect_stdout: "true false" +} + +issue_3238_2: { + options = { + collapse_vars: true, + unsafe: true, + } + input: { + function f(a) { + var b, c; + if (a) { + b = Error(); + c = Error(); + } + return b === c; + } + console.log(f(0), f(1)); + } + expect: { + function f(a) { + var b, c; + if (a) { + b = Error(); + c = Error(); + } + return b === c; + } + console.log(f(0), f(1)); + } + expect_stdout: "true false" +} + +issue_3238_3: { + options = { + collapse_vars: true, + unsafe: true, + } + input: { + function f(a) { + var b, c; + if (a) { + b = new Date(); + c = new Date(); + } + return b === c; + } + console.log(f(0), f(1)); + } + expect: { + function f(a) { + var b, c; + if (a) { + b = new Date(); + c = new Date(); + } + return b === c; + } + console.log(f(0), f(1)); + } + expect_stdout: "true false" +} + +issue_3238_4: { + options = { + collapse_vars: true, + unsafe: true, + } + input: { + function f(a) { + var b, c; + if (a) { + b = a && {}; + c = a && {}; + } + return b === c; + } + console.log(f(0), f(1)); + } + expect: { + function f(a) { + var b, c; + if (a) { + b = a && {}; + c = a && {}; + } + return b === c; + } + console.log(f(0), f(1)); + } + expect_stdout: "true false" +} + +issue_3238_5: { + options = { + collapse_vars: true, + unsafe: true, + } + input: { + function f(a) { + var b, c; + if (a) { + b = a ? [] : 42; + c = a ? [] : 42; + } + return b === c; + } + console.log(f(0), f(1)); + } + expect: { + function f(a) { + var b, c; + if (a) { + b = a ? [] : 42; + c = a ? [] : 42; + } + return b === c; + } + console.log(f(0), f(1)); + } + expect_stdout: "true false" +} + +issue_3238_6: { + options = { + collapse_vars: true, + unsafe: true, + } + input: { + function f(a) { + var b, c; + if (a) { + b = a && 0 || []; + c = a && 0 || []; + } + return b === c; + } + console.log(f(0), f(1)); + } + expect: { + function f(a) { + var b, c; + if (a) { + b = a && 0 || []; + c = a && 0 || []; + } + return b === c; + } + console.log(f(0), f(1)); + } + expect_stdout: "true false" +} -- 2.34.1