From: Alex Lam S.L Date: Thu, 19 Jul 2018 06:45:36 +0000 (+0800) Subject: fix corner case in `ie8` (#3216) X-Git-Url: https://git.ndcode.org/public/gitweb.cgi?a=commitdiff_plain;h=cea685f8d98ad198f3a99d8a217183d15650959d;p=UglifyJS.git fix corner case in `ie8` (#3216) fixes #3215 --- diff --git a/lib/compress.js b/lib/compress.js index c28305a4..364bd362 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -1264,6 +1264,9 @@ merge(Compressor.prototype, { if (node instanceof AST_Exit) { return side_effects || lhs instanceof AST_PropAccess || may_modify(lhs); } + if (node instanceof AST_Function) { + return compressor.option("ie8") && node.name && node.name.name in lvalues; + } if (node instanceof AST_PropAccess) { return side_effects || node.expression.may_throw_on_access(compressor); } @@ -1610,10 +1613,7 @@ merge(Compressor.prototype, { if (def.orig.length == 1 && def.orig[0] instanceof AST_SymbolDefun) return false; if (def.scope !== scope) return true; return !all(def.references, function(ref) { - var s = ref.scope; - // "block" scope within AST_Catch - if (s.TYPE == "Scope") s = s.parent_scope; - return s === scope; + return ref.scope.resolve() === scope; }); } @@ -2704,35 +2704,30 @@ merge(Compressor.prototype, { if (right === this.right) return this; var result; switch (this.operator) { - case "&&" : result = left && right; break; - case "||" : result = left || right; break; - case "|" : result = left | right; break; - case "&" : result = left & right; break; - case "^" : result = left ^ right; break; - case "+" : result = left + right; break; - case "*" : result = left * right; break; - case "/" : result = left / right; break; - case "%" : result = left % right; break; - case "-" : result = left - right; break; - case "<<" : result = left << right; break; - case ">>" : result = left >> right; break; - case ">>>" : result = left >>> right; break; - case "==" : result = left == right; break; - case "===" : result = left === right; break; - case "!=" : result = left != right; break; - case "!==" : result = left !== right; break; - case "<" : result = left < right; break; - case "<=" : result = left <= right; break; - case ">" : result = left > right; break; - case ">=" : result = left >= right; break; - default: - return this; - } - if (isNaN(result) && compressor.find_parent(AST_With)) { - // leave original expression as is - return this; - } - return result; + case "&&" : result = left && right; break; + case "||" : result = left || right; break; + case "|" : result = left | right; break; + case "&" : result = left & right; break; + case "^" : result = left ^ right; break; + case "+" : result = left + right; break; + case "*" : result = left * right; break; + case "/" : result = left / right; break; + case "%" : result = left % right; break; + case "-" : result = left - right; break; + case "<<" : result = left << right; break; + case ">>" : result = left >> right; break; + case ">>>": result = left >>> right; break; + case "==" : result = left == right; break; + case "===": result = left === right; break; + case "!=" : result = left != right; break; + case "!==": result = left !== right; break; + case "<" : result = left < right; break; + case "<=" : result = left <= right; break; + case ">" : result = left > right; break; + case ">=" : result = left >= right; break; + default : return this; + } + return isNaN(result) && compressor.find_parent(AST_With) ? this : result; }); def(AST_Conditional, function(compressor, cached, depth) { var condition = this.condition._eval(compressor, cached, depth); @@ -3412,6 +3407,14 @@ merge(Compressor.prototype, { init.walk(tw); }); } + var drop_fn_name = compressor.option("keep_fnames") ? return_false : compressor.option("ie8") ? function(def) { + return !compressor.exposed(def) && !def.references.length; + } : function(def) { + // any declarations with same name will overshadow + // name of this anonymous function and can therefore + // never be used anywhere + return !(def.id in in_use_ids) || def.orig.length > 1; + }; // pass 3: we should drop declarations not in_use var tt = new TreeTransformer(function(node, descend, in_list) { var parent = tt.parent(); @@ -3439,15 +3442,8 @@ merge(Compressor.prototype, { } } if (scope !== self) return; - if (node instanceof AST_Function - && node.name - && !compressor.option("ie8") - && !compressor.option("keep_fnames")) { - var def = node.name.definition(); - // any declarations with same name will overshadow - // name of this anonymous function and can therefore - // never be used anywhere - if (!(def.id in in_use_ids) || def.orig.length > 1) node.name = null; + if (node instanceof AST_Function && node.name && drop_fn_name(node.name.definition())) { + node.name = null; } if (node instanceof AST_Lambda && !(node instanceof AST_Accessor)) { var trim = !compressor.option("keep_fargs"); @@ -5726,10 +5722,10 @@ merge(Compressor.prototype, { if (def) { return def.optimize(compressor); } - // testing against !self.scope.uses_with first is an optimization if (!compressor.option("ie8") && is_undeclared_ref(self) - && (!self.scope.uses_with || !compressor.find_parent(AST_With))) { + // testing against `self.scope.uses_with` is an optimization + && !(self.scope.uses_with && compressor.find_parent(AST_With))) { switch (self.name) { case "undefined": return make_node(AST_Undefined, self).optimize(compressor); diff --git a/lib/scope.js b/lib/scope.js index 8ce3e4da..ab207d60 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -118,11 +118,10 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) { descend(); scope = save_scope; defun = save_defun; - return true; // don't descend again in TreeWalker + return true; } if (node instanceof AST_With) { - for (var s = scope; s; s = s.parent_scope) - s.uses_with = true; + for (var s = scope; s; s = s.parent_scope) s.uses_with = true; return; } if (node instanceof AST_Symbol) { @@ -132,18 +131,13 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) { node.thedef = node; node.references = []; } - if (node instanceof AST_SymbolDefun || options.ie8 && node instanceof AST_SymbolLambda) { - // Careful here, the scope where this should be defined is - // the parent scope. The reason is that we enter a new - // scope when we encounter the AST_Defun node (which is - // instanceof AST_Scope) but we get to the symbol a bit - // later. - (node.scope = defun.parent_scope).def_function(node, defun); - } - else if (node instanceof AST_SymbolLambda) { + if (node instanceof AST_SymbolDefun) { + // This should be defined in the parent scope, as we encounter the + // AST_Defun node before getting to its AST_Symbol. + (node.scope = defun.parent_scope.resolve()).def_function(node, defun); + } else if (node instanceof AST_SymbolLambda) { defun.def_function(node, node.name == "arguments" ? undefined : defun); - } - else if (node instanceof AST_SymbolVar) { + } else if (node instanceof AST_SymbolVar) { defun.def_variable(node, node.TYPE == "SymbolVar" ? null : undefined); if (defun !== scope) { node.mark_enclosed(options); @@ -153,8 +147,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) { } node.reference(options); } - } - else if (node instanceof AST_SymbolCatch) { + } else if (node instanceof AST_SymbolCatch) { scope.def_variable(node).defun = defun; } }); @@ -162,9 +155,9 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) { // pass 2: find back references and eval self.globals = new Dictionary(); - var tw = new TreeWalker(function(node, descend) { - if (node instanceof AST_LoopControl && node.label) { - node.label.thedef.references.push(node); + var tw = new TreeWalker(function(node) { + if (node instanceof AST_LoopControl) { + if (node.label) node.label.thedef.references.push(node); return true; } if (node instanceof AST_SymbolRef) { @@ -185,35 +178,43 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options) { return true; } // ensure mangling works if catch reuses a scope variable - var def; - if (node instanceof AST_SymbolCatch && (def = node.definition().redefined())) { - var s = node.scope; - while (s) { + if (node instanceof AST_SymbolCatch) { + var def = node.definition().redefined(); + if (def) for (var s = node.scope; s; s = s.parent_scope) { push_uniq(s.enclosed, def); if (s === def.scope) break; - s = s.parent_scope; } + return true; } }); self.walk(tw); // pass 3: fix up any scoping issue with IE8 - if (options.ie8) { - self.walk(new TreeWalker(function(node, descend) { - if (node instanceof AST_SymbolCatch) { - var name = node.name; - var refs = node.thedef.references; - var scope = node.thedef.defun; - var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node); - refs.forEach(function(ref) { - ref.thedef = def; - ref.reference(options); - }); - node.thedef = def; - node.reference(options); - return true; + if (options.ie8) self.walk(new TreeWalker(function(node) { + if (node instanceof AST_SymbolCatch) { + redefine(node, node.thedef.defun); + return true; + } + if (node instanceof AST_SymbolLambda) { + var def = node.thedef; + if (def.orig.length == 1) { + redefine(node, node.scope.parent_scope); + node.thedef.init = def.init; } - })); + return true; + } + })); + + function redefine(node, scope) { + var name = node.name; + var refs = node.thedef.references; + var def = scope.find_variable(name) || self.globals.get(name) || scope.def_variable(node); + refs.forEach(function(ref) { + ref.thedef = def; + ref.reference(options); + }); + node.thedef = def; + node.reference(options); } }); @@ -252,8 +253,7 @@ AST_Lambda.DEFMETHOD("init_scope_vars", function() { AST_Symbol.DEFMETHOD("mark_enclosed", function(options) { var def = this.definition(); - var s = this.scope; - while (s) { + for (var s = this.scope; s; s = s.parent_scope) { push_uniq(s.enclosed, def); if (options.keep_fnames) { s.functions.each(function(d) { @@ -261,7 +261,6 @@ AST_Symbol.DEFMETHOD("mark_enclosed", function(options) { }); } if (s === def.scope) break; - s = s.parent_scope; } }); @@ -298,6 +297,12 @@ AST_Scope.DEFMETHOD("def_variable", function(symbol, init) { return symbol.thedef = def; }); +AST_Lambda.DEFMETHOD("resolve", return_this); +AST_Scope.DEFMETHOD("resolve", function() { + return this.parent_scope; +}); +AST_Toplevel.DEFMETHOD("resolve", return_this); + function names_in_use(scope, options) { var names = scope.names_in_use; if (!names) { @@ -410,7 +415,7 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options) { var save_nesting = lname; descend(); lname = save_nesting; - return true; // don't descend again in TreeWalker + return true; } if (node instanceof AST_Scope) { descend(); @@ -422,7 +427,9 @@ AST_Toplevel.DEFMETHOD("mangle_names", function(options) { } if (node instanceof AST_Label) { var name; - do name = base54(++lname); while (!is_identifier(name)); + do { + name = base54(++lname); + } while (!is_identifier(name)); node.mangled_name = name; return true; } diff --git a/lib/utils.js b/lib/utils.js index 4a61623a..40b65e2a 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -172,9 +172,8 @@ function string_template(text, props) { } function remove(array, el) { - for (var i = array.length; --i >= 0;) { - if (array[i] === el) array.splice(i, 1); - } + var index = array.indexOf(el); + if (index >= 0) array.splice(index, 1); } function makePredicate(words) { diff --git a/test/compress/collapse_vars.js b/test/compress/collapse_vars.js index 15d8243c..31eb2383 100644 --- a/test/compress/collapse_vars.js +++ b/test/compress/collapse_vars.js @@ -5730,3 +5730,100 @@ issue_3096: { } expect_stdout: "ab" } + +issue_3215_1: { + options = { + collapse_vars: true, + evaluate: true, + ie8: false, + inline: true, + passes: 2, + side_effects: true, + unused: true, + } + input: { + console.log(function a() { + var a = 42; + return typeof a; + }()); + } + expect: { + console.log("number"); + } + expect_stdout: "number" +} + +issue_3215_2: { + options = { + collapse_vars: true, + evaluate: true, + ie8: true, + inline: true, + passes: 2, + side_effects: true, + unused: true, + } + input: { + console.log(function a() { + var a = 42; + return typeof a; + }()); + } + expect: { + console.log(function a() { + var a = 42; + return typeof a; + }()); + } + expect_stdout: "number" +} + +issue_3215_3: { + options = { + collapse_vars: true, + evaluate: true, + ie8: false, + inline: true, + passes: 2, + side_effects: true, + unused: true, + } + input: { + console.log(function() { + var a = 42; + (function a() {}); + return typeof a; + }()); + } + expect: { + console.log("number"); + } + expect_stdout: "number" +} + +issue_3215_4: { + options = { + collapse_vars: true, + evaluate: true, + ie8: true, + inline: true, + passes: 2, + side_effects: true, + unused: true, + } + input: { + console.log(function() { + var a = 42; + (function a() {}); + return typeof a; + }()); + } + expect: { + console.log(function() { + var a = 42; + (function a() {}); + return typeof a; + }()); + } + expect_stdout: "number" +} diff --git a/test/compress/ie8.js b/test/compress/ie8.js index 3fce9ef4..c5a20ed7 100644 --- a/test/compress/ie8.js +++ b/test/compress/ie8.js @@ -458,8 +458,8 @@ issue_2976_2: { } expect: { console.log(function f() { - var n; - return n === f ? "FAIL" : "PASS"; + var o; + return o === f ? "FAIL" : "PASS"; }()); } expect_stdout: "PASS" @@ -477,9 +477,9 @@ issue_2976_3: { }()); } expect: { - console.log(function o() { - var n; - return n === o ? "FAIL" : "PASS"; + console.log(function r() { + var o; + return o === r ? "FAIL" : "PASS"; }()); } expect_stdout: "PASS" @@ -678,6 +678,7 @@ issue_3206_1: { input: { console.log(function() { var foo = function bar() {}; + var baz = function moo() {}; return "function" == typeof bar; }()); } @@ -700,6 +701,7 @@ issue_3206_2: { input: { console.log(function() { var foo = function bar() {}; + var baz = function moo() {}; return "function" == typeof bar; }()); } @@ -711,3 +713,151 @@ issue_3206_2: { } expect_stdout: "false" } + +issue_3215_1: { + mangle = { + ie8: false, + } + input: { + console.log(function foo() { + var bar = function bar(name) { + return "PASS"; + }; + try { + "moo"; + } catch (e) { + bar = function bar(name) { + return "FAIL"; + }; + } + return bar; + }()()); + } + expect: { + console.log(function n() { + var o = function n(o) { + return "PASS"; + }; + try { + "moo"; + } catch (n) { + o = function n(o) { + return "FAIL"; + }; + } + return o; + }()()); + } + expect_stdout: "PASS" +} + +issue_3215_2: { + mangle = { + ie8: true, + } + input: { + console.log(function foo() { + var bar = function bar(name) { + return "PASS"; + }; + try { + "moo"; + } catch (e) { + bar = function bar(name) { + return "FAIL"; + }; + } + return bar; + }()()); + } + expect: { + console.log(function foo() { + var r = function r(o) { + return "PASS"; + }; + try { + "moo"; + } catch (o) { + r = function r(o) { + return "FAIL"; + }; + } + return r; + }()()); + } + expect_stdout: "PASS" +} + +issue_3215_3: { + mangle = { + ie8: false, + } + input: { + console.log(function foo() { + var bar = function bar(name) { + return "FAIL"; + }; + try { + moo; + } catch (e) { + bar = function bar(name) { + return "PASS"; + }; + } + return bar; + }()()); + } + expect: { + console.log(function n() { + var o = function n(o) { + return "FAIL"; + }; + try { + moo; + } catch (n) { + o = function n(o) { + return "PASS"; + }; + } + return o; + }()()); + } + expect_stdout: "PASS" +} + +issue_3215_4: { + mangle = { + ie8: true, + } + input: { + console.log(function foo() { + var bar = function bar(name) { + return "FAIL"; + }; + try { + moo; + } catch (e) { + bar = function bar(name) { + return "PASS"; + }; + } + return bar; + }()()); + } + expect: { + console.log(function foo() { + var r = function r(o) { + return "FAIL"; + }; + try { + moo; + } catch (o) { + r = function r(o) { + return "PASS"; + }; + } + return r; + }()()); + } + expect_stdout: "PASS" +} diff --git a/test/compress/reduce_vars.js b/test/compress/reduce_vars.js index c11d20c2..dad8ca3b 100644 --- a/test/compress/reduce_vars.js +++ b/test/compress/reduce_vars.js @@ -5231,11 +5231,11 @@ defun_catch_4: { try { throw 42; } catch (a) { - function a() {} console.log(a); } } - expect_stdout: true + expect_stdout: "42" + node_version: "<=4" } defun_catch_5: { @@ -5257,10 +5257,10 @@ defun_catch_5: { throw 42; } catch (a) { console.log(a); - function a() {} } } - expect_stdout: true + expect_stdout: "42" + node_version: "<=4" } defun_catch_6: { diff --git a/test/ufuzz.json b/test/ufuzz.json index ef4319b9..d20741a8 100644 --- a/test/ufuzz.json +++ b/test/ufuzz.json @@ -16,6 +16,7 @@ }, {}, { + "ie8": true, "toplevel": true }, {