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

fix hoist_funs on block-scoped function under "use strict" #2013

Merged
merged 2 commits into from
May 27, 2017

Conversation

alexlamsl
Copy link
Collaborator

Technically not part of ES5, but commonly used code exists in the wild.

@kzc this should directly address #1953 (comment)

Technically not part of ES5, but commonly used code exists in the wild.
lib/compress.js Outdated
@@ -2251,7 +2251,8 @@ merge(Compressor.prototype, {
dirs.push(node);
return make_node(AST_EmptyStatement, node);
}
if (node instanceof AST_Defun && hoist_funs) {
if (node instanceof AST_Defun && hoist_funs
&& (tt.parent() === self || !compressor.has_directive("use strict"))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

instanceof is more than 10X slower than a boolean variable check, so could you put the hoist_funs condition first?

I appreciate this would only help the case when hoist_funs is disabled, but it also makes the code clearer.

Copy link
Collaborator Author

@alexlamsl alexlamsl May 27, 2017

Choose a reason for hiding this comment

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

I debated with myself a bit over this performance vs. minimal change thing, but I'll take your vote now that you've casted it 😉

@kzc
Copy link
Contributor

kzc commented May 27, 2017

I see. There's a method to your madness!

@kzc
Copy link
Contributor

kzc commented May 27, 2017

This PR should help to reduce false positives in test/ufuzz.js. It provides "defined" undefined behavior for ES5 running on ES6 engines.

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.

None yet

2 participants