Skip to content

Commit

Permalink
Include all flow nodes made within try blocks as antecedents for `c…
Browse files Browse the repository at this point in the history
…atch` or `finally` blocks (#29466)

* Include all flow nodes made within `try` blocks as antecedents for `catch` or `finally` blocks

* Fix typo
  • Loading branch information
weswigham authored Jan 18, 2019
1 parent b317334 commit 20285e6
Show file tree
Hide file tree
Showing 5 changed files with 500 additions and 6 deletions.
45 changes: 39 additions & 6 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ namespace ts {
IsObjectLiteralOrClassExpressionMethod = 1 << 7,
}

let flowNodeCreated: <T extends FlowNode>(node: T) => T = identity;

const binder = createBinder();

export function bindSourceFile(file: SourceFile, options: CompilerOptions) {
Expand Down Expand Up @@ -530,6 +532,7 @@ namespace ts {
blockScopeContainer.locals = undefined;
}
if (containerFlags & ContainerFlags.IsControlFlowContainer) {
const saveFlowNodeCreated = flowNodeCreated;
const saveCurrentFlow = currentFlow;
const saveBreakTarget = currentBreakTarget;
const saveContinueTarget = currentContinueTarget;
Expand All @@ -553,6 +556,7 @@ namespace ts {
currentContinueTarget = undefined;
activeLabels = undefined;
hasExplicitReturn = false;
flowNodeCreated = identity;
bindChildren(node);
// Reset all reachability check related flags on node (for incremental scenarios)
node.flags &= ~NodeFlags.ReachabilityAndEmitFlags;
Expand All @@ -579,6 +583,7 @@ namespace ts {
currentReturnTarget = saveReturnTarget;
activeLabels = saveActiveLabels;
hasExplicitReturn = saveHasExplicitReturn;
flowNodeCreated = saveFlowNodeCreated;
}
else if (containerFlags & ContainerFlags.IsInterface) {
seenThisKeyword = false;
Expand Down Expand Up @@ -858,25 +863,25 @@ namespace ts {
return antecedent;
}
setFlowNodeReferenced(antecedent);
return { flags, expression, antecedent };
return flowNodeCreated({ flags, expression, antecedent });
}

function createFlowSwitchClause(antecedent: FlowNode, switchStatement: SwitchStatement, clauseStart: number, clauseEnd: number): FlowNode {
if (!isNarrowingExpression(switchStatement.expression)) {
return antecedent;
}
setFlowNodeReferenced(antecedent);
return { flags: FlowFlags.SwitchClause, switchStatement, clauseStart, clauseEnd, antecedent };
return flowNodeCreated({ flags: FlowFlags.SwitchClause, switchStatement, clauseStart, clauseEnd, antecedent });
}

function createFlowAssignment(antecedent: FlowNode, node: Expression | VariableDeclaration | BindingElement): FlowNode {
setFlowNodeReferenced(antecedent);
return { flags: FlowFlags.Assignment, antecedent, node };
return flowNodeCreated({ flags: FlowFlags.Assignment, antecedent, node });
}

function createFlowArrayMutation(antecedent: FlowNode, node: CallExpression | BinaryExpression): FlowNode {
setFlowNodeReferenced(antecedent);
const res: FlowArrayMutation = { flags: FlowFlags.ArrayMutation, antecedent, node };
const res: FlowArrayMutation = flowNodeCreated({ flags: FlowFlags.ArrayMutation, antecedent, node });
return res;
}

Expand Down Expand Up @@ -1080,21 +1085,49 @@ namespace ts {
function bindTryStatement(node: TryStatement): void {
const preFinallyLabel = createBranchLabel();
const preTryFlow = currentFlow;
// TODO: Every statement in try block is potentially an exit point!
const tryPriors: FlowNode[] = [];
const oldFlowNodeCreated = flowNodeCreated;
// We hook the creation of all flow nodes within the `try` scope and store them so we can add _all_ of them
// as possible antecedents of the start of the `catch` or `finally` blocks.
// Don't bother intercepting the call if there's no finally or catch block that needs the information
if (node.catchClause || node.finallyBlock) {
flowNodeCreated = node => (tryPriors.push(node), node);
}
bind(node.tryBlock);
flowNodeCreated = oldFlowNodeCreated;
addAntecedent(preFinallyLabel, currentFlow);

const flowAfterTry = currentFlow;
let flowAfterCatch = unreachableFlow;

if (node.catchClause) {
currentFlow = preTryFlow;
if (tryPriors.length) {
const preCatchFlow = createBranchLabel();
addAntecedent(preCatchFlow, currentFlow);
for (const p of tryPriors) {
addAntecedent(preCatchFlow, p);
}
currentFlow = finishFlowLabel(preCatchFlow);
}

bind(node.catchClause);
addAntecedent(preFinallyLabel, currentFlow);

flowAfterCatch = currentFlow;
}
if (node.finallyBlock) {
// We add the nodes within the `try` block to the `finally`'s antecedents if there's no catch block
// (If there is a `catch` block, it will have all these antecedents instead, and the `finally` will
// have the end of the `try` block and the end of the `catch` block)
if (!node.catchClause) {
if (tryPriors.length) {
for (const p of tryPriors) {
addAntecedent(preFinallyLabel, p);
}
}
}

// in finally flow is combined from pre-try/flow from try/flow from catch
// pre-flow is necessary to make sure that finally is reachable even if finally flows in both try and finally blocks are unreachable

Expand Down Expand Up @@ -1142,7 +1175,7 @@ namespace ts {
}
}
if (!(currentFlow.flags & FlowFlags.Unreachable)) {
const afterFinallyFlow: AfterFinallyFlow = { flags: FlowFlags.AfterFinally, antecedent: currentFlow };
const afterFinallyFlow: AfterFinallyFlow = flowNodeCreated({ flags: FlowFlags.AfterFinally, antecedent: currentFlow });
preFinallyFlow.lock = afterFinallyFlow;
currentFlow = afterFinallyFlow;
}
Expand Down
142 changes: 142 additions & 0 deletions tests/baselines/reference/controlFlowForCatchAndFinally.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
//// [controlFlowForCatchAndFinally.ts]
type Page = {close(): Promise<void>; content(): Promise<string>};
type Browser = {close(): Promise<void>};
declare function test1(): Promise<Browser>;
declare function test2(obj: Browser): Promise<Page>;
async function test(): Promise<string> {
let browser: Browser | undefined = undefined;
let page: Page | undefined = undefined;
try {
browser = await test1();
page = await test2(browser);
return await page.content();;
} finally {
if (page) {
await page.close(); // ok
}

if (browser) {
await browser.close(); // ok
}
}
}

declare class Aborter { abort(): void };
class Foo {
abortController: Aborter | undefined = undefined;

async operation() {
if (this.abortController !== undefined) {
this.abortController.abort();
this.abortController = undefined;
}
try {
this.abortController = new Aborter();
} catch (error) {
if (this.abortController !== undefined) {
this.abortController.abort(); // ok
}
}
}
}

//// [controlFlowForCatchAndFinally.js]
"use strict";
var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
return new (P || (P = Promise))(function (resolve, reject) {
function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
function step(result) { result.done ? resolve(result.value) : new P(function (resolve) { resolve(result.value); }).then(fulfilled, rejected); }
step((generator = generator.apply(thisArg, _arguments || [])).next());
});
};
var __generator = (this && this.__generator) || function (thisArg, body) {
var _ = { label: 0, sent: function() { if (t[0] & 1) throw t[1]; return t[1]; }, trys: [], ops: [] }, f, y, t, g;
return g = { next: verb(0), "throw": verb(1), "return": verb(2) }, typeof Symbol === "function" && (g[Symbol.iterator] = function() { return this; }), g;
function verb(n) { return function (v) { return step([n, v]); }; }
function step(op) {
if (f) throw new TypeError("Generator is already executing.");
while (_) try {
if (f = 1, y && (t = op[0] & 2 ? y["return"] : op[0] ? y["throw"] || ((t = y["return"]) && t.call(y), 0) : y.next) && !(t = t.call(y, op[1])).done) return t;
if (y = 0, t) op = [op[0] & 2, t.value];
switch (op[0]) {
case 0: case 1: t = op; break;
case 4: _.label++; return { value: op[1], done: false };
case 5: _.label++; y = op[1]; op = [0]; continue;
case 7: op = _.ops.pop(); _.trys.pop(); continue;
default:
if (!(t = _.trys, t = t.length > 0 && t[t.length - 1]) && (op[0] === 6 || op[0] === 2)) { _ = 0; continue; }
if (op[0] === 3 && (!t || (op[1] > t[0] && op[1] < t[3]))) { _.label = op[1]; break; }
if (op[0] === 6 && _.label < t[1]) { _.label = t[1]; t = op; break; }
if (t && _.label < t[2]) { _.label = t[2]; _.ops.push(op); break; }
if (t[2]) _.ops.pop();
_.trys.pop(); continue;
}
op = body.call(thisArg, _);
} catch (e) { op = [6, e]; y = 0; } finally { f = t = 0; }
if (op[0] & 5) throw op[1]; return { value: op[0] ? op[1] : void 0, done: true };
}
};
function test() {
return __awaiter(this, void 0, void 0, function () {
var browser, page;
return __generator(this, function (_a) {
switch (_a.label) {
case 0:
browser = undefined;
page = undefined;
_a.label = 1;
case 1:
_a.trys.push([1, , 5, 10]);
return [4 /*yield*/, test1()];
case 2:
browser = _a.sent();
return [4 /*yield*/, test2(browser)];
case 3:
page = _a.sent();
return [4 /*yield*/, page.content()];
case 4: return [2 /*return*/, _a.sent()];
case 5:
if (!page) return [3 /*break*/, 7];
return [4 /*yield*/, page.close()];
case 6:
_a.sent(); // ok
_a.label = 7;
case 7:
if (!browser) return [3 /*break*/, 9];
return [4 /*yield*/, browser.close()];
case 8:
_a.sent(); // ok
_a.label = 9;
case 9: return [7 /*endfinally*/];
case 10: return [2 /*return*/];
}
});
});
}
;
var Foo = /** @class */ (function () {
function Foo() {
this.abortController = undefined;
}
Foo.prototype.operation = function () {
return __awaiter(this, void 0, void 0, function () {
return __generator(this, function (_a) {
if (this.abortController !== undefined) {
this.abortController.abort();
this.abortController = undefined;
}
try {
this.abortController = new Aborter();
}
catch (error) {
if (this.abortController !== undefined) {
this.abortController.abort(); // ok
}
}
return [2 /*return*/];
});
});
};
return Foo;
}());
Loading

0 comments on commit 20285e6

Please sign in to comment.