From f9946767c9e517561253eed22558fb154daf84d1 Mon Sep 17 00:00:00 2001 From: "Alex Lam S.L" Date: Sun, 4 Oct 2020 18:58:50 +0100 Subject: [PATCH] retrofit `AST_BlockStatement` as block-scoped (#4177) --- lib/ast.js | 61 +++++++++++++++++++----------------- lib/compress.js | 35 +++++++++++---------- test/compress/drop-unused.js | 32 +++++++++++++++++++ test/compress/issue-1105.js | 22 ++++++++----- test/compress/merge_vars.js | 9 ++++-- test/mocha/with.js | 4 +-- 6 files changed, 104 insertions(+), 59 deletions(-) diff --git a/lib/ast.js b/lib/ast.js index 3d972117..7e691c2c 100644 --- a/lib/ast.js +++ b/lib/ast.js @@ -251,9 +251,40 @@ var AST_Block = DEFNODE("Block", "body", { }, }, AST_Statement); +var AST_BlockScope = DEFNODE("BlockScope", "enclosed functions make_def parent_scope variables", { + $documentation: "Base class for all statements introducing a lexical scope", + $propdoc: { + enclosed: "[SymbolDef*/S] a list of all symbol definitions that are accessed from this scope or any subscopes", + functions: "[Object/S] like `variables`, but only lists function declarations", + parent_scope: "[AST_Scope?/S] link to the parent scope", + variables: "[Object/S] a map of name -> SymbolDef for all variables/functions defined in this scope", + }, + clone: function(deep) { + var node = this._clone(deep); + if (this.enclosed) node.enclosed = this.enclosed.slice(); + if (this.functions) node.functions = this.functions.clone(); + if (this.variables) node.variables = this.variables.clone(); + return node; + }, + pinned: function() { + return this.resolve().pinned(); + }, + resolve: function() { + return this.parent_scope.resolve(); + }, + _validate: function() { + if (this.parent_scope == null) return; + if (!(this.parent_scope instanceof AST_BlockScope)) throw new Error("parent_scope must be AST_BlockScope"); + if (!(this.resolve() instanceof AST_Scope)) throw new Error("must be contained within AST_Scope"); + }, +}, AST_Block); + var AST_BlockStatement = DEFNODE("BlockStatement", null, { $documentation: "A block statement", -}, AST_Block); + initialize: function() { + this.variables = new Dictionary(); + }, +}, AST_BlockScope); var AST_EmptyStatement = DEFNODE("EmptyStatement", null, { $documentation: "The empty statement (empty block or simply a semicolon)" @@ -412,34 +443,6 @@ var AST_With = DEFNODE("With", "expression", { /* -----[ scope and functions ]----- */ -var AST_BlockScope = DEFNODE("BlockScope", "enclosed functions make_def parent_scope variables", { - $documentation: "Base class for all statements introducing a lexical scope", - $propdoc: { - enclosed: "[SymbolDef*/S] a list of all symbol definitions that are accessed from this scope or any subscopes", - functions: "[Object/S] like `variables`, but only lists function declarations", - parent_scope: "[AST_Scope?/S] link to the parent scope", - variables: "[Object/S] a map of name -> SymbolDef for all variables/functions defined in this scope", - }, - clone: function(deep) { - var node = this._clone(deep); - if (this.enclosed) node.enclosed = this.enclosed.slice(); - if (this.functions) node.functions = this.functions.clone(); - if (this.variables) node.variables = this.variables.clone(); - return node; - }, - pinned: function() { - return this.resolve().pinned(); - }, - resolve: function() { - return this.parent_scope.resolve(); - }, - _validate: function() { - if (this.parent_scope == null) return; - if (!(this.parent_scope instanceof AST_BlockScope)) throw new Error("parent_scope must be AST_BlockScope"); - if (!(this.resolve() instanceof AST_Scope)) throw new Error("must be contained within AST_Scope"); - }, -}, AST_Block); - var AST_Scope = DEFNODE("Scope", "cname uses_eval uses_with", { $documentation: "Base class for all statements introducing a lexical scope", $propdoc: { diff --git a/lib/compress.js b/lib/compress.js index 30757c44..7535d2bc 100644 --- a/lib/compress.js +++ b/lib/compress.js @@ -368,10 +368,10 @@ merge(Compressor.prototype, { && !(def.init instanceof AST_Function && def.init !== def.scope) && def.init; if (def.fixed instanceof AST_Defun && !all(def.references, function(ref) { - var scope = ref.scope; + var scope = ref.scope.resolve(); do { if (def.scope === scope) return true; - } while (scope instanceof AST_Function && (scope = scope.parent_scope)); + } while (scope instanceof AST_Function && (scope = scope.parent_scope.resolve())); })) { tw.defun_ids[def.id] = false; } @@ -854,7 +854,7 @@ merge(Compressor.prototype, { && !value.pinned() && (!d.in_loop || tw.parent() instanceof AST_Call) || !d.in_loop - && d.scope === this.scope + && d.scope === this.scope.resolve() && value.is_constant_expression(); } else { d.single_use = false; @@ -2585,7 +2585,7 @@ merge(Compressor.prototype, { var lhs = expr.left; if (!(lhs instanceof AST_SymbolRef)) break; if (is_undeclared_ref(lhs)) break; - if (lhs.scope !== scope) break; + if (lhs.scope.resolve() !== scope) break; var def = lhs.definition(); if (def.scope !== scope) break; if (def.orig.length > def.eliminated + 1) break; @@ -5179,12 +5179,12 @@ merge(Compressor.prototype, { if (!(node_def.id in in_use_ids)) { in_use_ids[node_def.id] = true; in_use.push(node_def); - if (node.scope !== node_def.scope) { - var redef = node_def.redefined(); - if (redef && !(redef.id in in_use_ids)) { - in_use_ids[redef.id] = true; - in_use.push(redef); - } + } + if (node.scope !== node_def.scope) { + var redef = node_def.redefined(); + if (redef && !(redef.id in in_use_ids)) { + in_use_ids[redef.id] = true; + in_use.push(redef); } } if (track_assigns(node_def, node)) add_assigns(node_def, node); @@ -7031,6 +7031,9 @@ merge(Compressor.prototype, { var stat = fn.body[i]; if (stat instanceof AST_Defun) { if (!safe_to_inject || var_exists(used, stat.name.name)) return false; + if (!all(stat.enclosed, function(def) { + return def.scope === stat || !catches[def.name]; + })) return false; continue; } if (!(stat instanceof AST_Var)) continue; @@ -7067,7 +7070,7 @@ merge(Compressor.prototype, { } } while (!(scope instanceof AST_Scope)); var safe_to_inject = (!(scope instanceof AST_Toplevel) || compressor.toplevel.vars) - && (exp !== fn || fn.parent_scope === compressor.find_parent(AST_Scope)); + && (exp !== fn || fn.parent_scope.resolve() === compressor.find_parent(AST_Scope)); var inline = compressor.option("inline"); var used = Object.create(catches); if (!can_inject_args(catches, used, inline >= 2 && safe_to_inject)) return false; @@ -8087,7 +8090,7 @@ merge(Compressor.prototype, { if (!compressor.option("ie8") && is_undeclared_ref(self) // testing against `self.scope.uses_with` is an optimization - && !(self.scope.uses_with && compressor.find_parent(AST_With))) { + && !(self.scope.resolve().uses_with && compressor.find_parent(AST_With))) { switch (self.name) { case "undefined": return make_node(AST_Undefined, self).optimize(compressor); @@ -8104,16 +8107,14 @@ merge(Compressor.prototype, { var single_use = def.single_use && !(parent instanceof AST_Call && parent.is_expr_pure(compressor)); if (single_use) { if (fixed instanceof AST_Lambda) { - if ((def.scope !== self.scope || def.in_loop) + if ((def.scope !== self.scope.resolve() || def.in_loop) && (!compressor.option("reduce_funcs") || def.escaped.depth == 1 || fixed.inlined)) { single_use = false; } else if (recursive_ref(compressor, def)) { single_use = false; } else if (fixed.name && fixed.name.definition() !== def) { single_use = false; - } else if (fixed.parent_scope !== self.scope - || !(self.scope instanceof AST_Scope) - || def.orig[0] instanceof AST_SymbolFunarg) { + } else if (fixed.parent_scope !== self.scope.resolve() || def.orig[0] instanceof AST_SymbolFunarg) { single_use = fixed.is_constant_expression(self.scope); if (single_use == "f") { var scope = self.scope; @@ -8894,7 +8895,7 @@ merge(Compressor.prototype, { && expr instanceof AST_SymbolRef && is_arguments(def = expr.definition()) && prop instanceof AST_Number - && (fn = expr.scope) === find_lambda()) { + && (fn = expr.scope.resolve()) === find_lambda()) { var index = prop.value; if (parent instanceof AST_UnaryPrefix && parent.operator == "delete") { if (!def.deleted) def.deleted = []; diff --git a/test/compress/drop-unused.js b/test/compress/drop-unused.js index 160b96b4..616bcaf7 100644 --- a/test/compress/drop-unused.js +++ b/test/compress/drop-unused.js @@ -2993,6 +2993,38 @@ issue_4146: { expect_stdout: "function" } +var_catch_redefined: { + options = { + toplevel: true, + unused: true, + } + input: { + var a = "FAIL"; + try { + throw "PASS"; + } catch (a) { + function f() { + return a; + } + console.log(a); + } + f(); + } + expect: { + var a = "FAIL"; + try { + throw "PASS"; + } catch (a) { + function f() { + return a; + } + console.log(a); + } + f(); + } + expect_stdout: "PASS" +} + single_use_catch_redefined: { options = { reduce_vars: true, diff --git a/test/compress/issue-1105.js b/test/compress/issue-1105.js index df6c7dca..fee6540a 100644 --- a/test/compress/issue-1105.js +++ b/test/compress/issue-1105.js @@ -151,15 +151,18 @@ Infinity_not_in_with_scope: { unused: true, } input: { - var o = { Infinity: 'oInfinity' }; + var o = { Infinity: "FAIL" }; var vInfinity = "Infinity"; vInfinity = Infinity; + console.log(vInfinity); } expect: { - var o = { Infinity: 'oInfinity' } - var vInfinity = "Infinity" - vInfinity = 1/0 + var o = { Infinity: "FAIL" }; + var vInfinity = "Infinity"; + vInfinity = 1/0; + console.log(vInfinity); } + expect_stdout: "Infinity" } Infinity_in_with_scope: { @@ -167,15 +170,18 @@ Infinity_in_with_scope: { unused: true, } input: { - var o = { Infinity: 'oInfinity' }; + var o = { Infinity: "PASS" }; var vInfinity = "Infinity"; with (o) { vInfinity = Infinity; } + console.log(vInfinity); } expect: { - var o = { Infinity: 'oInfinity' } - var vInfinity = "Infinity" - with (o) vInfinity = Infinity + var o = { Infinity: "PASS" }; + var vInfinity = "Infinity"; + with (o) vInfinity = Infinity; + console.log(vInfinity); } + expect_stdout: "PASS" } assorted_Infinity_NaN_undefined_in_with_scope: { diff --git a/test/compress/merge_vars.js b/test/compress/merge_vars.js index a12781d3..28a0135d 100644 --- a/test/compress/merge_vars.js +++ b/test/compress/merge_vars.js @@ -466,7 +466,7 @@ issue_4112: { var o = e; for (e in o); var a = function() {}; - console.log; + console.log(typeof a); return a; } }()); @@ -479,12 +479,15 @@ issue_4112: { var a = e; for (e in a); a = function() {}; - console.log; + console.log(typeof a); return a; } }()); } - expect_stdout: "function" + expect_stdout: [ + "function", + "function", + ] } issue_4115: { diff --git a/test/mocha/with.js b/test/mocha/with.js index c6283800..7c17077e 100644 --- a/test/mocha/with.js +++ b/test/mocha/with.js @@ -17,7 +17,7 @@ describe("With", function() { var ast = UglifyJS.parse("with(e) {f(1, 2)}"); ast.figure_out_scope(); assert.equal(ast.uses_with, true); - assert.equal(ast.body[0].expression.scope.uses_with, true); - assert.equal(ast.body[0].body.body[0].body.expression.scope.uses_with, true); + assert.equal(ast.body[0].expression.scope.resolve().uses_with, true); + assert.equal(ast.body[0].body.body[0].body.expression.scope.resolve().uses_with, true); }); }); -- 2.34.1