Skip to content

Fix stack overflow in circular assignment declaration#34543

Merged
sandersn merged 4 commits intomasterfrom
fix-stack-overflow-in-circular-assignment-decl
Oct 18, 2019
Merged

Fix stack overflow in circular assignment declaration#34543
sandersn merged 4 commits intomasterfrom
fix-stack-overflow-in-circular-assignment-decl

Conversation

@sandersn
Copy link
Copy Markdown
Member

The declaration also needs to have multiple assignments so that it has a ValueModule flag.

The fix is to remove the possibly-circular call to getTypeofFuncClassEnumModule in the circularity-detection code path.

Fixes #33006

@sandersn
Copy link
Copy Markdown
Member Author

This was introduced in #29335, but that test still passes with the fallback calls to getTypeOfFuncClassEnumModule removed.

@sandersn
Copy link
Copy Markdown
Member Author

Oops, forgot that some tests were skipped. The tests do still fail.

@sandersn
Copy link
Copy Markdown
Member Author

After some discussion, I don't think that #29335 is the correct fix; umdNamespaceMergedWithGlobalAugmentationIsNotCircular should still have an error, but a duplicate declaration error instead of a circular reference error.

@sandersn
Copy link
Copy Markdown
Member Author

Since 3.7's beta ends today, I decided on a compromise fix, which is to avoid the fallback for symbols with flag Assignment. Those symbols will have already been checked using the JS code path, so aren't candidates for the special kind of merges handled in #29335.

Later I'll see whether I can catch the merge from #29335 in mergeSymbol during checker startup and issue an error there.

@sandersn sandersn merged commit 82f927f into master Oct 18, 2019
@sandersn sandersn deleted the fix-stack-overflow-in-circular-assignment-decl branch October 18, 2019 16:06
@sandersn
Copy link
Copy Markdown
Member Author

@weswigham the React merge works because we allow aliases to merge with anything. If I resolveSymbol first and forbid merges between the source and the resolved alias, it breaks four tests and the breaks all look right to me. Opinions?

@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
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.

TSC Crash: Javascript Object assignment

2 participants