- Notifications
You must be signed in to change notification settings - Fork13.1k
Limit tuple size resulting from spread#42448
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
weswigham left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yeah, this looks pretty reasonable. It's another limit someone may eventually come to us and say "but I need it higher for so and so reasons", but we can deal with that as it happens.
ahejlsberg left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I like having a limiter here, but I'm thinking 10,000 is a more reasonable threshold. Also, I think it is better to have only a single diagnostic, "Expression produces a tuple type...", instead of two. We only have a single diagnostic for the other limiters and it makes it easer to search for solutions on stack overflow, etc.
DanielRosenwasser commentedJan 22, 2021
Looks good - 4.2-bound? |
andrewbranch commentedJan 22, 2021
The first message I wrote was “Spread produces a tuple type...” but I wasn’t sure how I felt about it since the error could appear far from the actual spread via instantiation. I guess I can delete the “Type” variant, it just felt wrong to call a type an expression. |
DanielRosenwasser commentedJan 22, 2021
I think seeing "spread" on a type alias would actually be confusing. I would prefer the 2 variants. |
andrewbranch commentedJan 29, 2021
|
Uh oh!
There was an error while loading.Please reload this page.
Adds a (currently very conservative, based on design meeting feedback) limit on how large of a tuple type we will create through spread normalization.
On each recursion, checking
T['length']kicks offresolveObjectTypeMembers, which instantiates base types. Recall that anN-tuple is modeled something likeSo in order to instantiate the base type, we have to pass along an array type mapper mapping T0...TN to the “type arguments,” or the element types of this particular tuple. So even though, in this case, every element type is
any, the source side of the mapper is anN+1-length array filled withN unique type parameter identities (plus thethistype parameter). When we instantiate each union constituentT0 | T1 | ... | TN, we have to search through thatN-length array of sources, even though in this case we’ll always findanyin the target side.So you can imagine by the time we get to the 15th recursion or so, this is starting to take a while, and we’re creating quite a lot of type parameter identities. Pretty much all the time was being spent in this spiral of trying to instantiate the Array base type.
Fixes#41771