Skip to content

Improve recursion identity for direct type instantiations #3445

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.

Sign up for GitHub

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

Merged
ahejlsberg merged 7 commits into from
Apr 22, 2026
Merged

Improve recursion identity for direct type instantiations #3445

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
08f1491
Improve recursion identity for direct type instantiations
ahejlsberg Apr 20, 2026
020d921
Add regression test
ahejlsberg Apr 20, 2026
e4bcd52
Update API package
ahejlsberg Apr 20, 2026
586394c
Fix the recursion identity for tuples from type nodes too (#3503)
ahejlsberg Apr 22, 2026
bcfcae8
Merge branch 'main' into fix-3426
ahejlsberg Apr 22, 2026
668feed
Merge branch 'fix-3426' of https://github.com/microsoft/typescript-go…
ahejlsberg Apr 22, 2026
9f79d71
Merge branch 'main' into fix-3426
ahejlsberg Apr 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions internal/checker/checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22739,7 +22739,7 @@ func (c *Checker) getTypeFromClassOrInterfaceReference(node *ast.Node, symbol *a
// of the class or interface.
localTypeArguments := c.fillMissingTypeArguments(c.getTypeArgumentsFromNode(node), typeParameters, minTypeArgumentCount, isJs)
typeArguments := append(d.OuterTypeParameters(), localTypeArguments...)
return c.createTypeReference(t, typeArguments)
return c.createTypeReferenceEx(t, typeArguments, ObjectFlagsFromTypeNode)
}
if c.checkNoTypeArguments(node, symbol) {
return t
Expand Down Expand Up @@ -23660,7 +23660,11 @@ func (c *Checker) getTypeFromArrayOrTupleTypeNode(node *ast.Node) *Type {
} else {
elementTypes = core.Map(node.Elements(), c.getTypeFromTypeNode)
}
links.resolvedType = c.createNormalizedTypeReference(target, elementTypes)
if target.objectFlags&ObjectFlagsTuple != 0 {
links.resolvedType = c.createNormalizedTupleType(target, elementTypes)
Copy link
Copy Markdown
Contributor

@Andarist Andarist Apr 21, 2026

Choose a reason for hiding this comment

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

Shouldn't this also accept and propagate ObjectFlagsFromTypeNode? FWIW, while testing this PR locally I ended up implementing this: https://github.com/microsoft/typescript-go/compare/fix-3426...Andarist:fix/deeply-nested-tuples-from-type-nodes?expand=1

Copy link
Copy Markdown
Member Author

@ahejlsberg ahejlsberg Apr 22, 2026

Choose a reason for hiding this comment

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

@Andarist Thanks!

} else {
links.resolvedType = c.createTypeReferenceEx(target, elementTypes, ObjectFlagsFromTypeNode)
}
}
}
return links.resolvedType
Expand Down Expand Up @@ -24623,13 +24627,16 @@ func (c *Checker) tryCreateTypeReference(target *Type, typeArguments []*Type) *T
}

func (c *Checker) createTypeReference(target *Type, typeArguments []*Type) *Type {
return c.createTypeReferenceEx(target, typeArguments, ObjectFlagsNone)
}

func (c *Checker) createTypeReferenceEx(target *Type, typeArguments []*Type, objectFlags ObjectFlags) *Type {
id := getTypeListKey(typeArguments)
intf := target.AsInterfaceType()
if t, ok := intf.instantiations[id]; ok {
return t
}
t := c.newObjectType(ObjectFlagsReference, target.symbol)
t.objectFlags |= c.getPropagatingFlagsOfTypes(typeArguments, TypeFlagsNone)
t := c.newObjectType(ObjectFlagsReference|objectFlags|c.getPropagatingFlagsOfTypes(typeArguments, TypeFlagsNone), target.symbol)
Comment on lines 24640 to +24645
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

createTypeReferenceEx caches instantiations only by typeArguments (getTypeListKey). That ignores the objectFlags parameter, so a previously-cached instantiation without ObjectFlagsFromTypeNode can be returned to callers that request it (and vice versa). This makes the new recursion-identity behavior depend on call order and can cause both missed recursion detection and missed errors. Consider including the relevant objectFlags bits (at least ObjectFlagsFromTypeNode) in the cache key and/or maintaining a separate instantiation cache for FromTypeNode references so the flag is applied deterministically only where intended.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

@ahejlsberg ahejlsberg Apr 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This is a good observation and indeed something that was considered. The issue with tracking the flag in the cache key is that it creates distinct object identities for effectively identical types, which we then have to reconcile in multiple places elsewhere, for example in order to avoid strange types like number[] | number[] (two types that differ only by the flag). The flag is only used by a heuristic that ultimately also depends on type IDs (which similarly depend on declaration order), so it is a reasonable compromise to just record it in the first type instantiation.

d := t.AsTypeReference()
d.target = target
d.resolvedTypeArguments = typeArguments
Expand Down
6 changes: 4 additions & 2 deletions internal/checker/relater.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,9 +839,11 @@ func getRecursionIdentity(t *Type) RecursionId {
// unique AST node.
return asRecursionId(t.AsTypeReference().node)
}
if t.symbol != nil && !(t.objectFlags&ObjectFlagsAnonymous != 0 && t.symbol.Flags&ast.SymbolFlagsClass != 0) {
if t.symbol != nil && !(t.objectFlags&ObjectFlagsAnonymous != 0 && t.symbol.Flags&ast.SymbolFlagsClass != 0) && t.objectFlags&ObjectFlagsFromTypeNode == 0 {
// We track object types that have a symbol by that symbol (representing the origin of the type), but
// exclude the static side of a class since it shares its symbol with the instance side.
// exclude the static sides of classes (since they share their symbols with the instance sides) and type
// references that originate in resolution of AST type nodes (since such type nodes cannot be the source
// of generative recursion without first being instantiated).
Comment on lines +842 to +846
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

This change adjusts recursion identity handling to fix a checker soundness gap, but the PR doesn't add a regression test for #3426 (nested arrays from distinct namespaces should now produce TS2322). Please add a minimal compiler test case under testdata/tests/cases/compiler/ that fails before this change and passes after, so we don’t regress this behavior again.

Copilot generated this review using guidance from repository custom instructions.
Copy link
Copy Markdown
Member Author

@ahejlsberg ahejlsberg Apr 20, 2026

Choose a reason for hiding this comment

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

Now fixed.

return asRecursionId(t.symbol)
}
if isTupleType(t) {
Expand Down
1 change: 1 addition & 0 deletions internal/checker/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ const (
// Flags that require TypeFlags.Object and ObjectFlags.Reference
ObjectFlagsIdenticalBaseTypeCalculated = 1 << 27 // has had `getSingleBaseForNonAugmentingSubtype` invoked on it already
ObjectFlagsIdenticalBaseTypeExists = 1 << 28 // has a defined cachedEquivalentBaseType member
ObjectFlagsFromTypeNode = 1 << 29 // Originates in resolution of AST type node
// Flags that require TypeFlags.UnionOrIntersection or TypeFlags.Substitution
ObjectFlagsIsGenericTypeComputed = 1 << 22 // IsGenericObjectType flag has been computed
ObjectFlagsIsGenericObjectType = 1 << 23 // Union or intersection contains generic object type
Expand Down
Loading