Skip to content

Commit

Permalink
Only call return() for an abrupt completion in user code (#51297)
Browse files Browse the repository at this point in the history
  • Loading branch information
@rbuckton
rbuckton committed Oct 28, 2022
1 parent a7a9d15 commit 5cfb3a2
Show file tree
Hide file tree
Showing 13 changed files with 528 additions and 266 deletions.
50 changes: 40 additions & 10 deletions src/compiler/transformers/es2018.ts
View file
Expand Up @@ -643,12 +643,26 @@ namespace ts {
return node;
}

function convertForOfStatementHead(node: ForOfStatement, boundValue: Expression) {
const binding = createForOfBindingStatement(factory, node.initializer, boundValue);
function convertForOfStatementHead(node: ForOfStatement, boundValue: Expression, nonUserCode: Identifier) {
const value = factory.createTempVariable(hoistVariableDeclaration);
const iteratorValueExpression = factory.createAssignment(value, boundValue);
const iteratorValueStatement = factory.createExpressionStatement(iteratorValueExpression);
setSourceMapRange(iteratorValueStatement, node.expression);

const exitNonUserCodeExpression = factory.createAssignment(nonUserCode, factory.createFalse());
const exitNonUserCodeStatement = factory.createExpressionStatement(exitNonUserCodeExpression);
setSourceMapRange(exitNonUserCodeStatement, node.expression);

const enterNonUserCodeExpression = factory.createAssignment(nonUserCode, factory.createTrue());
const enterNonUserCodeStatement = factory.createExpressionStatement(enterNonUserCodeExpression);
setSourceMapRange(exitNonUserCodeStatement, node.expression);

const statements: Statement[] = [];
const binding = createForOfBindingStatement(factory, node.initializer, value);
statements.push(visitNode(binding, visitor, isStatement));

let bodyLocation: TextRange | undefined;
let statementsLocation: TextRange | undefined;
const statements: Statement[] = [visitNode(binding, visitor, isStatement)];
const statement = visitIterationBody(node.statement, visitor, context);
if (isBlock(statement)) {
addRange(statements, statement.statements);
Expand All @@ -659,7 +673,7 @@ namespace ts {
statements.push(statement);
}

return setEmitFlags(
const body = setEmitFlags(
setTextRange(
factory.createBlock(
setTextRange(factory.createNodeArray(statements), statementsLocation),
Expand All @@ -669,6 +683,18 @@ namespace ts {
),
EmitFlags.NoSourceMap | EmitFlags.NoTokenSourceMaps
);

return factory.createBlock([
iteratorValueStatement,
exitNonUserCodeStatement,
factory.createTryStatement(
body,
/*catchClause*/ undefined,
factory.createBlock([
enterNonUserCodeStatement
])
)
]);
}

function createDownlevelAwait(expression: Expression) {
Expand All @@ -681,6 +707,8 @@ namespace ts {
const expression = visitNode(node.expression, visitor, isExpression);
const iterator = isIdentifier(expression) ? factory.getGeneratedNameForNode(expression) : factory.createTempVariable(/*recordTempVariable*/ undefined);
const result = isIdentifier(expression) ? factory.getGeneratedNameForNode(iterator) : factory.createTempVariable(/*recordTempVariable*/ undefined);
const nonUserCode = factory.createTempVariable(/*recordTempVariable*/ undefined);
const done = factory.createTempVariable(hoistVariableDeclaration);
const errorRecord = factory.createUniqueName("e");
const catchVariable = factory.getGeneratedNameForNode(errorRecord);
const returnMethod = factory.createTempVariable(/*recordTempVariable*/ undefined);
Expand All @@ -704,19 +732,21 @@ namespace ts {
/*initializer*/ setEmitFlags(
setTextRange(
factory.createVariableDeclarationList([
factory.createVariableDeclaration(nonUserCode, /*exclamationToken*/ undefined, /*type*/ undefined, factory.createTrue()),
setTextRange(factory.createVariableDeclaration(iterator, /*exclamationToken*/ undefined, /*type*/ undefined, initializer), node.expression),
factory.createVariableDeclaration(result)
]),
node.expression
),
EmitFlags.NoHoisting
),
/*condition*/ factory.createComma(
/*condition*/ factory.inlineExpressions([
factory.createAssignment(result, createDownlevelAwait(callNext)),
factory.createLogicalNot(getDone)
),
factory.createAssignment(done, getDone),
factory.createLogicalNot(done)
]),
/*incrementor*/ undefined,
/*statement*/ convertForOfStatementHead(node, getValue)
/*statement*/ convertForOfStatementHead(node, getValue, nonUserCode)
),
/*location*/ node
),
Expand Down Expand Up @@ -754,8 +784,8 @@ namespace ts {
factory.createIfStatement(
factory.createLogicalAnd(
factory.createLogicalAnd(
result,
factory.createLogicalNot(getDone)
factory.createLogicalNot(nonUserCode),
factory.createLogicalNot(done),
),
factory.createAssignment(
returnMethod,
Expand Down
27 changes: 27 additions & 0 deletions src/testRunner/unittests/evaluation/forAwaitOf.ts
View file
Expand Up @@ -102,4 +102,31 @@ describe("unittests:: evaluation:: forAwaitOfEvaluation", () => {
assert.instanceOf(result.output[1], Promise);
assert.instanceOf(result.output[2], Promise);
});

it("don't call return when non-user code throws (es2015)", async () => {
const result = evaluator.evaluateTypeScript(`
let returnCalled = false;
async function f() {
let i = 0;
const iterator = {
[Symbol.asyncIterator](): AsyncIterableIterator<any> { return this; },
async next() {
i++;
if (i < 2) return { value: undefined, done: false };
throw new Error();
},
async return() {
returnCalled = true;
}
};
for await (const item of iterator) {
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isFalse(await result.main());
});
});
109 changes: 79 additions & 30 deletions tests/baselines/reference/emitter.forAwait(target=es2015).js
View file
Expand Up @@ -68,18 +68,25 @@ var __asyncValues = (this && this.__asyncValues) || function (o) {
function settle(resolve, reject, d, v) { Promise.resolve(v).then(function(v) { resolve({ value: v, done: d }); }, reject); }
};
function f1() {
var e_1, _a;
var _a, e_1, _b, _c;
return __awaiter(this, void 0, void 0, function* () {
let y;
try {
for (var y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), !y_1_1.done;) {
const x = y_1_1.value;
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield _a.call(y_1);
if (!_d && !_a && (_b = y_1.return)) yield _b.call(y_1);
}
finally { if (e_1) throw e_1.error; }
}
Expand All @@ -103,18 +110,25 @@ var __asyncValues = (this && this.__asyncValues) || function (o) {
function settle(resolve, reject, d, v) { Promise.resolve(v).then(function(v) { resolve({ value: v, done: d }); }, reject); }
};
function f2() {
var e_1, _a;
var _a, e_1, _b, _c;
return __awaiter(this, void 0, void 0, function* () {
let x, y;
try {
for (var y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), !y_1_1.done;) {
x = y_1_1.value;
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
x = _c;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield _a.call(y_1);
if (!_d && !_a && (_b = y_1.return)) yield _b.call(y_1);
}
finally { if (e_1) throw e_1.error; }
}
Expand Down Expand Up @@ -142,17 +156,24 @@ var __asyncGenerator = (this && this.__asyncGenerator) || function (thisArg, _ar
};
function f3() {
return __asyncGenerator(this, arguments, function* f3_1() {
var e_1, _a;
var _a, e_1, _b, _c;
let y;
try {
for (var y_1 = __asyncValues(y), y_1_1; y_1_1 = yield __await(y_1.next()), !y_1_1.done;) {
const x = y_1_1.value;
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield __await(y_1.next()), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield __await(_a.call(y_1));
if (!_d && !_a && (_b = y_1.return)) yield __await(_b.call(y_1));
}
finally { if (e_1) throw e_1.error; }
}
Expand Down Expand Up @@ -180,17 +201,24 @@ var __asyncGenerator = (this && this.__asyncGenerator) || function (thisArg, _ar
};
function f4() {
return __asyncGenerator(this, arguments, function* f4_1() {
var e_1, _a;
var _a, e_1, _b, _c;
let x, y;
try {
for (var y_1 = __asyncValues(y), y_1_1; y_1_1 = yield __await(y_1.next()), !y_1_1.done;) {
x = y_1_1.value;
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield __await(y_1.next()), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
x = _c;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield __await(_a.call(y_1));
if (!_d && !_a && (_b = y_1.return)) yield __await(_b.call(y_1));
}
finally { if (e_1) throw e_1.error; }
}
Expand All @@ -215,19 +243,26 @@ var __asyncValues = (this && this.__asyncValues) || function (o) {
};
// https://github.com/Microsoft/TypeScript/issues/21363
function f5() {
var e_1, _a;
var _a, e_1, _b, _c;
return __awaiter(this, void 0, void 0, function* () {
let y;
try {
outer: for (var y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), !y_1_1.done;) {
const x = y_1_1.value;
continue outer;
outer: for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
continue outer;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield _a.call(y_1);
if (!_d && !_a && (_b = y_1.return)) yield _b.call(y_1);
}
finally { if (e_1) throw e_1.error; }
}
Expand Down Expand Up @@ -256,18 +291,25 @@ var __asyncGenerator = (this && this.__asyncGenerator) || function (thisArg, _ar
// https://github.com/Microsoft/TypeScript/issues/21363
function f6() {
return __asyncGenerator(this, arguments, function* f6_1() {
var e_1, _a;
var _a, e_1, _b, _c;
let y;
try {
outer: for (var y_1 = __asyncValues(y), y_1_1; y_1_1 = yield __await(y_1.next()), !y_1_1.done;) {
const x = y_1_1.value;
continue outer;
outer: for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield __await(y_1.next()), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
continue outer;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield __await(_a.call(y_1));
if (!_d && !_a && (_b = y_1.return)) yield __await(_b.call(y_1));
}
finally { if (e_1) throw e_1.error; }
}
Expand Down Expand Up @@ -296,18 +338,25 @@ var __asyncGenerator = (this && this.__asyncGenerator) || function (thisArg, _ar
// https://github.com/microsoft/TypeScript/issues/36166
function f7() {
return __asyncGenerator(this, arguments, function* f7_1() {
var e_1, _a;
var _a, e_1, _b, _c;
let y;
for (;;) {
try {
for (var y_1 = (e_1 = void 0, __asyncValues(y)), y_1_1; y_1_1 = yield __await(y_1.next()), !y_1_1.done;) {
const x = y_1_1.value;
for (var _d = true, y_1 = (e_1 = void 0, __asyncValues(y)), y_1_1; y_1_1 = yield __await(y_1.next()), _a = y_1_1.done, !_a;) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
}
finally {
_d = true;
}
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
try {
if (y_1_1 && !y_1_1.done && (_a = y_1.return)) yield __await(_a.call(y_1));
if (!_d && !_a && (_b = y_1.return)) yield __await(_b.call(y_1));
}
finally { if (e_1) throw e_1.error; }
}
Expand Down

0 comments on commit 5cfb3a2

Please sign in to comment.