Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle function/variable name collisions correctly #3451

Merged
merged 1 commit into from Oct 6, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 10 additions & 7 deletions lib/compress.js
Expand Up @@ -1293,6 +1293,7 @@ merge(Compressor.prototype, {
return lhs instanceof AST_PropAccess && lhs.equivalent_to(node.expression);
}
if (node instanceof AST_Debugger) return true;
if (node instanceof AST_Defun) return funarg && lhs.name === node.name.name;
if (node instanceof AST_IterationStatement) return !(node instanceof AST_For);
if (node instanceof AST_LoopControl) return true;
if (node instanceof AST_Try) return true;
Expand Down Expand Up @@ -5226,25 +5227,26 @@ merge(Compressor.prototype, {
return return_value(stat);
}

function var_exists(catches, name) {
return catches[name] || identifier_atom[name] || scope.var_names()[name];
function var_exists(defined, name) {
return defined[name] || identifier_atom[name] || scope.var_names()[name];
}

function can_inject_args(catches, safe_to_inject) {
function can_inject_args(catches, used, safe_to_inject) {
for (var i = 0; i < fn.argnames.length; i++) {
var arg = fn.argnames[i];
if (arg.__unused) continue;
if (!safe_to_inject || var_exists(catches, arg.name)) return false;
used[arg.name] = true;
if (in_loop) in_loop.push(arg.definition());
}
return true;
}

function can_inject_vars(catches, safe_to_inject) {
function can_inject_vars(catches, used, safe_to_inject) {
for (var i = 0; i < fn.body.length; i++) {
var stat = fn.body[i];
if (stat instanceof AST_Defun) {
if (!safe_to_inject || var_exists(catches, stat.name.name)) return false;
if (!safe_to_inject || var_exists(used, stat.name.name)) return false;
continue;
}
if (!(stat instanceof AST_Var)) continue;
Expand Down Expand Up @@ -5273,8 +5275,9 @@ merge(Compressor.prototype, {
var safe_to_inject = (!(scope instanceof AST_Toplevel) || compressor.toplevel.vars)
&& (exp !== fn || fn.parent_scope === compressor.find_parent(AST_Scope));
var inline = compressor.option("inline");
if (!can_inject_vars(catches, inline >= 3 && safe_to_inject)) return false;
if (!can_inject_args(catches, inline >= 2 && safe_to_inject)) return false;
var used = Object.create(catches);
if (!can_inject_args(catches, used, inline >= 2 && safe_to_inject)) return false;
if (!can_inject_vars(catches, used, inline >= 3 && safe_to_inject)) return false;
return !in_loop || in_loop.length == 0 || !is_reachable(fn, in_loop);
}

Expand Down
40 changes: 40 additions & 0 deletions test/compress/collapse_vars.js
Expand Up @@ -6197,3 +6197,43 @@ Infinity_assignment: {
}
expect_stdout: true
}

issue_3439_1: {
options = {
collapse_vars: true,
unused: true,
}
input: {
console.log(typeof function(a) {
function a() {}
return a;
}(42));
}
expect: {
console.log(typeof function(a) {
function a() {}
return a;
}(42));
}
expect_stdout: "function"
}

issue_3439_2: {
options = {
collapse_vars: true,
unused: true,
}
input: {
console.log(typeof function() {
var a = 42;
function a() {}
return a;
}());
}
expect: {
console.log(typeof function() {
return 42;
}());
}
expect_stdout: "number"
}
21 changes: 21 additions & 0 deletions test/compress/functions.js
Expand Up @@ -3149,6 +3149,27 @@ issue_3402: {
]
}

issue_3439: {
options = {
inline: true,
}
input: {
console.log(typeof function() {
return function(a) {
function a() {}
return a;
}(42);
}());
}
expect: {
console.log(typeof function(a) {
function a() {}
return a;
}(42));
}
expect_stdout: "function"
}

issue_3444: {
options = {
inline: true,
Expand Down