Skip to content

Optimize intersections of unions of unit types #24137

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 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 4 commits into from
May 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
37 changes: 33 additions & 4 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8319,7 +8319,7 @@ namespace ts {
includes & TypeFlags.Undefined ? includes & TypeFlags.NonWideningType ? undefinedType : undefinedWideningType :
neverType;
}
return getUnionTypeFromSortedList(typeSet, aliasSymbol, aliasTypeArguments);
return getUnionTypeFromSortedList(typeSet, includes & TypeFlags.NotUnit ? 0 : TypeFlags.UnionOfUnitTypes, aliasSymbol, aliasTypeArguments);
}

function getUnionTypePredicate(signatures: ReadonlyArray<Signature>): TypePredicate {
Expand Down Expand Up @@ -8359,7 +8359,7 @@ namespace ts {
}

// This function assumes the constituent type list is sorted and deduplicated.
function getUnionTypeFromSortedList(types: Type[], aliasSymbol?: Symbol, aliasTypeArguments?: Type[]): Type {
function getUnionTypeFromSortedList(types: Type[], unionOfUnitTypes: TypeFlags, aliasSymbol?: Symbol, aliasTypeArguments?: Type[]): Type {
Copy link
Member

Choose a reason for hiding this comment

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

Possible optimization: unionOfUnitTypes should just be additionalFlags, and we should calculate getPropagatingFlagsOfTypes piece-wise while doing includes, and just pass them in (rather then making another pass over the type here to calculate them).... although I know right now to save flag space we re-purpose the propagating flags during include calculation, which could make that.... hard.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but I don't really want to mess with that now.

if (types.length === 0) {
return neverType;
}
Expand All @@ -8370,7 +8370,7 @@ namespace ts {
let type = unionTypes.get(id);
if (!type) {
const propagatedFlags = getPropagatingFlagsOfTypes(types, /*excludeKinds*/ TypeFlags.Nullable);
type = <UnionType>createType(TypeFlags.Union | propagatedFlags);
type = <UnionType>createType(TypeFlags.Union | propagatedFlags | unionOfUnitTypes);
unionTypes.set(id, type);
type.types = types;
/*
Expand Down Expand Up @@ -8441,6 +8441,29 @@ namespace ts {
}
}

// When intersecting unions of unit types we can simply intersect based on type identity.
// Here we remove all unions of unit types from the given list and replace them with a
// a single union containing an intersection of the unit types.
function intersectUnionsOfUnitTypes(types: Type[]) {
const unionIndex = findIndex(types, t => (t.flags & TypeFlags.UnionOfUnitTypes) !== 0);
const unionType = <UnionType>types[unionIndex];
let intersection = unionType.types;
let i = types.length - 1;
while (i > unionIndex) {
const t = types[i];
if (t.flags & TypeFlags.UnionOfUnitTypes) {
intersection = filter(intersection, u => containsType((<UnionType>t).types, u));
orderedRemoveItemAt(types, i);
}
i--;
}
if (intersection === unionType.types) {
return false;
}
types[unionIndex] = getUnionTypeFromSortedList(intersection, unionType.flags & TypeFlags.UnionOfUnitTypes);
return true;
}

// We normalize combinations of intersection and union types based on the distributive property of the '&'
// operator. Specifically, because X & (A | B) is equivalent to X & A | X & B, we can transform intersection
// types with union type constituents into equivalent union types with intersection type constituents and
Expand Down Expand Up @@ -8475,6 +8498,12 @@ namespace ts {
return typeSet[0];
}
if (includes & TypeFlags.Union) {
if (includes & TypeFlags.UnionOfUnitTypes && intersectUnionsOfUnitTypes(typeSet)) {
// When the intersection creates a reduced set (which might mean that *all* union types have
// disappeared), we restart the operation to get a new set of combined flags. Once we have
// reduced we'll never reduce again, so this occurs at most once.
return getIntersectionType(typeSet, aliasSymbol, aliasTypeArguments);
}
// We are attempting to construct a type of the form X & (A | B) & Y. Transform this into a type of
// the form X & A & Y | X & B & Y and recursively reduce until no union type constituents remain.
const unionIndex = findIndex(typeSet, t => (t.flags & TypeFlags.Union) !== 0);
Expand Down Expand Up @@ -13234,7 +13263,7 @@ namespace ts {
if (type.flags & TypeFlags.Union) {
const types = (<UnionType>type).types;
const filtered = filter(types, f);
return filtered === types ? type : getUnionTypeFromSortedList(filtered);
return filtered === types ? type : getUnionTypeFromSortedList(filtered, type.flags & TypeFlags.UnionOfUnitTypes);
}
return f(type) ? type : neverType;
}
Expand Down
4 changes: 4 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3674,6 +3674,8 @@ namespace ts {
ContainsAnyFunctionType = 1 << 26, // Type is or contains the anyFunctionType
NonPrimitive = 1 << 27, // intrinsic object type
/* @internal */
UnionOfUnitTypes = 1 << 28, // Type is union of unit types
Copy link
Member

@weswigham weswigham May 15, 2018

Choose a reason for hiding this comment

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

With this, TypeFlags is full. We're going to need a new field for flags like this (very) soon (as I don't see any that are obviously ObjectFlags). (This and the propagating flags are effectively a union's equivalent of ObjectFlags)

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, TypeFlags being full is nothing new. Seems like it has been for the last many years. I actually think it is fine to put all of the flags to good use and then work out alternate solutions as the need arises. This particular flag could easily be moved to a property local to union types, but there's no reason to do so now.

/* @internal */
GenericMappedType = 1 << 29, // Flag used by maybeTypeOfKind

/* @internal */
Expand Down Expand Up @@ -3711,6 +3713,8 @@ namespace ts {
Narrowable = Any | StructuredOrInstantiable | StringLike | NumberLike | BooleanLike | ESSymbol | UniqueESSymbol | NonPrimitive,
NotUnionOrUnit = Any | ESSymbol | Object | NonPrimitive,
/* @internal */
NotUnit = Any | String | Number | Boolean | Enum | ESSymbol | Void | Never | StructuredOrInstantiable,
/* @internal */
RequiresWidening = ContainsWideningType | ContainsObjectLiteral,
/* @internal */
PropagatingFlags = ContainsWideningType | ContainsObjectLiteral | ContainsAnyFunctionType,
Expand Down
35 changes: 35 additions & 0 deletions tests/baselines/reference/intersectionsOfLargeUnions.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
tests/cases/compiler/intersectionsOfLargeUnions.ts(21,15): error TS2536: Type 'T' cannot be used to index type 'HTMLElementTagNameMap'.
tests/cases/compiler/intersectionsOfLargeUnions.ts(21,15): error TS2536: Type 'P' cannot be used to index type 'HTMLElementTagNameMap[T]'.


==== tests/cases/compiler/intersectionsOfLargeUnions.ts (2 errors) ====
// Repro from #23977

export function assertIsElement(node: Node | null): node is Element {
let nodeType = node === null ? null : node.nodeType;
return nodeType === 1;
}

export function assertNodeTagName<
T extends keyof ElementTagNameMap,
U extends ElementTagNameMap[T]>(node: Node | null, tagName: T): node is U {
if (assertIsElement(node)) {
const nodeTagName = node.tagName.toLowerCase();
return nodeTagName === tagName;
}
return false;
}

export function assertNodeProperty<
T extends keyof ElementTagNameMap,
P extends keyof ElementTagNameMap[T],
V extends HTMLElementTagNameMap[T][P]>(node: Node | null, tagName: T, prop: P, value: V) {
~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2536: Type 'T' cannot be used to index type 'HTMLElementTagNameMap'.
~~~~~~~~~~~~~~~~~~~~~~~~~~~
!!! error TS2536: Type 'P' cannot be used to index type 'HTMLElementTagNameMap[T]'.
if (assertNodeTagName(node, tagName)) {
node[prop];
}
}

51 changes: 51 additions & 0 deletions tests/baselines/reference/intersectionsOfLargeUnions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
//// [intersectionsOfLargeUnions.ts]
// Repro from #23977

export function assertIsElement(node: Node | null): node is Element {
let nodeType = node === null ? null : node.nodeType;
return nodeType === 1;
}

export function assertNodeTagName<
T extends keyof ElementTagNameMap,
U extends ElementTagNameMap[T]>(node: Node | null, tagName: T): node is U {
if (assertIsElement(node)) {
const nodeTagName = node.tagName.toLowerCase();
return nodeTagName === tagName;
}
return false;
}

export function assertNodeProperty<
T extends keyof ElementTagNameMap,
P extends keyof ElementTagNameMap[T],
V extends HTMLElementTagNameMap[T][P]>(node: Node | null, tagName: T, prop: P, value: V) {
if (assertNodeTagName(node, tagName)) {
node[prop];
}
}


//// [intersectionsOfLargeUnions.js]
"use strict";
// Repro from #23977
exports.__esModule = true;
function assertIsElement(node) {
var nodeType = node === null ? null : node.nodeType;
return nodeType === 1;
}
exports.assertIsElement = assertIsElement;
function assertNodeTagName(node, tagName) {
if (assertIsElement(node)) {
var nodeTagName = node.tagName.toLowerCase();
return nodeTagName === tagName;
}
return false;
}
exports.assertNodeTagName = assertNodeTagName;
function assertNodeProperty(node, tagName, prop, value) {
if (assertNodeTagName(node, tagName)) {
node[prop];
}
}
exports.assertNodeProperty = assertNodeProperty;
95 changes: 95 additions & 0 deletions tests/baselines/reference/intersectionsOfLargeUnions.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
=== tests/cases/compiler/intersectionsOfLargeUnions.ts ===
// Repro from #23977

export function assertIsElement(node: Node | null): node is Element {
>assertIsElement : Symbol(assertIsElement, Decl(intersectionsOfLargeUnions.ts, 0, 0))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 2, 32))
>Node : Symbol(Node, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 2, 32))
>Element : Symbol(Element, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))

let nodeType = node === null ? null : node.nodeType;
>nodeType : Symbol(nodeType, Decl(intersectionsOfLargeUnions.ts, 3, 7))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 2, 32))
>node.nodeType : Symbol(Node.nodeType, Decl(lib.d.ts, --, --))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 2, 32))
>nodeType : Symbol(Node.nodeType, Decl(lib.d.ts, --, --))

