Skip to content

Optimize creation of intersections of union types #25859

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
Jul 25, 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
80 changes: 63 additions & 17 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8533,6 +8533,15 @@ namespace ts {
return binarySearch(types, type, getTypeId, compareValues) >= 0;
}

function insertType(types: Type[], type: Type): boolean {
const index = binarySearch(types, type, getTypeId, compareValues);
if (index < 0) {
types.splice(~index, 0, type);
return true;
}
return false;
}

// Return true if the given intersection type contains
// more than one unit type or,
// an object type and a nullable type (null or undefined), or
Expand Down Expand Up @@ -8700,7 +8709,7 @@ namespace ts {
includes & TypeFlags.Undefined ? includes & TypeFlags.NonWideningType ? undefinedType : undefinedWideningType :
neverType;
}
return getUnionTypeFromSortedList(typeSet, includes & TypeFlags.NotUnit ? 0 : TypeFlags.UnionOfUnitTypes, aliasSymbol, aliasTypeArguments);
return getUnionTypeFromSortedList(typeSet, includes & TypeFlags.NotPrimitiveUnion ? 0 : TypeFlags.UnionOfPrimitiveTypes, aliasSymbol, aliasTypeArguments);
}

function getUnionTypePredicate(signatures: ReadonlyArray<Signature>): TypePredicate | undefined {
Expand Down Expand Up @@ -8823,26 +8832,63 @@ 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) {
// Check that the given type has a match in every union. A given type is matched by
// an identical type, and a literal type is additionally matched by its corresponding
// primitive type.
function eachUnionContains(unionTypes: UnionType[], type: Type) {
for (const u of unionTypes) {
if (!containsType(u.types, type)) {
const primitive = type.flags & TypeFlags.StringLiteral ? stringType :
Copy link
Member

@DanielRosenwasser DanielRosenwasser Jul 24, 2018

Choose a reason for hiding this comment

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

This logic is very close to getBaseTypeOfLiteralType; can you use that instead? That also covers enums, though it won't come into play for the motivating issue.

Copy link
Member

Choose a reason for hiding this comment

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

Although a 200 member enum could easily cause the same issues, so is probably worth covering!

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we can't use that instead because it maps an enum literal type to its defining union type (the union of all enum literal types in the same enum type declaration) which we don't want here. We only want to handle the special "infinite sets" denoted by string, number, and symbol.

type.flags & TypeFlags.NumberLiteral ? numberType :
type.flags & TypeFlags.UniqueESSymbol ? esSymbolType :
undefined;
if (!primitive || !containsType(u.types, primitive)) {
return false;
}
}
}
return true;
}

// If the given list of types contains more than one union of primitive types, replace the
// first with a union containing an intersection of those primitive types, then remove the
// other unions and return true. Otherwise, do nothing and return false.
function intersectUnionsOfPrimitiveTypes(types: Type[]) {
let unionTypes: UnionType[] | undefined;
const index = findIndex(types, t => (t.flags & TypeFlags.UnionOfPrimitiveTypes) !== 0);
let i = index + 1;
// Remove all but the first union of primitive types and collect them in
// the unionTypes array.
while (i < types.length) {
const t = types[i];
if (t.flags & TypeFlags.UnionOfUnitTypes) {
intersection = filter(intersection, u => containsType((<UnionType>t).types, u));
if (t.flags & TypeFlags.UnionOfPrimitiveTypes) {
(unionTypes || (unionTypes = [<UnionType>types[index]])).push(<UnionType>t);
orderedRemoveItemAt(types, i);
}
i--;
else {
i++;
}
}
if (intersection === unionType.types) {
// Return false if there was only one union of primitive types
if (!unionTypes) {
return false;
}
types[unionIndex] = getUnionTypeFromSortedList(intersection, unionType.flags & TypeFlags.UnionOfUnitTypes);
// We have more than one union of primitive types, now intersect them. For each
// type in each union we check if the type is matched in every union and if so
// we include it in the result.
const checked: Type[] = [];
const result: Type[] = [];
for (const u of unionTypes) {
for (const t of u.types) {
Copy link
Member

Choose a reason for hiding this comment

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

In the worst-case, this can force checked to reallocate or bulk-move all the elements a lot. Do we believe that won't be an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

We could potentially use a Map but that's going to allocate more memory and go slower in the typical cases (i.e. reasonably sized sets of types with a high degree of similarity). I'm not too concerned with it.

if (insertType(checked, t)) {
if (eachUnionContains(unionTypes, t)) {
insertType(result, t);
}
}
}
}
// Finally replace the first union with the result
types[index] = getUnionTypeFromSortedList(result, TypeFlags.UnionOfPrimitiveTypes);
return true;
}

Expand Down Expand Up @@ -8883,7 +8929,7 @@ namespace ts {
return typeSet[0];
}
if (includes & TypeFlags.Union) {
if (includes & TypeFlags.UnionOfUnitTypes && intersectUnionsOfUnitTypes(typeSet)) {
if (includes & TypeFlags.UnionOfPrimitiveTypes && intersectUnionsOfPrimitiveTypes(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.
Expand Down Expand Up @@ -13980,7 +14026,7 @@ namespace ts {
if (type.flags & TypeFlags.Union) {
const types = (<UnionType>type).types;
const filtered = filter(types, f);
return filtered === types ? type : getUnionTypeFromSortedList(filtered, type.flags & TypeFlags.UnionOfUnitTypes);
return filtered === types ? type : getUnionTypeFromSortedList(filtered, type.flags & TypeFlags.UnionOfPrimitiveTypes);
}
return f(type) ? type : neverType;
}
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3675,7 +3675,7 @@ namespace ts {
/* @internal */
FreshLiteral = 1 << 25, // Fresh literal or unique type
/* @internal */
UnionOfUnitTypes = 1 << 26, // Type is union of unit types
UnionOfPrimitiveTypes = 1 << 26, // Type is union of primitive types
/* @internal */
ContainsWideningType = 1 << 27, // Type is or contains undefined or null widening type
/* @internal */
Expand Down Expand Up @@ -3720,7 +3720,7 @@ namespace ts {
Narrowable = Any | Unknown | StructuredOrInstantiable | StringLike | NumberLike | BooleanLike | ESSymbol | UniqueESSymbol | NonPrimitive,
NotUnionOrUnit = Any | Unknown | ESSymbol | Object | NonPrimitive,
/* @internal */
NotUnit = Any | String | Number | Boolean | Enum | ESSymbol | Void | Never | StructuredOrInstantiable,
NotPrimitiveUnion = Any | Unknown | Enum | Void | Never | StructuredOrInstantiable,
/* @internal */
RequiresWidening = ContainsWideningType | ContainsObjectLiteral,
/* @internal */
Expand Down
45 changes: 45 additions & 0 deletions tests/baselines/reference/intersectionsOfLargeUnions2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
tests/cases/compiler/intersectionsOfLargeUnions2.ts(31,15): error TS2536: Type 'T' cannot be used to index type 'HTMLElementTagNameMap'.
tests/cases/compiler/intersectionsOfLargeUnions2.ts(31,15): error TS2536: Type 'P' cannot be used to index type 'HTMLElementTagNameMap[T]'.


==== tests/cases/compiler/intersectionsOfLargeUnions2.ts (2 errors) ====
// Repro from #24233

declare global {
interface ElementTagNameMap {
[index: number]: HTMLElement
}

interface HTMLElement {
[index: number]: HTMLElement;
}
}

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];
}
}

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

declare global {
interface ElementTagNameMap {
[index: number]: HTMLElement
}

interface HTMLElement {
[index: number]: HTMLElement;
}
}

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];
}
}


//// [intersectionsOfLargeUnions2.js]
"use strict";
// Repro from #24233
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;
115 changes: 115 additions & 0 deletions tests/baselines/reference/intersectionsOfLargeUnions2.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
=== tests/cases/compiler/intersectionsOfLargeUnions2.ts ===
// Repro from #24233

declare global {
>global : Symbol(global, Decl(intersectionsOfLargeUnions2.ts, 0, 0))

interface ElementTagNameMap {
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 2, 16))

[index: number]: HTMLElement
>index : Symbol(index, Decl(intersectionsOfLargeUnions2.ts, 4, 9))
>HTMLElement : Symbol(HTMLElement, Decl(intersectionsOfLargeUnions2.ts, 5, 5))
}

interface HTMLElement {
>HTMLElement : Symbol(HTMLElement, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 5, 5))

[index: number]: HTMLElement;
>index : Symbol(index, Decl(intersectionsOfLargeUnions2.ts, 8, 9))
>HTMLElement : Symbol(HTMLElement, Decl(intersectionsOfLargeUnions2.ts, 5, 5))
}
}

