Skip to content

Commit

Permalink
close #481; prevent enclosed conflicts in scopes which are pulled int…
Browse files Browse the repository at this point in the history 
…o other scopes through inline
  • Loading branch information
@fabiosantoscode
fabiosantoscode committed Jan 7, 2020
1 parent 6355c91 commit 2ad90a6
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 31 deletions.
64 changes: 35 additions & 29 deletions lib/compress/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -912,18 +912,6 @@ function is_modified(compressor, tw, node, value, level, immutable) {
this.definition().fixed = false;
});

function scope_encloses_variables_in_this_scope(tw, pulled_scope, limit_scope) {
let scope = find_scope(tw);
const child_names = pulled_scope.enclosed.filter(d => !pulled_scope.variables.has(d.name)).map(d => d.name);
if (!child_names.length) return false;
while (scope && !(scope instanceof AST_Toplevel) && scope !== limit_scope) {
if (child_names.some(name => scope.variables.has(name))) {
return true;
}
scope = scope.parent_scope;
}
return false;
}
def_reduce_vars(AST_SymbolRef, function(tw, descend, compressor) {
var d = this.definition();
d.references.push(this);
Expand All @@ -947,12 +935,9 @@ function is_modified(compressor, tw, node, value, level, immutable) {
&& ref_once(tw, compressor, d)
) {
d.single_use =
!(fixed_value instanceof AST_Lambda && scope_encloses_variables_in_this_scope(tw, fixed_value, d.scope))
&& (
fixed_value instanceof AST_Lambda && !fixed_value.pinned()
|| fixed_value instanceof AST_Class
|| d.scope === this.scope && fixed_value.is_constant_expression()
);
fixed_value instanceof AST_Lambda && !fixed_value.pinned()
|| fixed_value instanceof AST_Class
|| d.scope === this.scope && fixed_value.is_constant_expression();
} else {
d.single_use = false;
}
Expand Down Expand Up @@ -1076,7 +1061,7 @@ AST_Toplevel.DEFMETHOD("reset_opt_flags", function(compressor) {
});

AST_Symbol.DEFMETHOD("fixed_value", function() {
var fixed = this.definition().fixed;
var fixed = this.thedef.fixed;
if (!fixed || fixed instanceof AST_Node) return fixed;
return fixed();
});
Expand Down Expand Up @@ -5222,14 +5207,14 @@ def_optimize(AST_Call, function(self, compressor) {
var is_regular_func = is_func && !fn.is_generator && !fn.async;
var can_inline = is_regular_func && compressor.option("inline") && !self.is_expr_pure(compressor);
if (can_inline && stat instanceof AST_Return) {
let value = stat.value;
if (!value || value.is_constant_expression()) {
if (value) {
value = value.clone(true);
let returned = stat.value;
if (!returned || returned.is_constant_expression()) {
if (returned) {
returned = returned.clone(true);
} else {
value = make_node(AST_Undefined, self);
returned = make_node(AST_Undefined, self);
}
var args = self.args.concat(value);
const args = self.args.concat(returned);
return make_sequence(self, args).optimize(compressor);
}

Expand All @@ -5238,8 +5223,8 @@ def_optimize(AST_Call, function(self, compressor) {
fn.argnames.length === 1
&& (fn.argnames[0] instanceof AST_SymbolFunarg)
&& self.args.length < 2
&& value instanceof AST_SymbolRef
&& value.name === fn.argnames[0].name
&& returned instanceof AST_SymbolRef
&& returned.name === fn.argnames[0].name
) {
// replace call with first argument or undefined if none passed
return (self.args[0] || make_node(AST_Undefined)).optimize(compressor);
Expand All @@ -5264,6 +5249,7 @@ def_optimize(AST_Call, function(self, compressor) {
&& !has_annotation(self, _PURE | _NOINLINE)
&& !fn.contains_this()
&& can_inject_symbols()
&& !scope_encloses_variables_in_this_scope(scope, fn)
&& !(scope instanceof AST_Class)
) {
set_flag(fn, SQUEEZED);
Expand Down Expand Up @@ -6202,7 +6188,9 @@ def_optimize(AST_SymbolRef, function(self, compressor) {
}
var fixed = self.fixed_value();
var single_use = def.single_use
&& !(parent instanceof AST_Call && parent.is_expr_pure(compressor));
&& !(parent instanceof AST_Call
&& (parent.is_expr_pure(compressor))
|| has_annotation(parent, _NOINLINE));
if (single_use && (fixed instanceof AST_Lambda || fixed instanceof AST_Class)) {
if (retain_top_func(fixed, compressor)) {
single_use = false;
Expand All @@ -6226,9 +6214,13 @@ def_optimize(AST_SymbolRef, function(self, compressor) {
}
}
if (single_use && fixed instanceof AST_Lambda) {
const block_scope = find_scope(compressor, true);
single_use =
def.scope === self.scope
|| parent instanceof AST_Call && parent.expression === self;
&& !scope_encloses_variables_in_this_scope(block_scope, fixed)
|| parent instanceof AST_Call
&& parent.expression === self
&& !scope_encloses_variables_in_this_scope(block_scope, fixed);
}
if (single_use && fixed instanceof AST_Class) {
const extends_inert = !fixed.extends
Expand Down Expand Up @@ -6327,6 +6319,20 @@ def_optimize(AST_SymbolRef, function(self, compressor) {
}
});

function scope_encloses_variables_in_this_scope(scope, pulled_scope) {
for (const enclosed of pulled_scope.enclosed) {
if (pulled_scope.variables.has(enclosed.name)) {
continue;
}
const looked_up = scope.find_variable(enclosed.name);
if (looked_up) {
if (looked_up === enclosed) continue;
return true;
}
}
return false;
}

function is_atomic(lhs, self) {
return lhs instanceof AST_SymbolRef || lhs.TYPE === self.TYPE;
}
Expand Down
4 changes: 4 additions & 0 deletions lib/scope.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,10 @@ class SymbolDef {
this.fixed = false;
Object.seal(this);
}
fixed_value() {
if (!this.fixed || this.fixed instanceof AST_Node) return this.fixed;
return this.fixed();
}
unmangleable(options) {
if (!options) options = {};

Expand Down
84 changes: 84 additions & 0 deletions test/compress/inline.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,58 @@ inline_into_scope_conflict: {
expect_stdout: "PASS"
}

inline_into_scope_conflict_enclosed: {
options = {
reduce_vars: true,
inline: true,
unused: true,
toplevel: true
}
input: {
global.same_name = "PASS"

function $(same_name) {
if (same_name) indirection_1(same_name)
}
function indirection_2() {
console.log(same_name)
}
function indirection_1() {
indirection_2()
}
$("FAIL")
}
expect_stdout: "PASS"
}

inline_into_scope_conflict_enclosed_2: {
options = {
reduce_vars: true,
inline: true,
unused: true,
toplevel: true
}
input: {
global.same_name = () => console.log("PASS")
function $(same_name) {
console.log(same_name === undefined ? "PASS" : "FAIL")
indirection_1();
}
function indirection_1() {
return indirection_2()
}
function indirection_2() {
for (const x of [1]) { same_name(); return; }
}
$();
}
expect_stdout: [
"PASS",
"PASS"
]
}


noinline_annotation: {
options = {
reduce_vars: true,
Expand Down Expand Up @@ -199,6 +251,38 @@ noinline_annotation_2: {
}
}

noinline_annotation_3: {
options = {
reduce_vars: true,
inline: true,
unused: true
}
input: {
(function() {
function foo(val) { return val; }
function bar() {
var pass = 1;
pass = /*@__NOINLINE__*/ foo(pass);
window.data = pass;
}
window.bar = bar;
bar();
})();
}
expect: {
(function() {
function foo(val) { return val; }
function bar() {
var pass = 1;
pass = foo(pass);
window.data = pass;
}
window.bar = bar;
bar();
})();
}
}

inline_annotation: {
options = {
reduce_vars: true,
Expand Down
10 changes: 8 additions & 2 deletions test/compress/issue-t292.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,14 @@ no_flatten_with_arg_colliding_with_arg_value_inner_scope: {
}
expect: {
var g=["a"];
function problem(arg){return g.indexOf(arg)}
console.log(function(problem){return g[problem]}(function(arg){return problem(arg)}("a")));
function problem(arg) {
return g.indexOf(arg)
}
console.log(function(problem) {
return g[problem]
}(function(arg) {
return problem(arg)
}("a")));
}
expect_stdout: "a"
}
Expand Down

0 comments on commit 2ad90a6

Please sign in to comment.