return nodeType === 1;
>nodeType : Symbol(nodeType, Decl(intersectionsOfLargeUnions.ts, 3, 7))
}

export function assertNodeTagName<
>assertNodeTagName : Symbol(assertNodeTagName, Decl(intersectionsOfLargeUnions.ts, 5, 1))

T extends keyof ElementTagNameMap,
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 7, 34))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.d.ts, --, --))

U extends ElementTagNameMap[T]>(node: Node | null, tagName: T): node is U {
>U : Symbol(U, Decl(intersectionsOfLargeUnions.ts, 8, 38))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 7, 34))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 9, 36))
>Node : Symbol(Node, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions.ts, 9, 54))
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 7, 34))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 9, 36))
>U : Symbol(U, Decl(intersectionsOfLargeUnions.ts, 8, 38))

if (assertIsElement(node)) {
>assertIsElement : Symbol(assertIsElement, Decl(intersectionsOfLargeUnions.ts, 0, 0))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 9, 36))

const nodeTagName = node.tagName.toLowerCase();
>nodeTagName : Symbol(nodeTagName, Decl(intersectionsOfLargeUnions.ts, 11, 13))
>node.tagName.toLowerCase : Symbol(String.toLowerCase, Decl(lib.d.ts, --, --))
>node.tagName : Symbol(Element.tagName, Decl(lib.d.ts, --, --))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 9, 36))
>tagName : Symbol(Element.tagName, Decl(lib.d.ts, --, --))
>toLowerCase : Symbol(String.toLowerCase, Decl(lib.d.ts, --, --))

return nodeTagName === tagName;
>nodeTagName : Symbol(nodeTagName, Decl(intersectionsOfLargeUnions.ts, 11, 13))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions.ts, 9, 54))
}
return false;
}

export function assertNodeProperty<
>assertNodeProperty : Symbol(assertNodeProperty, Decl(intersectionsOfLargeUnions.ts, 15, 1))

T extends keyof ElementTagNameMap,
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 17, 35))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.d.ts, --, --))

P extends keyof ElementTagNameMap[T],
>P : Symbol(P, Decl(intersectionsOfLargeUnions.ts, 18, 38))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 17, 35))

V extends HTMLElementTagNameMap[T][P]>(node: Node | null, tagName: T, prop: P, value: V) {
>V : Symbol(V, Decl(intersectionsOfLargeUnions.ts, 19, 41))
>HTMLElementTagNameMap : Symbol(HTMLElementTagNameMap, Decl(lib.d.ts, --, --))
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 17, 35))
>P : Symbol(P, Decl(intersectionsOfLargeUnions.ts, 18, 38))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 20, 43))
>Node : Symbol(Node, Decl(lib.d.ts, --, --), Decl(lib.d.ts, --, --))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions.ts, 20, 61))
>T : Symbol(T, Decl(intersectionsOfLargeUnions.ts, 17, 35))
>prop : Symbol(prop, Decl(intersectionsOfLargeUnions.ts, 20, 73))
>P : Symbol(P, Decl(intersectionsOfLargeUnions.ts, 18, 38))
>value : Symbol(value, Decl(intersectionsOfLargeUnions.ts, 20, 82))
>V : Symbol(V, Decl(intersectionsOfLargeUnions.ts, 19, 41))

if (assertNodeTagName(node, tagName)) {
>assertNodeTagName : Symbol(assertNodeTagName, Decl(intersectionsOfLargeUnions.ts, 5, 1))
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 20, 43))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions.ts, 20, 61))

node[prop];
>node : Symbol(node, Decl(intersectionsOfLargeUnions.ts, 20, 43))
>prop : Symbol(prop, Decl(intersectionsOfLargeUnions.ts, 20, 73))
}
}

Loading