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

close #1338 where possible #1360

Merged
merged 3 commits into from
Mar 21, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 3 additions & 17 deletions lib/ast.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,21 +249,8 @@ var AST_SimpleStatement = DEFNODE("SimpleStatement", "body", function AST_Simple

function walk_body(node, visitor) {
const body = node.body;
if (visitor.walk_defun_first) {
for (var i = 0, len = body.length; i < len; i++) {
if (body[i] instanceof AST_Defun) {
body[i]._walk(visitor);
}
}
for (var i = 0, len = body.length; i < len; i++) {
if (!(body[i] instanceof AST_Defun)) {
body[i]._walk(visitor);
}
}
} else {
for (var i = 0, len = body.length; i < len; i++) {
body[i]._walk(visitor);
}
for (var i = 0, len = body.length; i < len; i++) {
body[i]._walk(visitor);
}
}

Expand Down Expand Up @@ -3042,11 +3029,10 @@ const walk_abort = Symbol("abort walk");
/* -----[ TreeWalker ]----- */

class TreeWalker {
constructor(callback, { walk_defun_first = false } = {}) {
constructor(callback) {
this.visit = callback;
this.stack = [];
this.directives = Object.create(null);
this.walk_defun_first = walk_defun_first;
}

_visit(node, descend) {
Expand Down
2 changes: 1 addition & 1 deletion lib/compress/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -539,7 +539,7 @@ AST_Toplevel.DEFMETHOD("reset_opt_flags", function(compressor) {
}
return node.reduce_vars(preparation, descend, compressor);
}
}, { walk_defun_first: true });
});
// Stack of look-up tables to keep track of whether a `SymbolDef` has been
// properly assigned before use:
// - `push()` & `pop()` when visiting conditional branches
Expand Down
85 changes: 83 additions & 2 deletions lib/compress/reduce-vars.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,11 @@ import {
AST_Number,
AST_ObjectKeyVal,
AST_PropAccess,
AST_Scope,
AST_Sequence,
AST_SimpleStatement,
AST_Symbol,
AST_SymbolBlockDeclaration,
AST_SymbolCatch,
AST_SymbolConst,
AST_SymbolDefun,
Expand All @@ -91,6 +93,7 @@ import {
AST_Yield,

walk,
walk_abort,
walk_body,

_INLINE,
Expand Down Expand Up @@ -447,9 +450,7 @@ def_reduce_vars(AST_Default, function(tw, descend) {

function mark_lambda(tw, descend, compressor) {
clear_flag(this, INLINED);

push(tw);

reset_variables(tw, compressor, this);

var iife;
Expand Down Expand Up @@ -484,9 +485,86 @@ function mark_lambda(tw, descend, compressor) {
descend();
pop(tw);

handle_defined_after_hoist(this);

return true;
}

/**
* It's possible for a hoisted function to use something that's not defined yet. Example:
*
* hoisted();
* var defined_after = true;
* function hoisted() {
* // use defined_after
* }
*
* This function is called on the parent to handle this issue.
*/
function handle_defined_after_hoist(parent) {
const defuns = [];
walk(parent, node => {
if (node === parent) return;
if (node instanceof AST_Defun) defuns.push(node);
if (
node instanceof AST_Scope
|| node instanceof AST_SimpleStatement
) return true;
});

for (const defun of defuns) {
const fname_def = defun.name.definition();
const found_self_ref_in_other_defuns = defuns.some(
d => d !== defun && d.enclosed.indexOf(fname_def) !== -1
);

for (const def of defun.enclosed) {
if (
def.fixed === false
|| def === fname_def
|| def.scope.get_defun_scope() !== parent
) {
continue;
}

// defun is hoisted, so always safe
if (
def.assignments === 0
&& def.orig.length === 1
&& def.orig[0] instanceof AST_SymbolDefun
) {
continue;
}

if (found_self_ref_in_other_defuns) {
def.fixed = false;
continue;
}

// Detect `call_defun(); var used_in_defun = ...`
// Because `used_in_defun` can no longer be fixed
let found_defun = false;
let found_def_after_defun = false;
walk(parent, node => {
if (node === defun) return true;

if (node instanceof AST_Symbol) {
if (!found_defun && node.thedef === fname_def) {
found_defun = true;
} else if (found_defun && node.thedef === def) {
found_def_after_defun = true;
return walk_abort;
}
}
});

if (found_def_after_defun) {
def.fixed = false;
}
}
}
}

def_reduce_vars(AST_Lambda, mark_lambda);

def_reduce_vars(AST_Do, function(tw, descend, compressor) {
Expand Down Expand Up @@ -607,6 +685,9 @@ def_reduce_vars(AST_Toplevel, function(tw, descend, compressor) {
reset_def(compressor, def);
});
reset_variables(tw, compressor, this);
descend();
handle_defined_after_hoist(this);
return true;
});

def_reduce_vars(AST_Try, function(tw, descend, compressor) {
Expand Down
101 changes: 95 additions & 6 deletions test/compress/reduce_vars.js
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,73 @@ defun_reference_1: {
}
}

defun_reference_2: {
defun_reference_indirection_1: {
options = {
toplevel: true,
reduce_vars: true,
unused: true,
evaluate: true
}
input: {
if (id(true)) {
var first = indirection();
var on = true;
function indirection() {
log_on()
}
function log_on() {
console.log(on)
}
}
}
expect: {
if (id(true)) {
(function () {
log_on();
})();
var on = true;
function log_on() {
console.log(on);
}
}
}
expect_stdout: "undefined"
}

defun_reference_indirection_2: {
options = {
toplevel: true,
reduce_vars: true,
unused: true,
evaluate: true
}
input: {
if (id(true)) {
var first = indirection_2();
var on = true;
function log_on() {
console.log(on)
}
function indirection_2() {
log_on()
}
}
}
expect: {
if (id(true)) {
(function () {
log_on();
})();
var on = true;
function log_on() {
console.log(on);
}
}
}
expect_stdout: "undefined"
}

defun_reference_used_before_def: {
options = {
toplevel: true,
reduce_vars: true,
Expand All @@ -1301,7 +1367,7 @@ defun_reference_2: {
expect_stdout: "undefined"
}

defun_reference_3: {
defun_reference_fixed: {
options = {
toplevel: true,
reduce_vars: true,
Expand All @@ -1310,8 +1376,8 @@ defun_reference_3: {
}
input: {
if (id(true)) {
var first = log_on();
var on = true;
var first = log_on();
function log_on() {
console.log(on)
}
Expand All @@ -1320,14 +1386,37 @@ defun_reference_3: {
expect: {
if (id(true)) {
(function () {
console.log(on)
console.log(true)
})();
var on = true;
}
}
expect_stdout: "undefined"
expect_stdout: "true"
}

defun_reference_fixed_let: {
options = {
toplevel: true,
reduce_vars: true,
unused: true,
evaluate: true,
passes: 3
}
input: {
let on = true;
function log_on() {
console.log(on)
}
var first = log_on();
}
expect: {
(function () {
console.log(true)
})();
}
expect_stdout: "true"
}


defun_inline_1: {
options = {
reduce_funcs: true,
Expand Down