Skip to content

Fix handling of function declarations in loops with let/consts #1127

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

Merged
merged 5 commits into from
Sep 11, 2015

Conversation

Dominator008
Copy link
Contributor

Convert function declaration to variable declaration so that the IIFE can still be applied.

Unmark calls to captured functions like $jscomp$loop.x() as FREE_CALL.

Fixes #1124

@@ -708,6 +708,46 @@ public void testDoWhileForOfCapturedLet() {
"} while (false);"));
}

public void testIssue1124() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's a convention that "issue 1124" means 1124 on code.google.com. Let's add a comment with a link so it's unambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -343,6 +343,10 @@ private void transformLoopClosure() {
}

// Change reference to GETPROP.
Copy link
Contributor

Choose a reason for hiding this comment

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

Now this comment is misplaced.

@MatrixFrog
Copy link
Contributor

Can you make sure the pull request description is thorough since that will end up being the CL description internally?

@Dominator008
Copy link
Contributor Author

I assume "pull request description" is just the first comment?

@MatrixFrog
Copy link
Contributor

Not sure but I'm seeing this now, which is much better :)

Convert function declaration to variable declaration so that the IIFE can still be applied.

Unmark calls to captured functions like $jscomp$loop.x() as FREE_CALL.

Fixes #1124

@Dominator008
Copy link
Contributor Author

Sounds good :)

@Dominator008
Copy link
Contributor Author

Should we merge this?

" ($jscomp$loop$0.x)();",
" };",
" }($jscomp$loop$0);",
" $jscomp$loop$0 = {x:$jscomp$loop$0.x};",
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems quite strange but I guess that's not really related to this CL.

MatrixFrog added a commit that referenced this pull request Sep 11, 2015
Fix handling of function declarations in loops with let/consts
@MatrixFrog MatrixFrog merged commit 6ecf097 into google:master Sep 11, 2015
@Dominator008 Dominator008 deleted the issue1124 branch September 12, 2015 00:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants