Skip to content

Fix infinite recursion in union type reduction #3157

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 6 commits into from
May 15, 2015
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
29 changes: 13 additions & 16 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ module ts {
let undefinedType = createIntrinsicType(TypeFlags.Undefined | TypeFlags.ContainsUndefinedOrNull, "undefined");
let nullType = createIntrinsicType(TypeFlags.Null | TypeFlags.ContainsUndefinedOrNull, "null");
let unknownType = createIntrinsicType(TypeFlags.Any, "unknown");
let circularType = createIntrinsicType(TypeFlags.Any, "__circular__");

let emptyObjectType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
let anyFunctionType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
Expand Down Expand Up @@ -3566,28 +3567,14 @@ module ts {
return false;
}

// Since removeSubtypes checks the subtype relation, and the subtype relation on a union
// may attempt to reduce a union, it is possible that removeSubtypes could be called
// recursively on the same set of types. The removeSubtypesStack is used to track which
// sets of types are currently undergoing subtype reduction.
let removeSubtypesStack: string[] = [];
function removeSubtypes(types: Type[]) {
let typeListId = getTypeListId(types);
if (removeSubtypesStack.lastIndexOf(typeListId) >= 0) {
return;
}

removeSubtypesStack.push(typeListId);

let i = types.length;
while (i > 0) {
i--;
if (isSubtypeOfAny(types[i], types)) {
types.splice(i, 1);
}
}

removeSubtypesStack.pop();
}

function containsAnyType(types: Type[]) {
Expand Down Expand Up @@ -3642,10 +3629,20 @@ module ts {
return type;
}

// Subtype reduction is basically an optimization we do to avoid excessively large union types, which take longer
// to process and look strange in quick info and error messages. Semantically there is no difference between the
// reduced type and the type itself. So, when we detect a circularity we simply say that the reduced type is the
// type itself.
function getReducedTypeOfUnionType(type: UnionType): Type {
// If union type was created without subtype reduction, perform the deferred reduction now
if (!type.reducedType) {
type.reducedType = getUnionType(type.types, /*noSubtypeReduction*/ false);
type.reducedType = circularType;
let reducedType = getUnionType(type.types, /*noSubtypeReduction*/ false);
if (type.reducedType === circularType) {
type.reducedType = reducedType;
}
}
else if (type.reducedType === circularType) {
type.reducedType = type;
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. No error when we hit the circularity? Or is that handled elsewhere? If handled elsewhere, can you comment in the code where.
  2. It seems like you're setting the reduced type to the type itself. That seems... odd. What's the intuition for why that's the appropriate type.

Copy link
Member Author

Choose a reason for hiding this comment

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

Subtype reduction is basically an optimization we do to avoid excessively large union types, which take longer to process and look strange in quick info and error messages. Semantically there is no difference between the reduced type and the type itself. So, when we detect a circularity we simply say that the reduced type is the type itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. We should definitely state this in the code itself with a comment of some sort. It's not at all self-evident, and it's definitely important for the code to make this clear that this is expected and desirable behavior.

}
return type.reducedType;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//// [unionTypeWithRecursiveSubtypeReduction.ts]
//// [unionTypeWithRecursiveSubtypeReduction1.ts]
class Module {
public members: Class[];
}
Expand All @@ -16,9 +16,10 @@ class Property {
}

var t: Class | Property;
t.parent;
t.parent;


//// [unionTypeWithRecursiveSubtypeReduction.js]
//// [unionTypeWithRecursiveSubtypeReduction1.js]
var Module = (function () {
function Module() {
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,45 +1,45 @@
=== tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction.ts ===
=== tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction1.ts ===
class Module {
>Module : Symbol(Module, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 0, 0))
>Module : Symbol(Module, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 0, 0))

public members: Class[];
>members : Symbol(members, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 0, 14))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 6, 1))
>members : Symbol(members, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 0, 14))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 6, 1))
}

class Namespace {
>Namespace : Symbol(Namespace, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 2, 1))
>Namespace : Symbol(Namespace, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 2, 1))

public members: (Class | Property)[];
>members : Symbol(members, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 4, 17))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 6, 1))
>Property : Symbol(Property, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 10, 1))
>members : Symbol(members, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 4, 17))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 6, 1))
>Property : Symbol(Property, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 10, 1))
}

class Class {
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 6, 1))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 6, 1))

public parent: Namespace;
>parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 8, 13))
>Namespace : Symbol(Namespace, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 2, 1))
>parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 8, 13))
>Namespace : Symbol(Namespace, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 2, 1))
}

class Property {
>Property : Symbol(Property, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 10, 1))
>Property : Symbol(Property, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 10, 1))

public parent: Module | Class;
>parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 12, 16))
>Module : Symbol(Module, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 0, 0))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 6, 1))
>parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 12, 16))
>Module : Symbol(Module, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 0, 0))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 6, 1))
}

var t: Class | Property;
>t : Symbol(t, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 16, 3))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 6, 1))
>Property : Symbol(Property, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 10, 1))
>t : Symbol(t, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 16, 3))
>Class : Symbol(Class, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 6, 1))
>Property : Symbol(Property, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 10, 1))

t.parent;
>t.parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 8, 13), Decl(unionTypeWithRecursiveSubtypeReduction.ts, 12, 16))
>t : Symbol(t, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 16, 3))
>parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction.ts, 8, 13), Decl(unionTypeWithRecursiveSubtypeReduction.ts, 12, 16))
>t.parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 8, 13), Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 12, 16))
>t : Symbol(t, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 16, 3))
>parent : Symbol(parent, Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 8, 13), Decl(unionTypeWithRecursiveSubtypeReduction1.ts, 12, 16))

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
=== tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction.ts ===
=== tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction1.ts ===
class Module {
>Module : Module

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@ tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction2.ts(20,1): error TS2
!!! error TS2322: Types of property 'parent' are incompatible.
!!! error TS2322: Type 'Namespace' is not assignable to type 'Module | Class'.
!!! error TS2322: Type 'Namespace' is not assignable to type 'Class'.
!!! error TS2322: Property 'parent' is missing in type 'Namespace'.
!!! error TS2322: Property 'parent' is missing in type 'Namespace'.

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ class Property {
var c: Class;
var p: Property;
c = p;
p = c;
p = c;


//// [unionTypeWithRecursiveSubtypeReduction2.js]
var Module = (function () {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction3.ts(5,5): error TS2322: Type '{ prop: number; } | { prop: { prop: number; } | any; }' is not assignable to type 'string'.
Type '{ prop: number; }' is not assignable to type 'string'.


==== tests/cases/compiler/unionTypeWithRecursiveSubtypeReduction3.ts (1 errors) ====
var a27: { prop: number } | { prop: T27 };
type T27 = typeof a27;

var b: T27;
var s: string = b;
~
!!! error TS2322: Type '{ prop: number; } | { prop: { prop: number; } | any; }' is not assignable to type 'string'.
!!! error TS2322: Type '{ prop: number; }' is not assignable to type 'string'.

Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [unionTypeWithRecursiveSubtypeReduction3.ts]
var a27: { prop: number } | { prop: T27 };
type T27 = typeof a27;

var b: T27;
var s: string = b;


//// [unionTypeWithRecursiveSubtypeReduction3.js]
var a27;
var b;
var s = b;
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ class Property {
}

var t: Class | Property;
t.parent;
t.parent;
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ class Property {
var c: Class;
var p: Property;
c = p;
p = c;
p = c;
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
var a27: { prop: number } | { prop: T27 };
type T27 = typeof a27;

var b: T27;
var s: string = b;