Skip to content

Fix stack overflow in circular assignment declaration - #34543

Merged
Nathan Shively-Sanders (sandersn) merged 4 commits into
masterfrom
fix-stack-overflow-in-circular-assignment-decl
Oct 18, 2019
Merged

Nathan Shively-Sanders (sandersn) merged 4 commits into
masterfrom
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

Copy link
Copy Markdown
Member Author

Wesley Wigham (@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 (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