From 0bedd031da4fa8f0eef0fbe6737aa3a4c156a61e Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Wed, 18 Nov 2020 00:22:54 +0000 Subject: [PATCH] fix corner cases in `collapse_vars`, `unused` & `varify` (#4292) fixes #4290 --- lib/compress.js | 73 ++++++++++++++++++++++------------------- test/compress/const.js | 15 +++++++++ test/compress/let.js | 50 ++++++++++++++++++++++++++++ test/compress/varify.js | 37 +++++++++++++++++++++ 4 files changed, 141 insertions(+), 34 deletions(-) diff --git a/lib/compress.js b/lib/compress.js index c86bb5f0..9adcec8c 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -368,7 +368,7 @@ merge(Compressor.prototype, { } while (sym = sym.parent_scope); } - function can_drop_symbol(tw, ref, keep_lambda) { + function can_drop_symbol(ref, keep_lambda) { var orig = ref.definition().orig; if (ref.in_arg && (orig[0] instanceof AST_SymbolFunarg || orig[1] instanceof AST_SymbolFunarg)) return false; return all(orig, function(sym) { @@ -678,7 +678,7 @@ merge(Compressor.prototype, { var d = sym.definition(); d.assignments++; if (!is_modified(compressor, tw, node, node.right, 0) - && can_drop_symbol(tw, sym) && safe_to_assign(tw, d)) { + && can_drop_symbol(sym) && safe_to_assign(tw, d)) { push_ref(d, sym); mark(tw, d); tw.loop_ids[d.id] = tw.in_loop; @@ -1706,9 +1706,11 @@ merge(Compressor.prototype, { if (node instanceof AST_DWLoop) return true; if (node instanceof AST_LoopControl) return true; if (node instanceof AST_SymbolRef) { - if (node.is_declared(compressor) ? node.fixed_value() || all(node.definition().orig, function(sym) { - return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet); - }) : parent instanceof AST_Assign && parent.operator == "=" && parent.left === node) return false; + if (node.is_declared(compressor)) { + if (node.fixed_value() || can_drop_symbol(node)) return false; + } else if (parent instanceof AST_Assign && parent.operator == "=" && parent.left === node) { + return false; + } if (!replace_all) return true; scan_rhs = false; return false; @@ -1779,14 +1781,12 @@ merge(Compressor.prototype, { } if (side_effects && may_modify(node)) return true; var def = node.definition(); - return (in_try || def.scope.resolve() !== scope) && !all(def.orig, function(sym) { - return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet); - }); + return (in_try || def.scope.resolve() !== scope) && !can_drop_symbol(node); } if (node instanceof AST_This) return symbol_in_lvalues(node, parent); if (node instanceof AST_VarDef) { if ((in_try || !lhs_local) && node.name instanceof AST_Destructured) return true; - return node.value && node.name.match_symbol(function(node) { + return (node.value || parent instanceof AST_Let) && node.name.match_symbol(function(node) { return node instanceof AST_SymbolDeclaration && (lvalues.has(node.name) || side_effects && may_modify(node)); }); @@ -4304,7 +4304,7 @@ merge(Compressor.prototype, { }); def(AST_SymbolDeclaration, return_false); def(AST_SymbolRef, function(compressor) { - return !this.is_declared(compressor) || !can_drop_symbol(compressor, this); + return !this.is_declared(compressor) || !can_drop_symbol(this); }); def(AST_This, return_false); def(AST_Try, function(compressor) { @@ -4974,7 +4974,7 @@ merge(Compressor.prototype, { var self = this; var drop_funcs = !(self instanceof AST_Toplevel) || compressor.toplevel.funcs; var drop_vars = !(self instanceof AST_Toplevel) || compressor.toplevel.vars; - var assign_as_unused = /keep_assign/.test(compressor.option("unused")) ? return_false : function(tw, node, props) { + var assign_as_unused = /keep_assign/.test(compressor.option("unused")) ? return_false : function(node, props) { var sym, nested = false; if (node instanceof AST_Assign) { if (node.write_only || node.operator == "=") sym = node.left; @@ -4990,7 +4990,7 @@ merge(Compressor.prototype, { } if (!(sym instanceof AST_SymbolRef)) return; if (compressor.exposed(sym.definition())) return; - if (!can_drop_symbol(tw, sym, nested)) return; + if (!can_drop_symbol(sym, nested)) return; return sym; }; var assign_in_use = Object.create(null); @@ -5046,8 +5046,9 @@ merge(Compressor.prototype, { var redef = def.redefined(); if (redef) var_defs_by_id.add(redef.id, defn); } - if ((!drop_vars || (node instanceof AST_Const ? def.redefined() : def.const_redefs)) - && !(def.id in in_use_ids)) { + if (!(def.id in in_use_ids) && (!drop_vars + || (node instanceof AST_Const ? def.redefined() : def.const_redefs) + || !(node instanceof AST_Var || is_safe_lexical(def)))) { in_use_ids[def.id] = true; in_use.push(def); } @@ -5128,7 +5129,7 @@ merge(Compressor.prototype, { var tt = new TreeTransformer(function(node, descend, in_list) { var parent = tt.parent(); if (drop_vars) { - var props = [], sym = assign_as_unused(tt, node, props); + var props = [], sym = assign_as_unused(node, props); if (sym) { var def = sym.definition(); var in_use = def.id in in_use_ids; @@ -5245,6 +5246,7 @@ merge(Compressor.prototype, { // into the next one, or next statement. var side_effects = []; var duplicated = 0; + var is_var = node instanceof AST_Var; node.definitions.forEach(function(def) { if (def.value) def.value = def.value.transform(tt); if (def.name instanceof AST_Destructured) { @@ -5313,7 +5315,11 @@ merge(Compressor.prototype, { return node; } if (node instanceof AST_SymbolDeclaration) { - return !drop_vars || node.definition().id in in_use_ids || is_catch(node) ? node : null; + if (!drop_vars) return node; + if (node.definition().id in in_use_ids) return node; + if (is_catch(node)) return node; + if (is_var && !can_drop_symbol(node)) return node; + return null; } }); var name = def.name.transform(trimmer); @@ -5326,7 +5332,8 @@ merge(Compressor.prototype, { return; } var sym = def.name.definition(); - if (!drop_vars || sym.id in in_use_ids) { + var drop_sym = !is_var || can_drop_symbol(def.name); + if (!drop_sym || !drop_vars || sym.id in in_use_ids) { if (def.value && indexOf_assign(sym, def) < 0) { var write_only = def.value.write_only; var value = def.value.drop_side_effect_free(compressor); @@ -5343,7 +5350,7 @@ merge(Compressor.prototype, { } var old_def, var_defs = var_defs_by_id.get(sym.id); if (!def.value && !(node instanceof AST_Let)) { - if (var_defs.length > 1) { + if (drop_sym && var_defs.length > 1) { AST_Node.info("Dropping declaration of variable {name} [{file}:{line},{col}]", template(def.name)); remove(var_defs, def); sym.eliminated++; @@ -5376,7 +5383,7 @@ merge(Compressor.prototype, { }); body.push(defun); } else { - if (var_defs.length > 1 && sym.orig.indexOf(def.name) > sym.eliminated) { + if (drop_sym && var_defs.length > 1 && sym.orig.indexOf(def.name) > sym.eliminated) { remove(var_defs, def); duplicated++; } @@ -5661,7 +5668,7 @@ merge(Compressor.prototype, { var def = node.expression.definition(); if (def.scope === self) assignments.add(def.id, node); } - var node_def, props = [], sym = assign_as_unused(tw, node, props); + var node_def, props = [], sym = assign_as_unused(node, props); if (sym && self.variables.get(sym.name) === (node_def = sym.definition())) { props.forEach(function(prop) { prop.walk(tw); @@ -6079,9 +6086,7 @@ merge(Compressor.prototype, { && all(def.references, function(ref) { return ref.fixed_value() === right; }) - && all(def.orig, function(sym) { - return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet); - }); + && can_drop_symbol(sym); } }); @@ -6299,7 +6304,7 @@ merge(Compressor.prototype, { return make_sequence(this, [ expression, property ]); }); def(AST_SymbolRef, function(compressor) { - return this.is_declared(compressor) && can_drop_symbol(compressor, this) ? null : this; + return this.is_declared(compressor) && can_drop_symbol(this) ? null : this; }); def(AST_This, return_null); def(AST_Unary, function(compressor, first_in_statement) { @@ -6308,9 +6313,7 @@ merge(Compressor.prototype, { this.write_only = !exp.has_side_effects(compressor); return this; } - if (this.operator == "typeof" && exp instanceof AST_SymbolRef && can_drop_symbol(compressor, exp)) { - return null; - } + if (this.operator == "typeof" && exp instanceof AST_SymbolRef && can_drop_symbol(exp)) return null; var node = exp.drop_side_effect_free(compressor, first_in_statement); if (first_in_statement && node && is_iife_call(node)) { if (node === exp && this.operator == "!") return this; @@ -7080,6 +7083,10 @@ merge(Compressor.prototype, { return make_sequence(this, assignments); }); + function is_safe_lexical(def) { + return def.orig.length < (def.orig[0] instanceof AST_SymbolLambda ? 3 : 2); + } + function may_overlap(compressor, def) { if (compressor.exposed(def)) return true; var scope = def.scope.resolve(); @@ -7109,7 +7116,8 @@ merge(Compressor.prototype, { return compressor.option("varify") && all(self.definitions, function(defn) { return !defn.name.match_symbol(function(node) { if (node instanceof AST_SymbolDeclaration) { - return !node.fixed_value() || may_overlap(compressor, node.definition()); + var def = node.definition(); + return !node.fixed_value() || !is_safe_lexical(def) || may_overlap(compressor, def); } }, true); }) ? to_var(self) : self; @@ -8015,12 +8023,9 @@ merge(Compressor.prototype, { // typeof always returns a non-empty string, thus it's // always true in booleans AST_Node.warn("Boolean expression always true [{file}:{line},{col}]", self.start); - return (exp instanceof AST_SymbolRef && all(exp.definition().orig, function(sym) { - return !(sym instanceof AST_SymbolConst || sym instanceof AST_SymbolLet); - }) ? make_node(AST_True, self) : make_sequence(self, [ - exp, - make_node(AST_True, self) - ])).optimize(compressor); + var exprs = [ make_node(AST_True, self) ]; + if (!(exp instanceof AST_SymbolRef && can_drop_symbol(exp))) exprs.unshift(exp); + return make_sequence(self, exprs).optimize(compressor); } } if (op == "-" && exp instanceof AST_Infinity) exp = exp.transform(compressor); diff --git a/test/compress/const.js b/test/compress/const.js index 333171b4..0f5e3fdd 100644 --- a/test/compress/const.js +++ b/test/compress/const.js @@ -1225,3 +1225,18 @@ issue_4274_2: { } expect_stdout: true } + +issue_4290_1: { + options = { + unused: true, + } + input: { + const a = 0; + var a; + } + expect: { + const a = 0; + var a; + } + expect_stdout: true +} diff --git a/test/compress/let.js b/test/compress/let.js index 20bec4d3..0476c431 100644 --- a/test/compress/let.js +++ b/test/compress/let.js @@ -1064,3 +1064,53 @@ issue_4276_2: { expect_stdout: "PASS" node_version: ">=4" } + +issue_4290_1: { + options = { + unused: true, + } + input: { + "use strict"; + let a; + var a; + } + expect: { + "use strict"; + let a; + var a; + } + expect_stdout: true + node_version: ">=4" +} + +issue_4290_2: { + options = { + collapse_vars: true, + } + input: { + "use strict"; + try { + console.log(function(a) { + a = c; + let c; + return a; + }()); + } catch (e) { + console.log("PASS"); + } + } + expect: { + "use strict"; + try { + console.log(function(a) { + a = c; + let c; + return a; + }()); + } catch (e) { + console.log("PASS"); + } + } + expect_stdout: "PASS" + node_version: ">=4" +} diff --git a/test/compress/varify.js b/test/compress/varify.js index d4894d2d..95acd032 100644 --- a/test/compress/varify.js +++ b/test/compress/varify.js @@ -353,3 +353,40 @@ forin_let_2: { ] node_version: ">=6" } + +issue_4290_1_const: { + options = { + reduce_vars: true, + toplevel: true, + varify: true, + } + input: { + const a = 0; + var a; + } + expect: { + const a = 0; + var a; + } + expect_stdout: true +} + +issue_4290_1_let: { + options = { + reduce_vars: true, + toplevel: true, + varify: true, + } + input: { + "use strict"; + let a = 0; + var a; + } + expect: { + "use strict"; + let a = 0; + var a; + } + expect_stdout: true + node_version: ">=4" +} -- 2.34.1