From: Alex Lam S.L Date: Sun, 10 Dec 2017 16:16:02 +0000 (+0800) Subject: handle exceptional flow correctly in `collapse_vars` (#2574) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=0e16d92786b8360848a4b56719135facabe7cd85;p=UglifyJS.git handle exceptional flow correctly in `collapse_vars` (#2574) fixes #2571 --- diff --git a/lib/compress.js b/lib/compress.js index ce0fbdd5..434becb6 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -868,6 +868,21 @@ merge(Compressor.prototype, { var stat_index = statements.length; var scanner = new TreeTransformer(function(node, descend) { if (abort) return node; + // Scan case expressions first in a switch statement + if (node instanceof AST_Switch) { + node.expression = node.expression.transform(scanner); + for (var i = 0, len = node.body.length; !abort && i < len; i++) { + var branch = node.body[i]; + if (branch instanceof AST_Case) + branch.expression = branch.expression.transform(scanner); + } + for (i = 0; !abort && i < len; i++) { + var branch = node.body[i]; + for (var j = 0, len2 = branch.body.length; j < len2; j++) + branch.body[j] = branch.body[j].transform(scanner); + } + return node; + } // Skip nodes before `candidate` as quickly as possible if (!hit) { if (node === candidate) { @@ -944,17 +959,17 @@ merge(Compressor.prototype, { || side_effects && !references_in_scope(node.definition())) || (sym = lhs_or_def(node)) && (sym instanceof AST_PropAccess || sym.name in lvalues) + || may_throw && node.has_side_effects(compressor) || (side_effects || !replace_all) && (parent instanceof AST_Binary && lazy_op(parent.operator) - || parent instanceof AST_Case || parent instanceof AST_Conditional || parent instanceof AST_If)) { if (!(node instanceof AST_Scope)) descend(node, scanner); abort = true; return node; } - // Skip (non-executed) functions and (leading) default case in switch statements - if (node instanceof AST_Default || node instanceof AST_Scope) return node; + // Skip (non-executed) functions + if (node instanceof AST_Scope) return node; }); var multi_replacer = new TreeTransformer(function(node) { if (abort) return node; @@ -1000,6 +1015,7 @@ merge(Compressor.prototype, { replace_all = def.references.length - def.replaced == 1; } var side_effects = value_has_side_effects(candidate); + var may_throw = candidate.may_throw(compressor); var funarg = candidate.name instanceof AST_SymbolFunarg; var hit = funarg; var abort = false, replaced = 0, can_replace = !args || !hit; @@ -2200,15 +2216,6 @@ merge(Compressor.prototype, { def(AST_Constant, return_false); def(AST_This, return_false); - def(AST_Call, function(compressor){ - if (!this.is_expr_pure(compressor)) return true; - for (var i = this.args.length; --i >= 0;) { - if (this.args[i].has_side_effects(compressor)) - return true; - } - return false; - }); - function any(list, compressor) { for (var i = list.length; --i >= 0;) if (list[i].has_side_effects(compressor)) @@ -2219,6 +2226,10 @@ merge(Compressor.prototype, { def(AST_Block, function(compressor){ return any(this.body, compressor); }); + def(AST_Call, function(compressor){ + return !this.is_expr_pure(compressor) + || any(this.args, compressor); + }); def(AST_Switch, function(compressor){ return this.expression.has_side_effects(compressor) || any(this.body, compressor); @@ -2243,8 +2254,7 @@ merge(Compressor.prototype, { def(AST_SimpleStatement, function(compressor){ return this.body.has_side_effects(compressor); }); - def(AST_Defun, return_true); - def(AST_Function, return_false); + def(AST_Lambda, return_false); def(AST_Binary, function(compressor){ return this.left.has_side_effects(compressor) || this.right.has_side_effects(compressor); @@ -2282,14 +2292,121 @@ merge(Compressor.prototype, { || this.property.has_side_effects(compressor); }); def(AST_Sequence, function(compressor){ - return this.expressions.some(function(expression, index) { - return expression.has_side_effects(compressor); - }); + return any(this.expressions, compressor); + }); + def(AST_Definitions, function(compressor){ + return any(this.definitions, compressor); + }); + def(AST_VarDef, function(compressor){ + return this.value; }); })(function(node, func){ node.DEFMETHOD("has_side_effects", func); }); + // determine if expression may throw + (function(def){ + def(AST_Node, return_true); + + def(AST_Constant, return_false); + def(AST_EmptyStatement, return_false); + def(AST_Lambda, return_false); + def(AST_SymbolDeclaration, return_false); + def(AST_This, return_false); + + function any(list, compressor) { + for (var i = list.length; --i >= 0;) + if (list[i].may_throw(compressor)) + return true; + return false; + } + + def(AST_Array, function(compressor){ + return any(this.elements, compressor); + }); + def(AST_Assign, function(compressor){ + return this.operator != "=" && this.left.may_throw(compressor) + || this.right.may_throw(compressor); + }); + def(AST_Binary, function(compressor){ + return this.left.may_throw(compressor) + || this.right.may_throw(compressor); + }); + def(AST_Block, function(compressor){ + return any(this.body, compressor); + }); + def(AST_Call, function(compressor){ + if (any(this.args, compressor)) return true; + if (this.is_expr_pure(compressor)) return false; + if (this.expression.may_throw(compressor)) return true; + return !(this.expression instanceof AST_Lambda) + || any(this.expression.body, compressor); + }); + def(AST_Case, function(compressor){ + return this.expression.may_throw(compressor) + || any(this.body, compressor); + }); + def(AST_Conditional, function(compressor){ + return this.condition.may_throw(compressor) + || this.consequent.may_throw(compressor) + || this.alternative.may_throw(compressor); + }); + 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.may_throw(compressor); + }); + def(AST_If, function(compressor){ + return this.condition.may_throw(compressor) + || this.body && this.body.may_throw(compressor) + || this.alternative && this.alternative.may_throw(compressor); + }); + def(AST_LabeledStatement, function(compressor){ + return this.body.may_throw(compressor); + }); + def(AST_Object, function(compressor){ + return any(this.properties, compressor); + }); + def(AST_ObjectProperty, function(compressor){ + return this.value.may_throw(compressor); + }); + def(AST_Sequence, function(compressor){ + return any(this.expressions, compressor); + }); + def(AST_SimpleStatement, function(compressor){ + return this.body.may_throw(compressor); + }); + def(AST_Sub, function(compressor){ + return this.expression.may_throw_on_access(compressor) + || this.expression.may_throw(compressor) + || this.property.may_throw(compressor); + }); + def(AST_Switch, function(compressor){ + return this.expression.may_throw(compressor) + || any(this.body, compressor); + }); + def(AST_SymbolRef, function(compressor){ + return !this.is_declared(compressor); + }); + def(AST_Try, function(compressor){ + return any(this.body, compressor) + || this.bcatch && this.bcatch.may_throw(compressor) + || this.bfinally && this.bfinally.may_throw(compressor); + }); + def(AST_Unary, function(compressor){ + if (this.operator == "typeof" && this.expression instanceof AST_SymbolRef) + return false; + return this.expression.may_throw(compressor); + }); + def(AST_VarDef, function(compressor){ + return this.value.may_throw(compressor); + }); + })(function(node, func){ + node.DEFMETHOD("may_throw", func); + }); + // determine if expression is constant (function(def){ function all(list) { diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index 68313354..ab86c6b4 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -69,10 +69,11 @@ collapse_vars_side_effects_1: { log(x, s.charAt(i++), y, 7); } function f4() { - var i = 10, + var log = console.log.bind(console), + i = 10, x = i += 2, y = i += 3; - console.log.bind(console)(x, i += 4, y, i); + log(x, i += 4, y, i); } f1(), f2(), f3(), f4(); } @@ -152,7 +153,7 @@ collapse_vars_issue_721: { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, - reduce_funcs: true, reduce_vars:true + reduce_funcs: true, reduce_vars:true, passes:2 } input: { define(["require", "exports", 'handlebars'], function (require, exports, hb) { @@ -675,8 +676,8 @@ collapse_vars_lvalues: { function f4(x) { var a = (x -= 3); return x + a; } function f5(x) { var w = e1(), v = e2(), c = v = --x; return (w = x) - c; } function f6(x) { var w = e1(), v = e2(); return (v = --x) - (w = x); } - function f7(x) { var w = e1(); return (w = x) - (e2() - x); } - function f8(x) { var w = e1(); return (w = x) - (e2() - x); } + function f7(x) { var w = e1(), v = e2(); return (w = x) - (v - x); } + function f8(x) { var w = e1(), v = e2(); return (w = x) - (v - x); } function f9(x) { var w = e1(); return e2() - x - (w = x); } } } @@ -685,7 +686,7 @@ collapse_vars_lvalues_drop_assign: { options = { collapse_vars:true, sequences:true, properties:true, dead_code:true, conditionals:true, comparisons:true, evaluate:true, booleans:true, loops:true, unused:true, hoist_funs:true, - keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true + keep_fargs:true, if_return:true, join_vars:true, cascade:true, side_effects:true, passes:3 } input: { function f0(x) { var i = ++x; return x += i; } @@ -706,10 +707,10 @@ collapse_vars_lvalues_drop_assign: { function f3(x) { var a = (x -= 3); return x + a; } function f4(x) { var a = (x -= 3); return x + a; } function f5(x) { e1(), e2(); var c = --x; return x - c; } - function f6(x) { e1(), e2(); return --x - x; } - function f7(x) { e1(); return x - (e2() - x); } - function f8(x) { e1(); return x - (e2() - x); } - function f9(x) { e1(); return e2() - x - x; } + function f6(x) { return e1(), e2(), --x - x; } + function f7(x) { return e1(), x - (e2() - x); } + function f8(x) { return e1(), x - (e2() - x); } + function f9(x) { return e1(), e2() - x - x; } } } @@ -1767,11 +1768,10 @@ switch_case: { } expect: { function f(x, y, z) { - var c = z; switch (x()) { default: d(); case y(): e(); - case c: f(); + case z: f(); } } } @@ -2195,7 +2195,7 @@ issue_315: { expect: { console.log(function() { var w, _i, _len, _ref, _results; - for (_results = [], _i = 0, _len = (_ref = "test".trim().split(" ")).length; _i < _len ; _i++) + for (_ref = "test".trim().split(" "), _results = [], _i = 0, _len = _ref.length; _i < _len ; _i++) w = _ref[_i], _results.push(w.toLowerCase()); return _results; }()); @@ -3108,8 +3108,8 @@ issue_2437: { return Object.defineProperty(XMLHttpRequest.prototype, "onreadystatechange", xhrDesc || {}), result; } - var req, detectFunc = function() {}; - (req = new XMLHttpRequest()).onreadystatechange = detectFunc; + var req = new XMLHttpRequest(), detectFunc = function() {}; + req.onreadystatechange = detectFunc; result = req[SYMBOL_FAKE_ONREADYSTATECHANGE_1] === detectFunc; req.onreadystatechange = null; }(); @@ -3675,3 +3675,57 @@ issue_2506: { } expect_stdout: "1" } + +issue_2571_1: { + options = { + collapse_vars: true, + toplevel: true, + } + input: { + var b = 1; + try { + var a = function f0(c) { + throw c; + }(2); + var d = --b + a; + } catch (e) { + } + console.log(b); + } + expect: { + var b = 1; + try { + var a = function f0(c) { + throw c; + }(2); + var d = --b + a; + } catch (e) { + } + console.log(b); + } + expect_stdout: "1" +} + +issue_2571_2: { + options = { + collapse_vars: true, + toplevel: true, + } + input: { + try { + var a = A, b = 1; + throw a; + } catch (e) { + console.log(b); + } + } + expect: { + try { + var a = A, b = 1; + throw a; + } catch (e) { + console.log(b); + } + } + expect_stdout: "undefined" +}