Properly handle control flows from returns in try/catch within IIFE (#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
This commit is contained in:
Anders Hejlsberg
2020-02-25 16:14:00 -08:00
committed by GitHub
parent e89df5ce6f
commit 9ed73ebbbf
11 changed files with 1202 additions and 106 deletions
+44 -41
View File
@@ -952,6 +952,10 @@ namespace ts {
return initFlowNode({ flags: FlowFlags.LoopLabel, antecedents: undefined });
}
function createReduceLabel(target: FlowLabel, antecedents: FlowNode[], antecedent: FlowNode): FlowReduceLabel {
return initFlowNode({ flags: FlowFlags.ReduceLabel, target, antecedents, antecedent });
}
function setFlowNodeReferenced(flow: FlowNode) {
// On first reference we set the Referenced flag, thereafter we set the Shared flag
flow.flags |= flow.flags & FlowFlags.Referenced ? FlowFlags.Shared : FlowFlags.Referenced;
@@ -1209,35 +1213,36 @@ namespace ts {
}
function bindTryStatement(node: TryStatement): void {
const preFinallyLabel = createBranchLabel();
// We conservatively assume that *any* code in the try block can cause an exception, but we only need
// to track code that causes mutations (because only mutations widen the possible control flow type of
// a variable). The currentExceptionTarget is the target label for control flows that result from
// exceptions. We add all mutation flow nodes as antecedents of this label such that we can analyze them
// as possible antecedents of the start of catch or finally blocks. Furthermore, we add the current
// control flow to represent exceptions that occur before any mutations.
// a variable). The exceptionLabel is the target label for control flows that result from exceptions.
// We add all mutation flow nodes as antecedents of this label such that we can analyze them as possible
// antecedents of the start of catch or finally blocks. Furthermore, we add the current control flow to
// represent exceptions that occur before any mutations.
const saveReturnTarget = currentReturnTarget;
const saveExceptionTarget = currentExceptionTarget;
currentReturnTarget = createBranchLabel();
currentExceptionTarget = node.catchClause ? createBranchLabel() : currentReturnTarget;
addAntecedent(currentExceptionTarget, currentFlow);
const normalExitLabel = createBranchLabel();
const returnLabel = createBranchLabel();
let exceptionLabel = createBranchLabel();
if (node.finallyBlock) {
currentReturnTarget = returnLabel;
}
addAntecedent(exceptionLabel, currentFlow);
currentExceptionTarget = exceptionLabel;
bind(node.tryBlock);
addAntecedent(preFinallyLabel, currentFlow);
const flowAfterTry = currentFlow;
let flowAfterCatch = unreachableFlow;
addAntecedent(normalExitLabel, currentFlow);
if (node.catchClause) {
// Start of catch clause is the target of exceptions from try block.
currentFlow = finishFlowLabel(currentExceptionTarget);
currentFlow = finishFlowLabel(exceptionLabel);
// The currentExceptionTarget now represents control flows from exceptions in the catch clause.
// Effectively, in a try-catch-finally, if an exception occurs in the try block, the catch block
// acts like a second try block.
currentExceptionTarget = currentReturnTarget;
addAntecedent(currentExceptionTarget, currentFlow);
exceptionLabel = createBranchLabel();
addAntecedent(exceptionLabel, currentFlow);
currentExceptionTarget = exceptionLabel;
bind(node.catchClause);
addAntecedent(preFinallyLabel, currentFlow);
flowAfterCatch = currentFlow;
addAntecedent(normalExitLabel, currentFlow);
}
const exceptionTarget = finishFlowLabel(currentExceptionTarget);
currentReturnTarget = saveReturnTarget;
currentExceptionTarget = saveExceptionTarget;
if (node.finallyBlock) {
@@ -1250,35 +1255,33 @@ namespace ts {
// When analyzing a control flow graph that starts inside a finally block we want to consider all
// five possibilities above. However, when analyzing a control flow graph that starts outside (past)
// the finally block, we only want to consider the first two (if we're past a finally block then it
// must have completed normally). To make this possible, we inject two extra nodes into the control
// flow graph: An after-finally with an antecedent of the control flow at the end of the finally
// block, and a pre-finally with an antecedent that represents all exceptional control flows. The
// 'lock' property of the pre-finally references the after-finally, and the after-finally has a
// boolean 'locked' property that we set to true when analyzing a control flow that contained the
// the after-finally node. When the lock associated with a pre-finally is locked, the antecedent of
// the pre-finally (i.e. the exceptional control flows) are skipped.
const preFinallyFlow: PreFinallyFlow = initFlowNode({ flags: FlowFlags.PreFinally, antecedent: exceptionTarget, lock: {} });
addAntecedent(preFinallyLabel, preFinallyFlow);
currentFlow = finishFlowLabel(preFinallyLabel);
// must have completed normally). Likewise, when analyzing a control flow graph from return statements
// in try or catch blocks in an IIFE, we only want to consider the third. To make this possible, we
// inject a ReduceLabel node into the control flow graph. This node contains an alternate reduced
// set of antecedents for the pre-finally label. As control flow analysis passes by a ReduceLabel
// node, the pre-finally label is temporarily switched to the reduced antecedent set.
const finallyLabel = createBranchLabel();
finallyLabel.antecedents = concatenate(concatenate(normalExitLabel.antecedents, exceptionLabel.antecedents), returnLabel.antecedents);
currentFlow = finallyLabel;
bind(node.finallyBlock);
// If the end of the finally block is reachable, but the end of the try and catch blocks are not,
// convert the current flow to unreachable. For example, 'try { return 1; } finally { ... }' should
// result in an unreachable current control flow.
if (!(currentFlow.flags & FlowFlags.Unreachable)) {
if ((flowAfterTry.flags & FlowFlags.Unreachable) && (flowAfterCatch.flags & FlowFlags.Unreachable)) {
currentFlow = flowAfterTry === reportedUnreachableFlow || flowAfterCatch === reportedUnreachableFlow
? reportedUnreachableFlow
: unreachableFlow;
}
if (currentFlow.flags & FlowFlags.Unreachable) {
// If the end of the finally block is unreachable, the end of the entire try statement is unreachable.
currentFlow = unreachableFlow;
}
if (!(currentFlow.flags & FlowFlags.Unreachable)) {
const afterFinallyFlow: AfterFinallyFlow = initFlowNode({ flags: FlowFlags.AfterFinally, antecedent: currentFlow });
preFinallyFlow.lock = afterFinallyFlow;
currentFlow = afterFinallyFlow;
else {
// If we have an IIFE return target and return statements in the try or catch blocks, add a control
// flow that goes back through the finally block and back through only the return statements.
if (currentReturnTarget && returnLabel.antecedents) {
addAntecedent(currentReturnTarget, createReduceLabel(finallyLabel, returnLabel.antecedents, currentFlow));
}
// If the end of the finally block is reachable, but the end of the try and catch blocks are not,
// convert the current flow to unreachable. For example, 'try { return 1; } finally { ... }' should
// result in an unreachable current control flow.
currentFlow = normalExitLabel.antecedents ? createReduceLabel(finallyLabel, normalExitLabel.antecedents, currentFlow) : unreachableFlow;
}
}
else {
currentFlow = finishFlowLabel(preFinallyLabel);
currentFlow = finishFlowLabel(normalExitLabel);
}
}
+19 -32
View File
@@ -19437,16 +19437,12 @@ namespace ts {
}
function isReachableFlowNode(flow: FlowNode) {
const result = isReachableFlowNodeWorker(flow, /*skipCacheCheck*/ false);
const result = isReachableFlowNodeWorker(flow, /*noCacheCheck*/ false);
lastFlowNode = flow;
lastFlowNodeReachable = result;
return result;
}
function isUnlockedReachableFlowNode(flow: FlowNode) {
return !(flow.flags & FlowFlags.PreFinally && (<PreFinallyFlow>flow).lock.locked) && isReachableFlowNodeWorker(flow, /*skipCacheCheck*/ false);
}
function isFalseExpression(expr: Expression): boolean {
const node = skipParentheses(expr);
return node.kind === SyntaxKind.FalseKeyword || node.kind === SyntaxKind.BinaryExpression && (
@@ -19464,11 +19460,11 @@ namespace ts {
if (!noCacheCheck) {
const id = getFlowNodeId(flow);
const reachable = flowNodeReachable[id];
return reachable !== undefined ? reachable : (flowNodeReachable[id] = isReachableFlowNodeWorker(flow, /*skipCacheCheck*/ true));
return reachable !== undefined ? reachable : (flowNodeReachable[id] = isReachableFlowNodeWorker(flow, /*noCacheCheck*/ true));
}
noCacheCheck = false;
}
if (flags & (FlowFlags.Assignment | FlowFlags.Condition | FlowFlags.ArrayMutation | FlowFlags.PreFinally)) {
if (flags & (FlowFlags.Assignment | FlowFlags.Condition | FlowFlags.ArrayMutation)) {
flow = (<FlowAssignment | FlowCondition | FlowArrayMutation | PreFinallyFlow>flow).antecedent;
}
else if (flags & FlowFlags.Call) {
@@ -19489,7 +19485,7 @@ namespace ts {
}
else if (flags & FlowFlags.BranchLabel) {
// A branching point is reachable if any branch is reachable.
return some((<FlowLabel>flow).antecedents, isUnlockedReachableFlowNode);
return some((<FlowLabel>flow).antecedents, f => isReachableFlowNodeWorker(f, /*noCacheCheck*/ false));
}
else if (flags & FlowFlags.LoopLabel) {
// A loop is reachable if the control flow path that leads to the top is reachable.
@@ -19503,12 +19499,14 @@ namespace ts {
}
flow = (<FlowSwitchClause>flow).antecedent;
}
else if (flags & FlowFlags.AfterFinally) {
// Cache is unreliable once we start locking nodes
else if (flags & FlowFlags.ReduceLabel) {
// Cache is unreliable once we start adjusting labels
lastFlowNode = undefined;
(<AfterFinallyFlow>flow).locked = true;
const result = isReachableFlowNodeWorker((<AfterFinallyFlow>flow).antecedent, /*skipCacheCheck*/ false);
(<AfterFinallyFlow>flow).locked = false;
const target = (<FlowReduceLabel>flow).target;
const saveAntecedents = target.antecedents;
target.antecedents = (<FlowReduceLabel>flow).antecedents;
const result = isReachableFlowNodeWorker((<FlowReduceLabel>flow).antecedent, /*noCacheCheck*/ false);
target.antecedents = saveAntecedents;
return result;
}
else {
@@ -19572,19 +19570,7 @@ namespace ts {
}
}
let type: FlowType | undefined;
if (flags & FlowFlags.AfterFinally) {
// block flow edge: finally -> pre-try (for larger explanation check comment in binder.ts - bindTryStatement
(<AfterFinallyFlow>flow).locked = true;
type = getTypeAtFlowNode((<AfterFinallyFlow>flow).antecedent);
(<AfterFinallyFlow>flow).locked = false;
}
else if (flags & FlowFlags.PreFinally) {
// locked pre-finally flows are filtered out in getTypeAtFlowBranchLabel
// so here just redirect to antecedent
flow = (<PreFinallyFlow>flow).antecedent;
continue;
}
else if (flags & FlowFlags.Assignment) {
if (flags & FlowFlags.Assignment) {
type = getTypeAtFlowAssignment(<FlowAssignment>flow);
if (!type) {
flow = (<FlowAssignment>flow).antecedent;
@@ -19620,6 +19606,13 @@ namespace ts {
continue;
}
}
else if (flags & FlowFlags.ReduceLabel) {
const target = (<FlowReduceLabel>flow).target;
const saveAntecedents = target.antecedents;
target.antecedents = (<FlowReduceLabel>flow).antecedents;
type = getTypeAtFlowNode((<FlowReduceLabel>flow).antecedent);
target.antecedents = saveAntecedents;
}
else if (flags & FlowFlags.Start) {
// Check if we should continue with the control flow of the containing function.
const container = (<FlowStart>flow).node;
@@ -19831,12 +19824,6 @@ namespace ts {
let seenIncomplete = false;
let bypassFlow: FlowSwitchClause | undefined;
for (const antecedent of flow.antecedents!) {
if (antecedent.flags & FlowFlags.PreFinally && (<PreFinallyFlow>antecedent).lock.locked) {
// if flow correspond to branch from pre-try to finally and this branch is locked - this means that
// we initially have started following the flow outside the finally block.
// in this case we should ignore this branch.
continue;
}
if (!bypassFlow && antecedent.flags & FlowFlags.SwitchClause && (<FlowSwitchClause>antecedent).clauseStart === (<FlowSwitchClause>antecedent).clauseEnd) {
// The antecedent is the bypass branch of a potentially exhaustive switch statement.
bypassFlow = <FlowSwitchClause>antecedent;
+9 -4
View File
@@ -2770,10 +2770,9 @@ namespace ts {
SwitchClause = 1 << 7, // Switch statement clause
ArrayMutation = 1 << 8, // Potential array mutation
Call = 1 << 9, // Potential assertion call
Referenced = 1 << 10, // Referenced as antecedent once
Shared = 1 << 11, // Referenced as antecedent more than once
PreFinally = 1 << 12, // Injected edge that links pre-finally label and pre-try flow
AfterFinally = 1 << 13, // Injected edge that links post-finally flow with the rest of the graph
ReduceLabel = 1 << 10, // Temporarily reduce antecedents of label
Referenced = 1 << 11, // Referenced as antecedent once
Shared = 1 << 12, // Referenced as antecedent more than once
Label = BranchLabel | LoopLabel,
Condition = TrueCondition | FalseCondition,
@@ -2853,6 +2852,12 @@ namespace ts {
antecedent: FlowNode;
}
export interface FlowReduceLabel extends FlowNodeBase {
target: FlowLabel;
antecedents: FlowNode[];
antecedent: FlowNode;
}
export type FlowType = Type | IncompleteType;
// Incomplete types occur during control flow analysis of loops. An IncompleteType
+16 -15
View File
@@ -46,10 +46,9 @@ namespace Debug {
readonly SwitchClause: number,
readonly ArrayMutation: number,
readonly Call: number,
readonly ReduceLabel: number,
readonly Referenced: number,
readonly Shared: number,
readonly PreFinally: number,
readonly AfterFinally: number,
readonly Label: number,
readonly Condition: number,
};
@@ -69,6 +68,7 @@ namespace Debug {
| FlowCondition
| FlowSwitchClause
| FlowArrayMutation
| FlowReduceLabel
;
interface FlowNodeBase {
@@ -119,6 +119,12 @@ namespace Debug {
antecedent: FlowNode;
}
export interface FlowReduceLabel extends FlowNodeBase {
target: FlowLabel;
antecedents: FlowNode[];
antecedent: FlowNode;
}
type FlowFlags = number;
let FlowFlags: TypeScriptModule["FlowFlags"];
let getSourceFileOfNode: TypeScriptModule["getSourceFileOfNode"];
@@ -199,8 +205,7 @@ namespace Debug {
FlowFlags.SwitchClause |
FlowFlags.ArrayMutation |
FlowFlags.Call |
FlowFlags.PreFinally |
FlowFlags.AfterFinally;
FlowFlags.ReduceLabel;
const hasNodeFlags =
FlowFlags.Start |
@@ -264,17 +269,14 @@ namespace Debug {
if (!graphNode) {
links[id] = graphNode = { id, flowNode, edges: [], text: renderFlowNode(flowNode), lane: -1, endLane: -1, level: -1 };
nodes.push(graphNode);
if (!(flowNode.flags & FlowFlags.PreFinally)) {
if (hasAntecedents(flowNode)) {
for (const antecedent of flowNode.antecedents) {
buildGraphEdge(graphNode, antecedent);
}
}
else if (hasAntecedent(flowNode)) {
buildGraphEdge(graphNode, flowNode.antecedent);
if (hasAntecedents(flowNode)) {
for (const antecedent of flowNode.antecedents) {
buildGraphEdge(graphNode, antecedent);
}
}
else if (hasAntecedent(flowNode)) {
buildGraphEdge(graphNode, flowNode.antecedent);
}
}
return graphNode;
}
@@ -341,8 +343,7 @@ namespace Debug {
if (flags & FlowFlags.SwitchClause) return "SwitchClause";
if (flags & FlowFlags.ArrayMutation) return "ArrayMutation";
if (flags & FlowFlags.Call) return "Call";
if (flags & FlowFlags.PreFinally) return "PreFinally";
if (flags & FlowFlags.AfterFinally) return "AfterFinally";
if (flags & FlowFlags.ReduceLabel) return "ReduceLabel";
if (flags & FlowFlags.Unreachable) return "Unreachable";
throw new Error();
}