export function assertIsElement(node: Node | null): node is Element {
>assertIsElement : Symbol(assertIsElement, Decl(intersectionsOfLargeUnions2.ts, 10, 1))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 12, 32))
>Node : Symbol(Node, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 12, 32))
>Element : Symbol(Element, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))

let nodeType = node === null ? null : node.nodeType;
>nodeType : Symbol(nodeType, Decl(intersectionsOfLargeUnions2.ts, 13, 7))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 12, 32))
>node.nodeType : Symbol(Node.nodeType, Decl(lib.dom.d.ts, --, --))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 12, 32))
>nodeType : Symbol(Node.nodeType, Decl(lib.dom.d.ts, --, --))

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

export function assertNodeTagName<
>assertNodeTagName : Symbol(assertNodeTagName, Decl(intersectionsOfLargeUnions2.ts, 15, 1))

T extends keyof ElementTagNameMap,
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 17, 34))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 2, 16))

U extends ElementTagNameMap[T]>(node: Node | null, tagName: T): node is U {
>U : Symbol(U, Decl(intersectionsOfLargeUnions2.ts, 18, 38))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 2, 16))
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 17, 34))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 19, 36))
>Node : Symbol(Node, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions2.ts, 19, 54))
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 17, 34))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 19, 36))
>U : Symbol(U, Decl(intersectionsOfLargeUnions2.ts, 18, 38))

