From b83dc88f9b9f0d874adea078a8c5efceb6841ab0 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Mon, 18 Apr 2016 13:51:40 -0700 Subject: [PATCH] Improve expression type caching to ensure consistent results --- src/compiler/checker.ts | 71 ++++++++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 1eeec55db8d..a7a875892d8 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -180,7 +180,9 @@ namespace ts { let jsxElementClassType: Type; let deferredNodes: Node[]; - let inFlowCheck = false; + + let flowStackStart = 0; + let flowStackCount = 0; const tupleTypes: Map = {}; const unionTypes: Map = {}; @@ -195,6 +197,8 @@ namespace ts { const symbolLinks: SymbolLinks[] = []; const nodeLinks: NodeLinks[] = []; const flowTypeCaches: Map[] = []; + const flowStackNodes: FlowNode[] = []; + const flowStackCacheKeys: string[] = []; const potentialThisCollisions: Node[] = []; const awaitedTypeStack: number[] = []; @@ -7409,7 +7413,7 @@ namespace ts { function getTypeWithDefault(type: Type, defaultExpression: Expression) { if (defaultExpression) { - const defaultType = checkExpressionCached(defaultExpression); + const defaultType = checkExpression(defaultExpression); return getUnionType([getTypeWithFacts(type, TypeFacts.NEUndefined), defaultType]); } return type; @@ -7436,7 +7440,7 @@ namespace ts { function getAssignedTypeOfBinaryExpression(node: BinaryExpression): Type { return node.parent.kind === SyntaxKind.ArrayLiteralExpression || node.parent.kind === SyntaxKind.PropertyAssignment ? getTypeWithDefault(getAssignedType(node), node.right) : - checkExpressionCached(node.right); + checkExpression(node.right); } function getAssignedTypeOfArrayLiteralElement(node: ArrayLiteralExpression, element: Expression): Type { @@ -7487,9 +7491,17 @@ namespace ts { return getTypeWithDefault(type, node.initializer); } + function getTypeOfInitializer(node: Expression) { + // Return the cached type if one is available. If the type of the variable was inferred + // from its initializer, we'll already have cached the type. Otherwise we compute it now + // without caching such that transient types are reflected. + const links = getNodeLinks(node); + return links.resolvedType || checkExpression(node); + } + function getInitialTypeOfVariableDeclaration(node: VariableDeclaration) { if (node.initializer) { - return checkExpressionCached(node.initializer); + return getTypeOfInitializer(node.initializer); } if (node.parent.parent.kind === SyntaxKind.ForInStatement) { return stringType; @@ -7529,11 +7541,7 @@ namespace ts { function getFlowTypeOfReference(reference: Node, declaredType: Type, initialType: Type) { let key: string; - const saveInFlowCheck = inFlowCheck; - inFlowCheck = true; - const result = reference.flowNode ? getTypeAtFlowNode(reference.flowNode) : declaredType; - inFlowCheck = saveInFlowCheck; - return result; + return reference.flowNode ? getTypeAtFlowNode(reference.flowNode) : declaredType; function getTypeAtFlowNode(flow: FlowNode): Type { while (true) { @@ -7601,30 +7609,41 @@ namespace ts { if (!key) { key = getFlowCacheKey(reference); } - let type = cache[key]; - if (type) { - return type; + const cached = cache[key]; + if (cached) { + return cached; } - cache[key] = resolvingFlowType; - type = getTypeAtFlowNode(flow); - cache[key] = type !== resolvingFlowType ? type : undefined; - return type; + // Return undefined if we're already processing the given node. + for (let i = flowStackStart; i < flowStackCount; i++) { + if (flowStackNodes[i] === flow && flowStackCacheKeys[i] === key) { + return undefined; + } + } + // Record node and key on stack of nodes being processed. + flowStackNodes[flowStackCount] = flow; + flowStackCacheKeys[flowStackCount] = key; + flowStackCount++; + const type = getTypeAtFlowNode(flow); + flowStackCount--; + // Record the result only if the cache is still empty. If checkExpressionCached was called + // during processing it is possible we've already recorded a result. + return cache[key] || (cache[key] = type); } function getTypeAtFlowLabel(flow: FlowLabel) { const antecedentTypes: Type[] = []; for (const antecedent of flow.antecedents) { - const t = getTypeAtFlowNodeCached(antecedent); - if (t !== resolvingFlowType) { + const type = getTypeAtFlowNodeCached(antecedent); + if (type) { // If the type at a particular antecedent path is the declared type and the // reference is known to always be assigned (i.e. when declared and initial types // are the same), there is no reason to process more antecedents since the only // possible outcome is subtypes that will be removed in the final union type anyway. - if (t === declaredType && declaredType === initialType) { - return t; + if (type === declaredType && declaredType === initialType) { + return type; } - if (!contains(antecedentTypes, t)) { - antecedentTypes.push(t); + if (!contains(antecedentTypes, type)) { + antecedentTypes.push(type); } } } @@ -11107,7 +11126,7 @@ namespace ts { // If signature resolution originated in control flow type analysis (for example to compute the // assigned type in a flow assignment) we don't cache the result as it may be based on temporary // types from the control flow analysis. - links.resolvedSignature = inFlowCheck ? cached : result; + links.resolvedSignature = flowStackStart === flowStackCount ? result : cached; return result; } @@ -12246,7 +12265,13 @@ namespace ts { function checkExpressionCached(node: Expression, contextualMapper?: TypeMapper): Type { const links = getNodeLinks(node); if (!links.resolvedType) { + // When computing a type that we're going to cache, we need to ignore any ongoing control flow + // analysis because variables may have transient types in indeterminable states. Moving flowStackStart + // to the top of the stack ensures all transient types are computed from a known point. + const saveFlowStackStart = flowStackStart; + flowStackStart = flowStackCount; links.resolvedType = checkExpression(node, contextualMapper); + flowStackStart = saveFlowStackStart; } return links.resolvedType; }