From e9ccb1664263c88702cf8223ce3dd73f7dfd4751 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 15 May 2017 15:32:14 -0700 Subject: [PATCH 1/2] Eliminate redundant exploration in type inference --- src/compiler/checker.ts | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d6f56a69b9..26cdab1e59 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -10096,21 +10096,29 @@ namespace ts { inferTypes(context.signature.typeParameters, context.inferences, originalSource, originalTarget); } + function getSymbolForInference(type: Type) { + // Exclude the static side of classes since it shares its symbol with the instance side which leads + // to false positives. + return type.flags & TypeFlags.Object && !(getObjectFlags(type) & ObjectFlags.Anonymous && type.symbol && type.symbol.flags & SymbolFlags.Class) ? type.symbol : undefined; + } + function inferTypes(typeVariables: TypeVariable[], typeInferences: TypeInferences[], originalSource: Type, originalTarget: Type) { - let sourceStack: Type[]; - let targetStack: Type[]; + let stack: Type[]; let depth = 0; let inferiority = 0; const visited = createMap(); inferFromTypes(originalSource, originalTarget); - function isInProcess(source: Type, target: Type) { - for (let i = 0; i < depth; i++) { - if (source === sourceStack[i] && target === targetStack[i]) { - return true; + function isInstantiationInProcess(type: Type) { + const symbol = getSymbolForInference(type); + if (symbol) { + for (let i = 0; i < depth; i++) { + const t = stack[i]; + if (getSymbolForInference(t) === symbol) { + return true; + } } } - return false; } function inferFromTypes(source: Type, target: Type) { @@ -10240,10 +10248,10 @@ namespace ts { else { source = getApparentType(source); if (source.flags & TypeFlags.Object) { - if (isInProcess(source, target)) { - return; - } - if (isDeeplyNestedType(source, sourceStack, depth) && isDeeplyNestedType(target, targetStack, depth)) { + // If we are already processing another target type with the same associated symbol (such as + // an instantiation of the same generic type), we do not explore this target as it would yield + // no further inferences. + if (isInstantiationInProcess(target)) { return; } const key = source.id + "," + target.id; @@ -10251,12 +10259,7 @@ namespace ts { return; } visited.set(key, true); - if (depth === 0) { - sourceStack = []; - targetStack = []; - } - sourceStack[depth] = source; - targetStack[depth] = target; + (stack || (stack = []))[depth] = target; depth++; inferFromObjectTypes(source, target); depth--; From ed1a6c10e2281d8c51b53d60e6957a03289d4a73 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Tue, 16 May 2017 09:12:32 -0700 Subject: [PATCH 2/2] Address CR feedback + defer creation of visited map --- src/compiler/checker.ts | 53 ++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 32 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 26cdab1e59..602125091a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -10096,31 +10096,12 @@ namespace ts { inferTypes(context.signature.typeParameters, context.inferences, originalSource, originalTarget); } - function getSymbolForInference(type: Type) { - // Exclude the static side of classes since it shares its symbol with the instance side which leads - // to false positives. - return type.flags & TypeFlags.Object && !(getObjectFlags(type) & ObjectFlags.Anonymous && type.symbol && type.symbol.flags & SymbolFlags.Class) ? type.symbol : undefined; - } - function inferTypes(typeVariables: TypeVariable[], typeInferences: TypeInferences[], originalSource: Type, originalTarget: Type) { - let stack: Type[]; - let depth = 0; + let symbolStack: Symbol[]; + let visited: Map; let inferiority = 0; - const visited = createMap(); inferFromTypes(originalSource, originalTarget); - function isInstantiationInProcess(type: Type) { - const symbol = getSymbolForInference(type); - if (symbol) { - for (let i = 0; i < depth; i++) { - const t = stack[i]; - if (getSymbolForInference(t) === symbol) { - return true; - } - } - } - } - function inferFromTypes(source: Type, target: Type) { if (!couldContainTypeVariables(target)) { return; @@ -10248,21 +10229,29 @@ namespace ts { else { source = getApparentType(source); if (source.flags & TypeFlags.Object) { + const key = source.id + "," + target.id; + if (visited && visited.get(key)) { + return; + } + (visited || (visited = createMap())).set(key, true); // If we are already processing another target type with the same associated symbol (such as // an instantiation of the same generic type), we do not explore this target as it would yield - // no further inferences. - if (isInstantiationInProcess(target)) { - return; + // no further inferences. We exclude the static side of classes from this check since it shares + // its symbol with the instance side which would lead to false positives. + const isNonConstructorObject = target.flags & TypeFlags.Object && + !(getObjectFlags(target) & ObjectFlags.Anonymous && target.symbol && target.symbol.flags & SymbolFlags.Class); + const symbol = isNonConstructorObject ? target.symbol : undefined; + if (symbol) { + if (contains(symbolStack, symbol)) { + return; + } + (symbolStack || (symbolStack = [])).push(symbol); + inferFromObjectTypes(source, target); + symbolStack.pop(); } - const key = source.id + "," + target.id; - if (visited.get(key)) { - return; + else { + inferFromObjectTypes(source, target); } - visited.set(key, true); - (stack || (stack = []))[depth] = target; - depth++; - inferFromObjectTypes(source, target); - depth--; } } }