Skip to content

fix AST_Node.optimize()#1602

Merged
alexlamsl merged 1 commit intomishoo:masterfrom
alexlamsl:transform
Mar 15, 2017
Merged

fix AST_Node.optimize()#1602
alexlamsl merged 1 commit intomishoo:masterfrom
alexlamsl:transform

Conversation

@alexlamsl
Copy link
Copy Markdown
Collaborator

Liberal use of Compressor.transform() and AST_Node.optimize() presents an issue for look-up operations like TreeWalker.in_boolean_context() and TreeWalker.parent().

This is an incremental fix such that AST_Node.optimize() would now contain the correct stack information when called correctly.

This is less ambitious than #1593, and attempting to gradually patch up incorrect usages without incurring performance penalties.

Liberal use of `Compressor.transform()` and `AST_Node.optimize()` presents an issue for look-up operations like `TreeWalker.in_boolean_context()` and `TreeWalker.parent()`.

This is an incremental fix such that `AST_Node.optimize()` would now contain the correct stack information when called correctly.
opt._optimized = true;
if (opt === self) return opt;
return opt.transform(compressor);
return opt;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Problem with calling opt.transform(compressor) here is that the stack would change from:

...
...
self

to:

...
...
self
opt

which will cause optimisations that depend knowing where they are in the AST to fail.

The solution is to wait for opt to be assigned on the relevant field in the parent AST_Node, then descend() from the parent again. As this node is already marked as _optimized, only its children be altered by optimize().

if (was_scope && node instanceof AST_Scope) {
node.drop_unused(this);
descend(node, this);
descend(node, this);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the repetition on purpose?

@alexlamsl alexlamsl merged commit 8223b2e into mishoo:master Mar 15, 2017
@alexlamsl alexlamsl deleted the transform branch March 15, 2017 10:44
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this pull request Mar 15, 2017
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this pull request Mar 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants