Merge pull request microsoft#8286 from Microsoft/controlFlowCacheFix

ahejlsberg · ahejlsberg · commit dc60e684689e · 2016-04-25T12:26:30.000-07:00
Control flow cache fix
diff --git a/src/compiler/binder.ts b/src/compiler/binder.ts
@@ -645,6 +645,13 @@ namespace ts {
             };
         }
 
+        function createFlowLoopLabel(): FlowLabel {
+            return {
+                kind: FlowKind.LoopLabel,
+                antecedents: undefined
+            };
+        }
+
         function addAntecedent(label: FlowLabel, antecedent: FlowNode): void {
             if (antecedent.kind !== FlowKind.Unreachable && !contains(label.antecedents, antecedent)) {
                 (label.antecedents || (label.antecedents = [])).push(antecedent);
@@ -755,7 +762,7 @@ namespace ts {
         }
 
         function bindWhileStatement(node: WhileStatement): void {
-            const preWhileLabel = createFlowLabel();
+            const preWhileLabel = createFlowLoopLabel();
             const preBodyLabel = createFlowLabel();
             const postWhileLabel = createFlowLabel();
             addAntecedent(preWhileLabel, currentFlow);
@@ -768,7 +775,7 @@ namespace ts {
         }
 
         function bindDoStatement(node: DoStatement): void {
-            const preDoLabel = createFlowLabel();
+            const preDoLabel = createFlowLoopLabel();
             const preConditionLabel = createFlowLabel();
             const postDoLabel = createFlowLabel();
             addAntecedent(preDoLabel, currentFlow);
@@ -781,7 +788,7 @@ namespace ts {
         }
 
         function bindForStatement(node: ForStatement): void {
-            const preLoopLabel = createFlowLabel();
+            const preLoopLabel = createFlowLoopLabel();
             const preBodyLabel = createFlowLabel();
             const postLoopLabel = createFlowLabel();
             bind(node.initializer);
@@ -796,7 +803,7 @@ namespace ts {
         }
 
         function bindForInOrForOfStatement(node: ForInStatement | ForOfStatement): void {
-            const preLoopLabel = createFlowLabel();
+            const preLoopLabel = createFlowLoopLabel();
             const postLoopLabel = createFlowLabel();
             addAntecedent(preLoopLabel, currentFlow);
             currentFlow = preLoopLabel;
@@ -943,7 +950,7 @@ namespace ts {
         }
 
         function bindLabeledStatement(node: LabeledStatement): void {
-            const preStatementLabel = createFlowLabel();
+            const preStatementLabel = createFlowLoopLabel();
             const postStatementLabel = createFlowLabel();
             bind(node.label);
             addAntecedent(preStatementLabel, currentFlow);
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -7592,6 +7592,7 @@ namespace ts {
                         case FlowKind.Condition:
                             return getTypeAtFlowCondition(<FlowCondition>flow);
                         case FlowKind.Label:
+                        case FlowKind.LoopLabel:
                             if ((<FlowLabel>flow).antecedents.length === 1) {
                                 flow = (<FlowLabel>flow).antecedents[0];
                                 continue;
@@ -7639,7 +7640,8 @@ namespace ts {
             }
 
             function getTypeAtFlowCondition(flow: FlowCondition) {
-                return narrowType(getTypeAtFlowNode(flow.antecedent), flow.expression, flow.assumeTrue);
+                const type = getTypeAtFlowNode(flow.antecedent);
+                return type && narrowType(type, flow.expression, flow.assumeTrue);
             }
 
             function getTypeAtFlowNodeCached(flow: FlowNode) {
@@ -7665,13 +7667,15 @@ namespace ts {
                 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);
+                return cache[key] || type && (cache[key] = type);
             }
 
             function getTypeAtFlowLabel(flow: FlowLabel) {
                 const antecedentTypes: Type[] = [];
                 for (const antecedent of flow.antecedents) {
-                    const type = getTypeAtFlowNodeCached(antecedent);
+                    const type = flow.kind === FlowKind.LoopLabel ?
+                        getTypeAtFlowNodeCached(antecedent) :
+                        getTypeAtFlowNode(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
@@ -7685,7 +7689,7 @@ namespace ts {
                         }
                     }
                 }
-                return antecedentTypes.length === 0 ? declaredType :
+                return antecedentTypes.length === 0 ? undefined :
                     antecedentTypes.length === 1 ? antecedentTypes[0] :
                     getUnionType(antecedentTypes);
             }
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -1519,6 +1519,7 @@ namespace ts {
         Unreachable,
         Start,
         Label,
+        LoopLabel,
         Assignment,
         Condition
     }
diff --git a/tests/baselines/reference/controlFlowIteration.js b/tests/baselines/reference/controlFlowIteration.js
@@ -0,0 +1,40 @@
+//// [controlFlowIteration.ts]
+
+let cond: boolean;
+
+function ff() {
+    let x: string | undefined;
+    while (true) {
+        if (cond) {
+            x = "";
+        }
+        else {
+            if (x) {
+                x.length;
+            }
+            if (x) {
+                x.length;
+            }
+        }
+    }
+}
+
+
+//// [controlFlowIteration.js]
+var cond;
+function ff() {
+    var x;
+    while (true) {
+        if (cond) {
+            x = "";
+        }
+        else {
+            if (x) {
+                x.length;
+            }
+            if (x) {
+                x.length;
+            }
+        }
+    }
+}
diff --git a/tests/baselines/reference/controlFlowIteration.symbols b/tests/baselines/reference/controlFlowIteration.symbols
@@ -0,0 +1,39 @@
+=== tests/cases/conformance/controlFlow/controlFlowIteration.ts ===
+
+let cond: boolean;
+>cond : Symbol(cond, Decl(controlFlowIteration.ts, 1, 3))
+
+function ff() {
+>ff : Symbol(ff, Decl(controlFlowIteration.ts, 1, 18))
+
+    let x: string | undefined;
+>x : Symbol(x, Decl(controlFlowIteration.ts, 4, 7))
+
+    while (true) {
+        if (cond) {
+>cond : Symbol(cond, Decl(controlFlowIteration.ts, 1, 3))
+
+            x = "";
+>x : Symbol(x, Decl(controlFlowIteration.ts, 4, 7))
+        }
+        else {
+            if (x) {
+>x : Symbol(x, Decl(controlFlowIteration.ts, 4, 7))
+
+                x.length;
+>x.length : Symbol(String.length, Decl(lib.d.ts, --, --))
+>x : Symbol(x, Decl(controlFlowIteration.ts, 4, 7))
+>length : Symbol(String.length, Decl(lib.d.ts, --, --))
+            }
+            if (x) {
+>x : Symbol(x, Decl(controlFlowIteration.ts, 4, 7))
+
+                x.length;
+>x.length : Symbol(String.length, Decl(lib.d.ts, --, --))
+>x : Symbol(x, Decl(controlFlowIteration.ts, 4, 7))
+>length : Symbol(String.length, Decl(lib.d.ts, --, --))
+            }
+        }
+    }
+}
+
diff --git a/tests/baselines/reference/controlFlowIteration.types b/tests/baselines/reference/controlFlowIteration.types
@@ -0,0 +1,43 @@
+=== tests/cases/conformance/controlFlow/controlFlowIteration.ts ===
+
+let cond: boolean;
+>cond : boolean
+
+function ff() {
+>ff : () => void
+
+    let x: string | undefined;
+>x : string | undefined
+
+    while (true) {
+>true : boolean
+
+        if (cond) {
+>cond : boolean
+
+            x = "";
+>x = "" : string
+>x : string | undefined
+>"" : string
+        }
+        else {
+            if (x) {
+>x : string | undefined
+
+                x.length;
+>x.length : number
+>x : string
+>length : number
+            }
+            if (x) {
+>x : string | undefined
+
+                x.length;
+>x.length : number
+>x : string
+>length : number
+            }
+        }
+    }
+}
+
diff --git a/tests/cases/conformance/controlFlow/controlFlowIteration.ts b/tests/cases/conformance/controlFlow/controlFlowIteration.ts
@@ -0,0 +1,20 @@
+// @strictNullChecks: true
+
+let cond: boolean;
+
+function ff() {
+    let x: string | undefined;
+    while (true) {
+        if (cond) {
+            x = "";
+        }
+        else {
+            if (x) {
+                x.length;
+            }
+            if (x) {
+                x.length;
+            }
+        }
+    }
+}

Original file line number	Diff line number	Diff line change
`@@ -7592,6 +7592,7 @@ namespace ts {`
`7592`	`7592`	`case FlowKind.Condition:`
`7593`	`7593`	`return getTypeAtFlowCondition(<FlowCondition>flow);`
`7594`	`7594`	`case FlowKind.Label:`
	`7595`	`+ case FlowKind.LoopLabel:`
`7595`	`7596`	`if ((<FlowLabel>flow).antecedents.length === 1) {`
`7596`	`7597`	`flow = (<FlowLabel>flow).antecedents[0];`
`7597`	`7598`	`continue;`
`@@ -7639,7 +7640,8 @@ namespace ts {`
`7639`	`7640`	`}`
`7640`	`7641`
`7641`	`7642`	`function getTypeAtFlowCondition(flow: FlowCondition) {`
`7642`		`- return narrowType(getTypeAtFlowNode(flow.antecedent), flow.expression, flow.assumeTrue);`
	`7643`	`+ const type = getTypeAtFlowNode(flow.antecedent);`
	`7644`	`+ return type && narrowType(type, flow.expression, flow.assumeTrue);`
`7643`	`7645`	`}`
`7644`	`7646`
`7645`	`7647`	`function getTypeAtFlowNodeCached(flow: FlowNode) {`
`@@ -7665,13 +7667,15 @@ namespace ts {`
`7665`	`7667`	`flowStackCount--;`
`7666`	`7668`	`// Record the result only if the cache is still empty. If checkExpressionCached was called`
`7667`	`7669`	`// during processing it is possible we've already recorded a result.`
`7668`		`- return cache[key] \|\| (cache[key] = type);`
	`7670`	`+ return cache[key] \|\| type && (cache[key] = type);`
`7669`	`7671`	`}`
`7670`	`7672`
`7671`	`7673`	`function getTypeAtFlowLabel(flow: FlowLabel) {`
`7672`	`7674`	`const antecedentTypes: Type[] = [];`
`7673`	`7675`	`for (const antecedent of flow.antecedents) {`
`7674`		`- const type = getTypeAtFlowNodeCached(antecedent);`
	`7676`	`+ const type = flow.kind === FlowKind.LoopLabel ?`
	`7677`	`+ getTypeAtFlowNodeCached(antecedent) :`
	`7678`	`+ getTypeAtFlowNode(antecedent);`
`7675`	`7679`	`if (type) {`
`7676`	`7680`	`// If the type at a particular antecedent path is the declared type and the`
`7677`	`7681`	`// reference is known to always be assigned (i.e. when declared and initial types`
`@@ -7685,7 +7689,7 @@ namespace ts {`
`7685`	`7689`	`}`
`7686`	`7690`	`}`
`7687`	`7691`	`}`
`7688`		`- return antecedentTypes.length === 0 ? declaredType :`
	`7692`	`+ return antecedentTypes.length === 0 ? undefined :`
`7689`	`7693`	`antecedentTypes.length === 1 ? antecedentTypes[0] :`
`7690`	`7694`	`getUnionType(antecedentTypes);`
`7691`	`7695`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1519,6 +1519,7 @@ namespace ts {`
`1519`	`1519`	`Unreachable,`
`1520`	`1520`	`Start,`
`1521`	`1521`	`Label,`
	`1522`	`+ LoopLabel,`
`1522`	`1523`	`Assignment,`
`1523`	`1524`	`Condition`
`1524`	`1525`	`}`