if (assertIsElement(node)) {
>assertIsElement : Symbol(assertIsElement, Decl(intersectionsOfLargeUnions2.ts, 10, 1))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 19, 36))

const nodeTagName = node.tagName.toLowerCase();
>nodeTagName : Symbol(nodeTagName, Decl(intersectionsOfLargeUnions2.ts, 21, 13))
>node.tagName.toLowerCase : Symbol(String.toLowerCase, Decl(lib.es5.d.ts, --, --))
>node.tagName : Symbol(Element.tagName, Decl(lib.dom.d.ts, --, --))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 19, 36))
>tagName : Symbol(Element.tagName, Decl(lib.dom.d.ts, --, --))
>toLowerCase : Symbol(String.toLowerCase, Decl(lib.es5.d.ts, --, --))

return nodeTagName === tagName;
>nodeTagName : Symbol(nodeTagName, Decl(intersectionsOfLargeUnions2.ts, 21, 13))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions2.ts, 19, 54))
}
return false;
}

export function assertNodeProperty<
>assertNodeProperty : Symbol(assertNodeProperty, Decl(intersectionsOfLargeUnions2.ts, 25, 1))

T extends keyof ElementTagNameMap,
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 27, 35))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 2, 16))

P extends keyof ElementTagNameMap[T],
>P : Symbol(P, Decl(intersectionsOfLargeUnions2.ts, 28, 38))
>ElementTagNameMap : Symbol(ElementTagNameMap, Decl(lib.dom.d.ts, --, --), Decl(intersectionsOfLargeUnions2.ts, 2, 16))
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 27, 35))

V extends HTMLElementTagNameMap[T][P]>(node: Node | null, tagName: T, prop: P, value: V) {
>V : Symbol(V, Decl(intersectionsOfLargeUnions2.ts, 29, 41))
>HTMLElementTagNameMap : Symbol(HTMLElementTagNameMap, Decl(lib.dom.d.ts, --, --))
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 27, 35))
>P : Symbol(P, Decl(intersectionsOfLargeUnions2.ts, 28, 38))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 30, 43))
>Node : Symbol(Node, Decl(lib.dom.d.ts, --, --), Decl(lib.dom.d.ts, --, --))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions2.ts, 30, 61))
>T : Symbol(T, Decl(intersectionsOfLargeUnions2.ts, 27, 35))
>prop : Symbol(prop, Decl(intersectionsOfLargeUnions2.ts, 30, 73))
>P : Symbol(P, Decl(intersectionsOfLargeUnions2.ts, 28, 38))
>value : Symbol(value, Decl(intersectionsOfLargeUnions2.ts, 30, 82))
>V : Symbol(V, Decl(intersectionsOfLargeUnions2.ts, 29, 41))

if (assertNodeTagName(node, tagName)) {
>assertNodeTagName : Symbol(assertNodeTagName, Decl(intersectionsOfLargeUnions2.ts, 15, 1))
>node : Symbol(node, Decl(intersectionsOfLargeUnions2.ts, 30, 43))
>tagName : Symbol(tagName, Decl(intersectionsOfLargeUnions2.ts, 30, 61))

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

Loading