Skip to content
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

Only allow expanding generics to form intersection constraints a limited number of times #47897

Closed
wants to merge 1 commit into from

Conversation

weswigham
Copy link
Member

Fixes #46900

This prevents us from (slowly) infinitely expanding our constraints as we're comparing intersections of generics to unions.

return result;
}
}
doDistributiveCheck = true;
Copy link
Member Author

@weswigham weswigham Feb 14, 2022

Choose a reason for hiding this comment

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

This just moves checking of the distributive constraint to after checking the normal conditional constraint, which is needed to remove an instantiation depth error from the example repo in #46900 after the intersection constraint is no longer expanded forever. (And is singlehandedly what causes the reshuffling of all the error messages in this PR)

@weswigham
Copy link
Member Author

I don't yet have a truly minimal repro from #46900 (just the submission that uses ~5 packages of types), because the types involved are quite complicated, however I'm going to endeavor to create one now that I have a fix that I know works ready.

@ahejlsberg
Copy link
Member

I'm not sure we need this PR as the issue appears to already be fixed by #47738. That PR moves logic from the isRelatedTo function (the uncached side of relationship checking) to the structuredTypeRelatedTo function (the cached side) where it should have been in the first place. Apart from the caching benefits this makes the logic subject to recursion depth checking which cuts off the infinite intersection constraint generation in #46900.

I think we should just merge #47738 for 4.6. I took Daniel's comment here to mean hold until after 4.6, but I do think the changes are safe.

@weswigham
Copy link
Member Author

We're already post-4.6 in main, so merge away~

@weswigham
Copy link
Member Author

Superseded by #47738

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tsc 4.5 hangs with unist-util-visit package (works fine with 4.4.4)
3 participants