Skip to content

Always create a temporary for iterated expressions in a for-of loop#5477

Merged
DanielRosenwasser merged 6 commits into
masterfrom
mutatedArrayInForOf
Oct 31, 2015
Merged

Always create a temporary for iterated expressions in a for-of loop#5477
DanielRosenwasser merged 6 commits into
masterfrom
mutatedArrayInForOf

Conversation

@DanielRosenwasser
Copy link
Copy Markdown
Member

Fixes #5475. The only special handling we use now is that we create a name based on the original identifier if we had an identifier.

@mhegazy
Copy link
Copy Markdown
Contributor

mhegazy commented Oct 30, 2015

@rbuckton can you take a look.

Comment thread src/compiler/emitter.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One possibility might be to check if the rhs is declared as const, and then we can emit it inline.

@rbuckton
Copy link
Copy Markdown
Contributor

Other than the const comment, 👍

Comment thread src/compiler/emitter.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you add tests for this case?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We already have tests for that case

@yuit
Copy link
Copy Markdown
Contributor

yuit commented Oct 30, 2015

otherwise lgtm

DanielRosenwasser added a commit that referenced this pull request Oct 31, 2015
Always create a temporary for iterated expressions in a for-of loop
@DanielRosenwasser DanielRosenwasser merged commit 5cbcafa into master Oct 31, 2015
@DanielRosenwasser DanielRosenwasser deleted the mutatedArrayInForOf branch October 31, 2015 00:42
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants