Skip to content

Fix crash inferring constrained variadic tuples with optional elements - #3943

Closed
Mateusz Burzyński (Andarist) wants to merge 4 commits into
microsoft:mainfrom
Andarist:fix/crash-tuple-implied-constraint-inference
Closed

Fix crash inferring constrained variadic tuples with optional elements#3943
Mateusz Burzyński (Andarist) wants to merge 4 commits into
microsoft:mainfrom
Andarist:fix/crash-tuple-implied-constraint-inference

Conversation

@Andarist

Copy link
Copy Markdown
Contributor

followup to #2928
ports microsoft/TypeScript#63014

Copilot AI review requested due to automatic review settings May 18, 2026 06:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR ports microsoft/TypeScript#63014 as a follow-up to #2928. It adds arity guards in inferFromObjectTypes so that when inferring a [...T, ...rest] or [...rest, ...T] shaped target (where T is constrained by a fixed-size tuple) against a source tuple, inference is only performed if the source actually has enough fixed elements to support the implied arity. This prevents incorrect/spurious inferences (and crashes) when the source's fixed-element count is smaller than the constraint's fixed length—particularly common with optional elements in the constraint.

Changes:

  • Guard the [...T, ...rest] middle case with startLength+impliedArity <= source.fixedLength.
  • Guard the [...rest, ...T] middle case with endLength+impliedArity <= getEndElementCount(source, ElementFlagsFixed) and move the trailing-slice inference inside the guard.
  • Add a new compiler test (inferTypesWithFixedTupleExtendsAtVariadicPosition2.ts) plus its .types, .symbols, and .errors.txt baselines.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/checker/inference.go Adds arity guards in the two variadic+constrained-tuple inference branches
testdata/tests/cases/compiler/inferTypesWithFixedTupleExtendsAtVariadicPosition2.ts New test covering many infer B extends [..] variadic tuple shapes incl. optional elements
testdata/baselines/reference/compiler/inferTypesWithFixedTupleExtendsAtVariadicPosition2.types Type baseline for the new test
testdata/baselines/reference/compiler/inferTypesWithFixedTupleExtendsAtVariadicPosition2.symbols Symbol baseline for the new test
testdata/baselines/reference/compiler/inferTypesWithFixedTupleExtendsAtVariadicPosition2.errors.txt Errors baseline for the new test

@@ -712,9 +712,11 @@ func (c *Checker) inferFromObjectTypes(n *InferenceState, source *Type, target *
constraint := c.getBaseConstraintOfType(info.typeParameter)
if constraint != nil && isTupleType(constraint) && constraint.TargetTupleType().combinedFlags&ElementFlagsVariable == 0 {
impliedArity := constraint.TargetTupleType().fixedLength

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixedLength still includes optional elements. In those cases, the checker could assume it needed more fixed source elements even when fewer were guaranteed, and that led to invalid slice math and crashing

@jakebailey

Copy link
Copy Markdown
Member

We have a couple of PRs touching this same code, e.g. #3917, #2928; I feel a little unclear what combo is correct in all of these

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) added the No linked issue This PR doesn't say what bug it fixes label Jun 8, 2026
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) added the Unmigrated PR This PR was open at the time of the repo move back to TypeScript label Aug 17, 2026
@RyanCavanaugh

Copy link
Copy Markdown
Member

Thank you for contributing to the TypeScript native port!

Development has moved from this repository back to the main microsoft/TypeScript repository. GitHub does not have PR transfer functionality, so we're closing this PR here.

If this change is still relevant, please reopen it as a new pull request in microsoft/TypeScript. We'd appreciate your help carrying the contribution over, and we apologize for the extra work.

See microsoft/typescript-go#4918 for more information about the move.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No linked issue This PR doesn't say what bug it fixes Unmigrated PR This PR was open at the time of the repo move back to TypeScript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants