Skip to content

Adds CommaList to avoid large deeply nested comma expressions#15791

Merged
rbuckton merged 2 commits into
masterfrom
fix13935
May 12, 2017
Merged

Adds CommaList to avoid large deeply nested comma expressions#15791
rbuckton merged 2 commits into
masterfrom
fix13935

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

When transforming a ObjectLiteralExpression with a large number of computed property names, we currently create a very large tree of binary expressions that is deeply nested along the right hand side. In some extreme cases, when we attempt to print the tree we end up exhausting all of the available stack space.

This change introduces a synthetic CommaList node which flattens a large number of comma expressions into a single list to reduce the overall tree depth.

Fixes #13935

Copy link
Copy Markdown
Contributor

@mhegazy mhegazy left a comment

Choose a reason for hiding this comment

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

Can you add a test for the stack over flow to make sure we do not regress this.

Comment thread src/compiler/factory.ts
return node;
}

export function createCommaList(elements: Expression[]) {
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.

rename to createCommaListExpression to align with the other create or update functions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, most other create/update functions for expressions elide the word Expression.

Comment thread src/compiler/factory.ts
return node;
}

export function updateCommaList(node: CommaListExpression, elements: Expression[]) {
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.

same as above

@rbuckton rbuckton merged commit 50e2912 into master May 12, 2017
@rbuckton rbuckton deleted the fix13935 branch May 12, 2017 23:49
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 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.

4 participants