Skip to content

Commit f673f48

Browse files
vladimavladima
authored
inject pre-finally and after-finally edges into flow graph to possible ignore pre-finally during flow walk (microsoft#13845)
1 parent ba8330c commit f673f48

7 files changed

Lines changed: 169 additions & 2 deletions

File tree

‎src/compiler/binder.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1045,7 +1045,35 @@ namespace ts {
10451045
if (node.finallyBlock) {
10461046
// in finally flow is combined from pre-try/flow from try/flow from catch
10471047
// pre-flow is necessary to make sure that finally is reachable even if finally flows in both try and finally blocks are unreachable
1048-
addAntecedent(preFinallyLabel, preTryFlow);
1048+
1049+
// also for finally blocks we inject two extra edges into the flow graph.
1050+
// first -> edge that connects pre-try flow with the label at the beginning of the finally block, it has lock associated with it
1051+
// second -> edge that represents post-finally flow.
1052+
// these edges are used in following scenario:
1053+
// let a; (1)
1054+
// try { a = someOperation(); (2)}
1055+
// finally { (3) console.log(a) } (4)
1056+
// (5) a
1057+
1058+
// flow graph for this case looks roughly like this (arrows show ):
1059+
// (1-pre-try-flow) <--.. <-- (2-post-try-flow)
1060+
// ^ ^
1061+
// |*****(3-pre-finally-label) -----|
1062+
// ^
1063+
// |-- ... <-- (4-post-finally-label) <--- (5)
1064+
// In case when we walk the flow starting from inside the finally block we want to take edge '*****' into account
1065+
// since it ensures that finally is always reachable. However when we start outside the finally block and go through label (5)
1066+
// then edge '*****' should be discarded because label 4 is only reachable if post-finally label-4 is reachable
1067+
// Simply speaking code inside finally block is treated as reachable as pre-try-flow
1068+
// since we conservatively assume that any line in try block can throw or return in which case we'll enter finally.
1069+
// However code after finally is reachable only if control flow was not abrupted in try/catch or finally blocks - it should be composed from
1070+
// final flows of these blocks without taking pre-try flow into account.
1071+
//
1072+
// extra edges that we inject allows to control this behavior
1073+
// if when walking the flow we step on post-finally edge - we can mark matching pre-finally edge as locked so it will be skipped.
1074+
const preFinallyFlow: PreFinallyFlow = { flags: FlowFlags.PreFinally, antecedent: preTryFlow, lock: {} };
1075+
addAntecedent(preFinallyLabel, preFinallyFlow);
1076+
10491077
currentFlow = finishFlowLabel(preFinallyLabel);
10501078
bind(node.finallyBlock);
10511079
// if flow after finally is unreachable - keep it
@@ -1061,6 +1089,11 @@ namespace ts {
10611089
: unreachableFlow;
10621090
}
10631091
}
1092+
if (!(currentFlow.flags & FlowFlags.Unreachable)) {
1093+
const afterFinallyFlow: AfterFinallyFlow = { flags: FlowFlags.AfterFinally, antecedent: currentFlow };
1094+
preFinallyFlow.lock = afterFinallyFlow;
1095+
currentFlow = afterFinallyFlow;
1096+
}
10641097
}
10651098
else {
10661099
currentFlow = finishFlowLabel(preFinallyLabel);

‎src/compiler/checker.ts‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9916,7 +9916,19 @@ namespace ts {
99169916
}
99179917
}
99189918
let type: FlowType;
9919-
if (flow.flags & FlowFlags.Assignment) {
9919+
if (flow.flags & FlowFlags.AfterFinally) {
9920+
// block flow edge: finally -> pre-try (for larger explanation check comment in binder.ts - bindTryStatement
9921+
(<AfterFinallyFlow>flow).locked = true;
9922+
type = getTypeAtFlowNode((<AfterFinallyFlow>flow).antecedent);
9923+
(<AfterFinallyFlow>flow).locked = false;
9924+
}
9925+
else if (flow.flags & FlowFlags.PreFinally) {
9926+
// locked pre-finally flows are filtered out in getTypeAtFlowBranchLabel
9927+
// so here just redirect to antecedent
9928+
flow = (<PreFinallyFlow>flow).antecedent;
9929+
continue;
9930+
}
9931+
else if (flow.flags & FlowFlags.Assignment) {
99209932
type = getTypeAtFlowAssignment(<FlowAssignment>flow);
99219933
if (!type) {
99229934
flow = (<FlowAssignment>flow).antecedent;
@@ -10072,6 +10084,12 @@ namespace ts {
1007210084
let subtypeReduction = false;
1007310085
let seenIncomplete = false;
1007410086
for (const antecedent of flow.antecedents) {
10087+
if (antecedent.flags & FlowFlags.PreFinally && (<PreFinallyFlow>antecedent).lock.locked) {
10088+
// if flow correspond to branch from pre-try to finally and this branch is locked - this means that
10089+
// we initially have started following the flow outside the finally block.
10090+
// in this case we should ignore this branch.
10091+
continue;
10092+
}
1007510093
const flowType = getTypeAtFlowNode(antecedent);
1007610094
const type = getTypeFromFlowType(flowType);
1007710095
// If the type at a particular antecedent path is the declared type and the

‎src/compiler/types.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2071,10 +2071,25 @@
20712071
ArrayMutation = 1 << 8, // Potential array mutation
20722072
Referenced = 1 << 9, // Referenced as antecedent once
20732073
Shared = 1 << 10, // Referenced as antecedent more than once
2074+
PreFinally = 1 << 11, // Injected edge that links pre-finally label and pre-try flow
2075+
AfterFinally = 1 << 12, // Injected edge that links post-finally flow with the rest of the graph
20742076
Label = BranchLabel | LoopLabel,
20752077
Condition = TrueCondition | FalseCondition
20762078
}
20772079

2080+
export interface FlowLock {
2081+
locked?: boolean;
2082+
}
2083+
2084+
export interface AfterFinallyFlow extends FlowNode, FlowLock {
2085+
antecedent: FlowNode;
2086+
}
2087+
2088+
export interface PreFinallyFlow extends FlowNode {
2089+
antecedent: FlowNode;
2090+
lock: FlowLock;
2091+
}
2092+
20782093
export interface FlowNode {
20792094
flags: FlowFlags;
20802095
id?: number; // Node id used by flow type cache in checker

‎tests/baselines/reference/flowAfterFinally1.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
//// [flowAfterFinally1.ts]
2+
declare function openFile(): void
3+
declare function closeFile(): void
4+
declare function someOperation(): {}
5+
6+
var result: {}
7+
openFile()
8+
try {
9+
result = someOperation()
10+
} finally {
11+
closeFile()
12+
}
13+
14+
result // should not error here
15+
16+
//// [flowAfterFinally1.js]
17+
var result;
18+
openFile();
19+
try {
20+
result = someOperation();
21+
}
22+
finally {
23+
closeFile();
24+
}
25+
result; // should not error here

‎tests/baselines/reference/flowAfterFinally1.symbols‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
=== tests/cases/compiler/flowAfterFinally1.ts ===
2+
declare function openFile(): void
3+
>openFile : Symbol(openFile, Decl(flowAfterFinally1.ts, 0, 0))
4+
5+
declare function closeFile(): void
6+
>closeFile : Symbol(closeFile, Decl(flowAfterFinally1.ts, 0, 33))
7+
8+
declare function someOperation(): {}
9+
>someOperation : Symbol(someOperation, Decl(flowAfterFinally1.ts, 1, 34))
10+
11+
var result: {}
12+
>result : Symbol(result, Decl(flowAfterFinally1.ts, 4, 3))
13+
14+
openFile()
15+
>openFile : Symbol(openFile, Decl(flowAfterFinally1.ts, 0, 0))
16+
17+
try {
18+
result = someOperation()
19+
>result : Symbol(result, Decl(flowAfterFinally1.ts, 4, 3))
20+
>someOperation : Symbol(someOperation, Decl(flowAfterFinally1.ts, 1, 34))
21+
22+
} finally {
23+
closeFile()
24+
>closeFile : Symbol(closeFile, Decl(flowAfterFinally1.ts, 0, 33))
25+
}
26+
27+
result // should not error here
28+
>result : Symbol(result, Decl(flowAfterFinally1.ts, 4, 3))
29+

‎tests/baselines/reference/flowAfterFinally1.types‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
=== tests/cases/compiler/flowAfterFinally1.ts ===
2+
declare function openFile(): void
3+
>openFile : () => void
4+
5+
declare function closeFile(): void
6+
>closeFile : () => void
7+
8+
declare function someOperation(): {}
9+
>someOperation : () => {}
10+
11+
var result: {}
12+
>result : {}
13+
14+
openFile()
15+
>openFile() : void
16+
>openFile : () => void
17+
18+
try {
19+
result = someOperation()
20+
>result = someOperation() : {}
21+
>result : {}
22+
>someOperation() : {}
23+
>someOperation : () => {}
24+
25+
} finally {
26+
closeFile()
27+
>closeFile() : void
28+
>closeFile : () => void
29+
}
30+
31+
result // should not error here
32+
>result : {}
33+

‎tests/cases/compiler/flowAfterFinally1.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// @strictNullChecks: true
2+
declare function openFile(): void
3+
declare function closeFile(): void
4+
declare function someOperation(): {}
5+
6+
var result: {}
7+
openFile()
8+
try {
9+
result = someOperation()
10+
} finally {
11+
closeFile()
12+
}
13+
14+
result // should not error here

0 commit comments

Comments
 (0)