Skip to content

Commit 9ed73eb

Browse files
ahejlsbergahejlsberg
authored
Properly handle control flows from returns in try/catch within IIFE (microsoft#36901)
* Properly handle control flows from returns in try/catch within IIFE * Accept new baselines * Add tests * Accept new baselines * When end of finally is unreachable, end of try statement is too * Add additional test case
1 parent e89df5c commit 9ed73eb

File tree

11 files changed

+1202
-106
lines changed

11 files changed

+1202
-106
lines changed

‎src/compiler/binder.ts‎

Lines changed: 44 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,10 @@ namespace ts {
952952
return initFlowNode({ flags: FlowFlags.LoopLabel, antecedents: undefined });
953953
}
954954

955+
function createReduceLabel(target: FlowLabel, antecedents: FlowNode[], antecedent: FlowNode): FlowReduceLabel {
956+
return initFlowNode({ flags: FlowFlags.ReduceLabel, target, antecedents, antecedent });
957+
}
958+
955959
function setFlowNodeReferenced(flow: FlowNode) {
956960
// On first reference we set the Referenced flag, thereafter we set the Shared flag
957961
flow.flags |= flow.flags & FlowFlags.Referenced ? FlowFlags.Shared : FlowFlags.Referenced;
@@ -1209,35 +1213,36 @@ namespace ts {
12091213
}
12101214

12111215
function bindTryStatement(node: TryStatement): void {
1212-
const preFinallyLabel = createBranchLabel();
12131216
// We conservatively assume that *any* code in the try block can cause an exception, but we only need
12141217
// to track code that causes mutations (because only mutations widen the possible control flow type of
1215-
// a variable). The currentExceptionTarget is the target label for control flows that result from
1216-
// exceptions. We add all mutation flow nodes as antecedents of this label such that we can analyze them
1217-
// as possible antecedents of the start of catch or finally blocks. Furthermore, we add the current
1218-
// control flow to represent exceptions that occur before any mutations.
1218+
// a variable). The exceptionLabel is the target label for control flows that result from exceptions.
1219+
// We add all mutation flow nodes as antecedents of this label such that we can analyze them as possible
1220+
// antecedents of the start of catch or finally blocks. Furthermore, we add the current control flow to
1221+
// represent exceptions that occur before any mutations.
12191222
const saveReturnTarget = currentReturnTarget;
12201223
const saveExceptionTarget = currentExceptionTarget;
1221-
currentReturnTarget = createBranchLabel();
1222-
currentExceptionTarget = node.catchClause ? createBranchLabel() : currentReturnTarget;
1223-
addAntecedent(currentExceptionTarget, currentFlow);
1224+
const normalExitLabel = createBranchLabel();
1225+
const returnLabel = createBranchLabel();
1226+
let exceptionLabel = createBranchLabel();
1227+
if (node.finallyBlock) {
1228+
currentReturnTarget = returnLabel;
1229+
}
1230+
addAntecedent(exceptionLabel, currentFlow);
1231+
currentExceptionTarget = exceptionLabel;
12241232
bind(node.tryBlock);
1225-
addAntecedent(preFinallyLabel, currentFlow);
1226-
const flowAfterTry = currentFlow;
1227-
let flowAfterCatch = unreachableFlow;
1233+
addAntecedent(normalExitLabel, currentFlow);
12281234
if (node.catchClause) {
12291235
// Start of catch clause is the target of exceptions from try block.
1230-
currentFlow = finishFlowLabel(currentExceptionTarget);
1236+
currentFlow = finishFlowLabel(exceptionLabel);
12311237
// The currentExceptionTarget now represents control flows from exceptions in the catch clause.
12321238
// Effectively, in a try-catch-finally, if an exception occurs in the try block, the catch block
12331239
// acts like a second try block.
1234-
currentExceptionTarget = currentReturnTarget;
1235-
addAntecedent(currentExceptionTarget, currentFlow);
1240+
exceptionLabel = createBranchLabel();
1241+
addAntecedent(exceptionLabel, currentFlow);
1242+
currentExceptionTarget = exceptionLabel;
12361243
bind(node.catchClause);
1237-
addAntecedent(preFinallyLabel, currentFlow);
1238-
flowAfterCatch = currentFlow;
1244+
addAntecedent(normalExitLabel, currentFlow);
12391245
}
1240-
const exceptionTarget = finishFlowLabel(currentExceptionTarget);
12411246
currentReturnTarget = saveReturnTarget;
12421247
currentExceptionTarget = saveExceptionTarget;
12431248
if (node.finallyBlock) {
@@ -1250,35 +1255,33 @@ namespace ts {
12501255
// When analyzing a control flow graph that starts inside a finally block we want to consider all
12511256
// five possibilities above. However, when analyzing a control flow graph that starts outside (past)
12521257
// the finally block, we only want to consider the first two (if we're past a finally block then it
1253-
// must have completed normally). To make this possible, we inject two extra nodes into the control
1254-
// flow graph: An after-finally with an antecedent of the control flow at the end of the finally
1255-
// block, and a pre-finally with an antecedent that represents all exceptional control flows. The
1256-
// 'lock' property of the pre-finally references the after-finally, and the after-finally has a
1257-
// boolean 'locked' property that we set to true when analyzing a control flow that contained the
1258-
// the after-finally node. When the lock associated with a pre-finally is locked, the antecedent of
1259-
// the pre-finally (i.e. the exceptional control flows) are skipped.
1260-
const preFinallyFlow: PreFinallyFlow = initFlowNode({ flags: FlowFlags.PreFinally, antecedent: exceptionTarget, lock: {} });
1261-
addAntecedent(preFinallyLabel, preFinallyFlow);
1262-
currentFlow = finishFlowLabel(preFinallyLabel);
1258+
// must have completed normally). Likewise, when analyzing a control flow graph from return statements
1259+
// in try or catch blocks in an IIFE, we only want to consider the third. To make this possible, we
1260+
// inject a ReduceLabel node into the control flow graph. This node contains an alternate reduced
1261+
// set of antecedents for the pre-finally label. As control flow analysis passes by a ReduceLabel
1262+
// node, the pre-finally label is temporarily switched to the reduced antecedent set.
1263+
const finallyLabel = createBranchLabel();
1264+
finallyLabel.antecedents = concatenate(concatenate(normalExitLabel.antecedents, exceptionLabel.antecedents), returnLabel.antecedents);
1265+
currentFlow = finallyLabel;
12631266
bind(node.finallyBlock);
1264-
// If the end of the finally block is reachable, but the end of the try and catch blocks are not,
1265-
// convert the current flow to unreachable. For example, 'try { return 1; } finally { ... }' should
1266-
// result in an unreachable current control flow.
1267-
if (!(currentFlow.flags & FlowFlags.Unreachable)) {
1268-
if ((flowAfterTry.flags & FlowFlags.Unreachable) && (flowAfterCatch.flags & FlowFlags.Unreachable)) {
1269-
currentFlow = flowAfterTry === reportedUnreachableFlow || flowAfterCatch === reportedUnreachableFlow
1270-
? reportedUnreachableFlow
1271-
: unreachableFlow;
1272-
}
1267+
if (currentFlow.flags & FlowFlags.Unreachable) {
1268+
// If the end of the finally block is unreachable, the end of the entire try statement is unreachable.
1269+
currentFlow = unreachableFlow;
12731270
}
1274-
if (!(currentFlow.flags & FlowFlags.Unreachable)) {
1275-
const afterFinallyFlow: AfterFinallyFlow = initFlowNode({ flags: FlowFlags.AfterFinally, antecedent: currentFlow });
1276-
preFinallyFlow.lock = afterFinallyFlow;
1277-
currentFlow = afterFinallyFlow;
1271+
else {
1272+
// If we have an IIFE return target and return statements in the try or catch blocks, add a control
1273+
// flow that goes back through the finally block and back through only the return statements.
1274+
if (currentReturnTarget && returnLabel.antecedents) {
1275+
addAntecedent(currentReturnTarget, createReduceLabel(finallyLabel, returnLabel.antecedents, currentFlow));
1276+
}
1277+
// If the end of the finally block is reachable, but the end of the try and catch blocks are not,
1278+
// convert the current flow to unreachable. For example, 'try { return 1; } finally { ... }' should
1279+
// result in an unreachable current control flow.
1280+
currentFlow = normalExitLabel.antecedents ? createReduceLabel(finallyLabel, normalExitLabel.antecedents, currentFlow) : unreachableFlow;
12781281
}
12791282
}
12801283
else {
1281-
currentFlow = finishFlowLabel(preFinallyLabel);
1284+
currentFlow = finishFlowLabel(normalExitLabel);
12821285
}
12831286
}
12841287

‎src/compiler/checker.ts‎

Lines changed: 19 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19437,16 +19437,12 @@ namespace ts {
1943719437
}
1943819438

1943919439
function isReachableFlowNode(flow: FlowNode) {
19440-
const result = isReachableFlowNodeWorker(flow, /*skipCacheCheck*/ false);
19440+
const result = isReachableFlowNodeWorker(flow, /*noCacheCheck*/ false);
1944119441
lastFlowNode = flow;
1944219442
lastFlowNodeReachable = result;
1944319443
return result;
1944419444
}
1944519445

19446-
function isUnlockedReachableFlowNode(flow: FlowNode) {
19447-
return !(flow.flags & FlowFlags.PreFinally && (<PreFinallyFlow>flow).lock.locked) && isReachableFlowNodeWorker(flow, /*skipCacheCheck*/ false);
19448-
}
19449-
1945019446
function isFalseExpression(expr: Expression): boolean {
1945119447
const node = skipParentheses(expr);
1945219448
return node.kind === SyntaxKind.FalseKeyword || node.kind === SyntaxKind.BinaryExpression && (
@@ -19464,11 +19460,11 @@ namespace ts {
1946419460
if (!noCacheCheck) {
1946519461
const id = getFlowNodeId(flow);
1946619462
const reachable = flowNodeReachable[id];
19467-
return reachable !== undefined ? reachable : (flowNodeReachable[id] = isReachableFlowNodeWorker(flow, /*skipCacheCheck*/ true));
19463+
return reachable !== undefined ? reachable : (flowNodeReachable[id] = isReachableFlowNodeWorker(flow, /*noCacheCheck*/ true));
1946819464
}
1946919465
noCacheCheck = false;
1947019466
}
19471-
if (flags & (FlowFlags.Assignment | FlowFlags.Condition | FlowFlags.ArrayMutation | FlowFlags.PreFinally)) {
19467+
if (flags & (FlowFlags.Assignment | FlowFlags.Condition | FlowFlags.ArrayMutation)) {
1947219468
flow = (<FlowAssignment | FlowCondition | FlowArrayMutation | PreFinallyFlow>flow).antecedent;
1947319469
}
1947419470
else if (flags & FlowFlags.Call) {
@@ -19489,7 +19485,7 @@ namespace ts {
1948919485
}
1949019486
else if (flags & FlowFlags.BranchLabel) {
1949119487
// A branching point is reachable if any branch is reachable.
19492-
return some((<FlowLabel>flow).antecedents, isUnlockedReachableFlowNode);
19488+
return some((<FlowLabel>flow).antecedents, f => isReachableFlowNodeWorker(f, /*noCacheCheck*/ false));
1949319489
}
1949419490
else if (flags & FlowFlags.LoopLabel) {
1949519491
// A loop is reachable if the control flow path that leads to the top is reachable.
@@ -19503,12 +19499,14 @@ namespace ts {
1950319499
}
1950419500
flow = (<FlowSwitchClause>flow).antecedent;
1950519501
}
19506-
else if (flags & FlowFlags.AfterFinally) {
19507-
// Cache is unreliable once we start locking nodes
19502+
else if (flags & FlowFlags.ReduceLabel) {
19503+
// Cache is unreliable once we start adjusting labels
1950819504
lastFlowNode = undefined;
19509-
(<AfterFinallyFlow>flow).locked = true;
19510-
const result = isReachableFlowNodeWorker((<AfterFinallyFlow>flow).antecedent, /*skipCacheCheck*/ false);
19511-
(<AfterFinallyFlow>flow).locked = false;
19505+
const target = (<FlowReduceLabel>flow).target;
19506+
const saveAntecedents = target.antecedents;
19507+
target.antecedents = (<FlowReduceLabel>flow).antecedents;
19508+
const result = isReachableFlowNodeWorker((<FlowReduceLabel>flow).antecedent, /*noCacheCheck*/ false);
19509+
target.antecedents = saveAntecedents;
1951219510
return result;
1951319511
}
1951419512
else {
@@ -19572,19 +19570,7 @@ namespace ts {
1957219570
}
1957319571
}
1957419572
let type: FlowType | undefined;
19575-
if (flags & FlowFlags.AfterFinally) {
19576-
// block flow edge: finally -> pre-try (for larger explanation check comment in binder.ts - bindTryStatement
19577-
(<AfterFinallyFlow>flow).locked = true;
19578-
type = getTypeAtFlowNode((<AfterFinallyFlow>flow).antecedent);
19579-
(<AfterFinallyFlow>flow).locked = false;
19580-
}
19581-
else if (flags & FlowFlags.PreFinally) {
19582-
// locked pre-finally flows are filtered out in getTypeAtFlowBranchLabel
19583-
// so here just redirect to antecedent
19584-
flow = (<PreFinallyFlow>flow).antecedent;
19585-
continue;
19586-
}
19587-
else if (flags & FlowFlags.Assignment) {
19573+
if (flags & FlowFlags.Assignment) {
1958819574
type = getTypeAtFlowAssignment(<FlowAssignment>flow);
1958919575
if (!type) {
1959019576
flow = (<FlowAssignment>flow).antecedent;
@@ -19620,6 +19606,13 @@ namespace ts {
1962019606
continue;
1962119607
}
1962219608
}
19609+
else if (flags & FlowFlags.ReduceLabel) {
19610+
const target = (<FlowReduceLabel>flow).target;
19611+
const saveAntecedents = target.antecedents;
19612+
target.antecedents = (<FlowReduceLabel>flow).antecedents;
19613+
type = getTypeAtFlowNode((<FlowReduceLabel>flow).antecedent);
19614+
target.antecedents = saveAntecedents;
19615+
}
1962319616
else if (flags & FlowFlags.Start) {
1962419617
// Check if we should continue with the control flow of the containing function.
1962519618
const container = (<FlowStart>flow).node;
@@ -19831,12 +19824,6 @@ namespace ts {
1983119824
let seenIncomplete = false;
1983219825
let bypassFlow: FlowSwitchClause | undefined;
1983319826
for (const antecedent of flow.antecedents!) {
19834-
if (antecedent.flags & FlowFlags.PreFinally && (<PreFinallyFlow>antecedent).lock.locked) {
19835-
// if flow correspond to branch from pre-try to finally and this branch is locked - this means that
19836-
// we initially have started following the flow outside the finally block.
19837-
// in this case we should ignore this branch.
19838-
continue;
19839-
}
1984019827
if (!bypassFlow && antecedent.flags & FlowFlags.SwitchClause && (<FlowSwitchClause>antecedent).clauseStart === (<FlowSwitchClause>antecedent).clauseEnd) {
1984119828
// The antecedent is the bypass branch of a potentially exhaustive switch statement.
1984219829
bypassFlow = <FlowSwitchClause>antecedent;

‎src/compiler/types.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2770,10 +2770,9 @@ namespace ts {
27702770
SwitchClause = 1 << 7, // Switch statement clause
27712771
ArrayMutation = 1 << 8, // Potential array mutation
27722772
Call = 1 << 9, // Potential assertion call
2773-
Referenced = 1 << 10, // Referenced as antecedent once
2774-
Shared = 1 << 11, // Referenced as antecedent more than once
2775-
PreFinally = 1 << 12, // Injected edge that links pre-finally label and pre-try flow
2776-
AfterFinally = 1 << 13, // Injected edge that links post-finally flow with the rest of the graph
2773+
ReduceLabel = 1 << 10, // Temporarily reduce antecedents of label
2774+
Referenced = 1 << 11, // Referenced as antecedent once
2775+
Shared = 1 << 12, // Referenced as antecedent more than once
27772776

27782777
Label = BranchLabel | LoopLabel,
27792778
Condition = TrueCondition | FalseCondition,
@@ -2853,6 +2852,12 @@ namespace ts {
28532852
antecedent: FlowNode;
28542853
}
28552854

2855+
export interface FlowReduceLabel extends FlowNodeBase {
2856+
target: FlowLabel;
2857+
antecedents: FlowNode[];
2858+
antecedent: FlowNode;
2859+
}
2860+
28562861
export type FlowType = Type | IncompleteType;
28572862

28582863
// Incomplete types occur during control flow analysis of loops. An IncompleteType

‎src/debug/dbg.ts‎

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,9 @@ namespace Debug {
4646
readonly SwitchClause: number,
4747
readonly ArrayMutation: number,
4848
readonly Call: number,
49+
readonly ReduceLabel: number,
4950
readonly Referenced: number,
5051
readonly Shared: number,
51-
readonly PreFinally: number,
52-
readonly AfterFinally: number,
5352
readonly Label: number,
5453
readonly Condition: number,
5554
};
@@ -69,6 +68,7 @@ namespace Debug {
6968
| FlowCondition
7069
| FlowSwitchClause
7170
| FlowArrayMutation
71+
| FlowReduceLabel
7272
;
7373

7474
interface FlowNodeBase {
@@ -119,6 +119,12 @@ namespace Debug {
119119
antecedent: FlowNode;
120120
}
121121

122+
export interface FlowReduceLabel extends FlowNodeBase {
123+
target: FlowLabel;
124+
antecedents: FlowNode[];
125+
antecedent: FlowNode;
126+
}
127+
122128
type FlowFlags = number;
123129
let FlowFlags: TypeScriptModule["FlowFlags"];
124130
let getSourceFileOfNode: TypeScriptModule["getSourceFileOfNode"];
@@ -199,8 +205,7 @@ namespace Debug {
199205
FlowFlags.SwitchClause |
200206
FlowFlags.ArrayMutation |
201207
FlowFlags.Call |
202-
FlowFlags.PreFinally |
203-
FlowFlags.AfterFinally;
208+
FlowFlags.ReduceLabel;
204209

205210
const hasNodeFlags =
206211
FlowFlags.Start |
@@ -264,17 +269,14 @@ namespace Debug {
264269
if (!graphNode) {
265270
links[id] = graphNode = { id, flowNode, edges: [], text: renderFlowNode(flowNode), lane: -1, endLane: -1, level: -1 };
266271
nodes.push(graphNode);
267-
if (!(flowNode.flags & FlowFlags.PreFinally)) {
268-
if (hasAntecedents(flowNode)) {
269-
270-
for (const antecedent of flowNode.antecedents) {
271-
buildGraphEdge(graphNode, antecedent);
272-
}
273-
}
274-
else if (hasAntecedent(flowNode)) {
275-
buildGraphEdge(graphNode, flowNode.antecedent);
272+
if (hasAntecedents(flowNode)) {
273+
for (const antecedent of flowNode.antecedents) {
274+
buildGraphEdge(graphNode, antecedent);
276275
}
277276
}
277+
else if (hasAntecedent(flowNode)) {
278+
buildGraphEdge(graphNode, flowNode.antecedent);
279+
}
278280
}
279281
return graphNode;
280282
}
@@ -341,8 +343,7 @@ namespace Debug {
341343
if (flags & FlowFlags.SwitchClause) return "SwitchClause";
342344
if (flags & FlowFlags.ArrayMutation) return "ArrayMutation";
343345
if (flags & FlowFlags.Call) return "Call";
344-
if (flags & FlowFlags.PreFinally) return "PreFinally";
345-
if (flags & FlowFlags.AfterFinally) return "AfterFinally";
346+
if (flags & FlowFlags.ReduceLabel) return "ReduceLabel";
346347
if (flags & FlowFlags.Unreachable) return "Unreachable";
347348
throw new Error();
348349
}

0 commit comments

Comments
 (0)