Skip to content

Fast path for negative case when relating to unions of primtives #53192

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
merged 2 commits into from
Mar 17, 2023
Merged

Fast path for negative case when relating to unions of primtives #53192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
922d47e
Fast path for negative case when relating to unions of primtives
ahejlsberg Mar 10, 2023
059a1f0
Fix comment
ahejlsberg Mar 10, 2023
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
25 changes: 18 additions & 7 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16548,7 +16548,7 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
for (const u of unionTypes) {
if (!containsType(u.types, type)) {
const primitive = type.flags & TypeFlags.StringLiteral ? stringType :
type.flags & TypeFlags.NumberLiteral ? numberType :
type.flags & (TypeFlags.Enum | TypeFlags.NumberLiteral) ? numberType :
type.flags & TypeFlags.BigIntLiteral ? bigintType :
type.flags & TypeFlags.UniqueESSymbol ? esSymbolType :
undefined;
Expand Down Expand Up @@ -16584,10 +16584,6 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return false;
}

function eachIsUnionContaining(types: Type[], flag: TypeFlags) {
return every(types, t => !!(t.flags & TypeFlags.Union) && some((t as UnionType).types, tt => !!(tt.flags & flag)));
}

function removeFromEach(types: Type[], flag: TypeFlags) {
for (let i = 0; i < types.length; i++) {
types[i] = filterType(types[i], t => !(t.flags & flag));
Expand Down Expand Up @@ -16719,12 +16715,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
// reduced we'll never reduce again, so this occurs at most once.
result = getIntersectionType(typeSet, aliasSymbol, aliasTypeArguments);
}
else if (eachIsUnionContaining(typeSet, TypeFlags.Undefined)) {
else if (every(typeSet, t => !!(t.flags & TypeFlags.Union && (t as UnionType).types[0].flags & TypeFlags.Undefined))) {
const containedUndefinedType = some(typeSet, containsMissingType) ? missingType : undefinedType;
removeFromEach(typeSet, TypeFlags.Undefined);
result = getUnionType([getIntersectionType(typeSet), containedUndefinedType], UnionReduction.Literal, aliasSymbol, aliasTypeArguments);
}
else if (eachIsUnionContaining(typeSet, TypeFlags.Null)) {
else if (every(typeSet, t => !!(t.flags & TypeFlags.Union && ((t as UnionType).types[0].flags & TypeFlags.Null || (t as UnionType).types[1].flags & TypeFlags.Null)))) {
Copy link

Choose a reason for hiding this comment

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

(lines:-5+0)

Why does this only need to check?

  • types[0] for TypeFlags.Undefined
  • types[0] | types[1] for TypeFlags.Null

Is there some rule/convention that null is first in UnionType#types if it's part of the union, unless undefined is in the union (which is really always first if it is part of the union)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, union types are sorted like this internally - so if a union contains undefined it has to be the first element etc.

removeFromEach(typeSet, TypeFlags.Null);
result = getUnionType([getIntersectionType(typeSet), nullType], UnionReduction.Literal, aliasSymbol, aliasTypeArguments);
}
Expand Down Expand Up @@ -20837,6 +20833,21 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (containsType(targetTypes, source)) {
return Ternary.True;
}
if (getObjectFlags(target) & ObjectFlags.PrimitiveUnion && !(source.flags & TypeFlags.EnumLiteral) && (
source.flags & (TypeFlags.StringLiteral | TypeFlags.BooleanLiteral | TypeFlags.BigIntLiteral) ||
Copy link
Member

Choose a reason for hiding this comment

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

This can be expanded to cover unique symbol literals, too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but we'd then have to exclude them from the alternateForm check below because unique symbols don't have fresh forms. The extra complexity just doesn't seem worth it, I honestly can't imagine union types with multiple thousand unique symbols types anyways.

(relation === subtypeRelation || relation === strictSubtypeRelation) && source.flags & TypeFlags.NumberLiteral)) {
// When relating a literal type to a union of primitive types, we know the relation is false unless
// the union contains the base primitive type or the literal type in one of its fresh/regular forms.
// We exclude numeric literals for non-subtype relations because numeric literals are assignable to
// numeric enum literals with the same value. Similarly, we exclude enum literal types because
// identically named enum types are related (see isEmumTypeRelatedTo).
const alternateForm = source === (source as StringLiteralType).regularType ? (source as StringLiteralType).freshType : (source as StringLiteralType).regularType;
const primitive = source.flags & TypeFlags.StringLiteral ? stringType :
source.flags & TypeFlags.NumberLiteral ? numberType :
source.flags & TypeFlags.BigIntLiteral ? bigintType :
undefined;
return primitive && containsType(targetTypes, primitive) || alternateForm && containsType(targetTypes, alternateForm) ? Ternary.True : Ternary.False;
}
const match = getMatchingUnionConstituentForType(target as UnionType, source);
if (match) {
const related = isRelatedTo(source, match, RecursionFlags.Target, /*reportErrors*/ false);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6130,7 +6130,7 @@ export const enum TypeFlags {
/** @internal */
IncludesInstantiable = Substitution,
/** @internal */
NotPrimitiveUnion = Any | Unknown | Enum | Void | Never | Object | Intersection | IncludesInstantiable,
NotPrimitiveUnion = Any | Unknown | Void | Never | Object | Intersection | IncludesInstantiable,
}

export type DestructuringPattern = BindingPattern | ObjectLiteralExpression | ArrayLiteralExpression;
Expand Down