This repository was archived by the owner on Sep 1, 2026. It is now read-only.
Repository navigation
Optimize narrowTypeByEquality and narrowTypeBySwitchOnDiscriminant - #4781
Merged
Merged
Conversation
Member
Author
|
TypeScript Bot (@typescript-bot) test it |
Contributor
There was a problem hiding this comment.
Pull request overview
Optimizes primitive-union narrowing during control-flow analysis.
Changes:
- Adds fast paths for equality and switch narrowing.
- Optimizes type removal and centralizes union membership checks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
internal/checker/flow.go |
Adds narrowing fast paths. |
internal/checker/checker.go |
Optimizes union helpers and type removal. |
Comment on lines
+601
to
+603
| filteredType := c.removeType(t, c.getRegularTypeOfLiteralType(valueType)) | ||
| if filteredType != t { | ||
| return filteredType |
Comment on lines
+589
to
+594
| if !doubleEquals && t.objectFlags&ObjectFlagsPrimitiveUnion != 0 && valueType.flags&TypeFlagsPrimitive != 0 { | ||
| regularType := c.getRegularTypeOfLiteralType(valueType) | ||
| if c.unionContainsType(t, regularType) { | ||
| return regularType | ||
| } | ||
| } |
Comment on lines
+1123
to
+1128
| if t.objectFlags&ObjectFlagsPrimitiveUnion != 0 && discriminantType.flags&TypeFlagsPrimitive != 0 { | ||
| regularType := c.getRegularTypeOfLiteralType(discriminantType) | ||
| if c.unionContainsType(t, regularType) { | ||
| caseType = regularType | ||
| } | ||
| } |
|
Anders Hejlsberg (@ahejlsberg) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Anders Hejlsberg (@ahejlsberg) Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Anders Hejlsberg (@ahejlsberg) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Anders Hejlsberg (@ahejlsberg) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Member
Author
|
TypeScript Bot (@typescript-bot) perf test this faster |
|
Anders Hejlsberg (@ahejlsberg) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
The fast paths introduce incorrect narrowing for computed enums and non-primitive union constituents.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (1)
internal/checker/flow.go:1129
- This shortcut never applies to grouped case labels. Their
discriminantTypeis a union, whose flags do not includeTypeFlagsPrimitive, andunionContainsTypewould in any event look for that whole union as one direct constituent. Large switches with many empty fallthrough labels therefore still use the O(union constituents × grouped cases) relational fallback. Check membership for each regular discriminant constituent instead, and cover a grouped-case switch in the performance regression.
if t.objectFlags&ObjectFlagsPrimitiveUnion != 0 && discriminantType.flags&TypeFlagsPrimitive != 0 && c.isUniformUnionType(t) {
regularType := c.getRegularTypeOfLiteralType(discriminantType)
if c.unionContainsType(t, regularType, false /*matchSymbol*/) {
caseType = regularType
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| return c.replacePrimitivesWithLiterals(filteredType, valueType) | ||
| } | ||
| if isUnitType(valueType) { | ||
| if c.isUniformUnionType(t) { |
Member
Author
There was a problem hiding this comment.
Yup, an oversight.
| var enumSymbol *ast.Symbol | ||
| var hasStringOrNumberLiteral bool | ||
| for _, t := range types { | ||
| if t.flags&TypeFlagsEnumLiteral != 0 { |
Member
Author
There was a problem hiding this comment.
We all hate that rule, but yeah.
Contributor
There was a problem hiding this comment.
🟡 Not ready to approve
The primitive-union guard needs regression coverage for intersection constituents.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (2)
internal/checker/checker.go:27797
- Add the missing “if” to make this sentence grammatical.
internal/checker/checker.go:27801 - The
PrimitiveUnionrestriction added after the reported intersection regression is not exercised by this test suite; all current cases are primitive unions. Please add a compiler case for"a" | ("a" & { brand: true })and verify that the false branch ofx === "a"remainsnever, so this correctness guard cannot regress unnoticed.
- Files reviewed: 8/8 changed files
- Comments generated: 0 new
- Review effort level: Medium
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Member
Author
I don't think so. Plenty of other stuff already depends on primitive-union not including intersection constituents. |
Jake Bailey (jakebailey)
approved these changes
Jul 30, 2026
Jake Bailey (jakebailey)
left a comment
Jake Bailey (jakebailey)
left a comment
Member
There was a problem hiding this comment.
Hate to use up two more flags but I also like to go fast
Jake Bailey (jakebailey)
pushed a commit
to jakebailey/TypeScript
that referenced
this pull request
Aug 13, 2026
This was referenced Oct 13, 2026
This was referenced Oct 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Inspired by the performance gains in #4711, this PR further optimizes union type narrowing in control flow analysis.