Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
andrewbranch merged 4 commits intomicrosoft:masterfromandrewbranch:bug/41771
Feb 2, 2021

Conversation

@andrewbranch
Copy link
Member

@andrewbranchandrewbranch commentedJan 21, 2021
edited
Loading

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, checkingT['length'] kicks offresolveObjectTypeMembers, which instantiates base types. Recall that anN-tuple is modeled something like

interfaceTuple<T0,T1, ...,TN>extendsArray<T0|T1| ...|TN>{0:T0;1:T1;// ...}

So 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 isany, the source side of the mapper is anN+1-length array filled withN unique type parameter identities (plus thethis type 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 findany in 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

Copy link
Member

@weswighamweswigham left a 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.

Copy link
Member

@ahejlsbergahejlsberg left a 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
Copy link
Member

Looks good - 4.2-bound?

@andrewbranch
Copy link
MemberAuthor

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 reacted with thumbs up emoji

@DanielRosenwasser
Copy link
Member

I think seeing "spread" on a type alias would actually be confusing. I would prefer the 2 variants.

DanielRosenwasser reacted with thumbs up emoji

@DanielRosenwasserDanielRosenwasser added the Breaking ChangeWould introduce errors in existing code labelJan 25, 2021
@andrewbranch
Copy link
MemberAuthor

  • I agree that 10k would be fine as a limit. Will update.
  • @DanielRosenwasser if you’re concerned about this going into 4.2 as a breaking change at this point, I think it can wait until 4.3. But I think as breaking changes go, it’s not super likely to be observed, especially after I update the limit to 10k.
  • Any consensus on the error message(s)?

@andrewbranchandrewbranch merged commit71de94a intomicrosoft:masterFeb 2, 2021
@andrewbranchandrewbranch deleted the bug/41771 branchFebruary 2, 2021 22:02
DetachHead added a commit to DetachHead/ts-helpers that referenced this pull requestApr 6, 2021
DetachHead added a commit to DetachHead/ts-helpers that referenced this pull requestApr 6, 2021
DetachHead added a commit to DetachHead/ts-helpers that referenced this pull requestJul 15, 2022
@microsoftmicrosoft locked asresolvedand limited conversation to collaboratorsOct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@weswighamweswighamweswigham approved these changes

@ahejlsbergahejlsbergahejlsberg approved these changes

@RyanCavanaughRyanCavanaughAwaiting requested review from RyanCavanaugh

Assignees

@andrewbranchandrewbranch

Labels

Author: TeamBreaking ChangeWould introduce errors in existing codeFor Milestone BugPRs that fix a bug with a specific milestone

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Type checker hangs on recursive type with arrays

5 participants

@andrewbranch@DanielRosenwasser@weswigham@ahejlsberg@typescript-bot

[8]ページ先頭

©2009-2025 Movatter.jp