Skip to content

Ensure call return() for an abrupt completion at for await of loop #52754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rbuckton merged 3 commits into from
May 1, 2023
Merged

Ensure call return() for an abrupt completion at for await of loop #52754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9735089
Ensure call return() for an abrupt completion at for await of loop
niceandneat Feb 14, 2023
287e104
Set non-user code flag at incrementor to make `continue` work again.
niceandneat Apr 29, 2023
a29c2f5
Add test cases for other loop contiditons.
niceandneat May 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 7 additions & 26 deletions src/compiler/transformers/es2018.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -765,11 +765,7 @@ export function transformES2018(context: TransformationContext): (x: SourceFile
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 statements: Statement[] = [iteratorValueStatement, exitNonUserCodeStatement];
const binding = createForOfBindingStatement(factory, node.initializer, value);
statements.push(visitNode(binding, visitor, isStatement));

Expand All @@ -785,28 +781,13 @@ export function transformES2018(context: TransformationContext): (x: SourceFile
statements.push(statement);
}

const body = setEmitFlags(
setTextRange(
factory.createBlock(
setTextRange(factory.createNodeArray(statements), statementsLocation),
/*multiLine*/ true
),
bodyLocation
return setTextRange(
factory.createBlock(
setTextRange(factory.createNodeArray(statements), statementsLocation),
/*multiLine*/ true
),
EmitFlags.NoSourceMap | EmitFlags.NoTokenSourceMaps
bodyLocation
);

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

function createDownlevelAwait(expression: Expression) {
Expand Down Expand Up @@ -857,7 +838,7 @@ export function transformES2018(context: TransformationContext): (x: SourceFile
factory.createAssignment(done, getDone),
factory.createLogicalNot(done)
]),
/*incrementor*/ undefined,
/*incrementor*/ factory.createAssignment(nonUserCode, factory.createTrue()),
/*statement*/ convertForOfStatementHead(node, getValue, nonUserCode)
),
/*location*/ node
Expand Down
167 changes: 167 additions & 0 deletions src/testRunner/unittests/evaluation/forAwaitOf.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,81 @@ describe("unittests:: evaluation:: forAwaitOfEvaluation", () => {
assert.instanceOf(result.output[2], Promise);
});

it("call return when user code return (es2015)", async () => {
const result = evaluator.evaluateTypeScript(`
let returnCalled = false;
async function f() {
const iterator = {
[Symbol.asyncIterator](): AsyncIterableIterator<any> { return this; },
async next() {
return { value: undefined, done: false };
},
async return() {
returnCalled = true;
}
};
for await (const item of iterator) {
return;
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isTrue(await result.main());
});

it("call return when user code break (es2015)", async () => {
const result = evaluator.evaluateTypeScript(`
let returnCalled = false;
async function f() {
const iterator = {
[Symbol.asyncIterator](): AsyncIterableIterator<any> { return this; },
async next() {
return { value: undefined, done: false };
},
async return() {
returnCalled = true;
}
};
for await (const item of iterator) {
break;
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isTrue(await result.main());
});

it("call return when user code throws (es2015)", async () => {
const result = evaluator.evaluateTypeScript(`
let returnCalled = false;
async function f() {
const iterator = {
[Symbol.asyncIterator](): AsyncIterableIterator<any> { return this; },
async next() {
return { value: undefined, done: false };
},
async return() {
returnCalled = true;
}
};
for await (const item of iterator) {
throw new Error();
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isTrue(await result.main());
});

it("don't call return when non-user code throws (es2015)", async () => {
const result = evaluator.evaluateTypeScript(`
let returnCalled = false;
Expand All @@ -132,4 +207,96 @@ describe("unittests:: evaluation:: forAwaitOfEvaluation", () => {
`, { target: ts.ScriptTarget.ES2015 });
assert.isFalse(await result.main());
});

it("don't call return when user code continue (es2015)", async () => {
Copy link
Copy Markdown
Contributor

@rbuckton rbuckton Apr 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests for break, return, and non-local continue (i.e., a labeled continue pointing to a label in an outer loop)?

Copy link
Copy Markdown
Contributor Author

@niceandneat niceandneat May 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added local label continue case.

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) {
continue;
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isFalse(await result.main());
});

it("don't call return when user code continue to local label (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;
}
};
outerLoop:
for (const outerItem of [1, 2, 3]) {
innerLoop:
for await (const item of iterator) {
continue innerLoop;
}
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isFalse(await result.main());
});

it("call return when user code continue to non-local label (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 };
return { value: undefined, done: true };
},
async return() {
returnCalled = true;
}
};
outerLoop:
for (const outerItem of [1, 2, 3]) {
innerLoop:
for await (const item of iterator) {
continue outerLoop;
}
}
}
export async function main() {
try { await f(); } catch { }
return returnCalled;
}
`, { target: ts.ScriptTarget.ES2015 });
assert.isTrue(await result.main());
});
});
67 changes: 16 additions & 51 deletions tests/baselines/reference/emitter.forAwait(target=es2015).js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,10 @@ function f1() {
return __awaiter(this, void 0, void 0, function* () {
let y;
try {
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a;) {
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a; _d = true) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
}
finally {
_d = true;
}
const x = _c;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down Expand Up @@ -114,15 +109,10 @@ function f2() {
return __awaiter(this, void 0, void 0, function* () {
let x, y;
try {
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a;) {
for (var _d = true, y_1 = __asyncValues(y), y_1_1; y_1_1 = yield y_1.next(), _a = y_1_1.done, !_a; _d = true) {
_c = y_1_1.value;
_d = false;
try {
x = _c;
}
finally {
_d = true;
}
x = _c;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down Expand Up @@ -159,15 +149,10 @@ function f3() {
var _a, e_1, _b, _c;
let y;
try {
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;) {
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; _d = true) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
}
finally {
_d = true;
}
const x = _c;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down Expand Up @@ -204,15 +189,10 @@ function f4() {
var _a, e_1, _b, _c;
let x, y;
try {
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;) {
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; _d = true) {
_c = y_1_1.value;
_d = false;
try {
x = _c;
}
finally {
_d = true;
}
x = _c;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down Expand Up @@ -247,16 +227,11 @@ function f5() {
return __awaiter(this, void 0, void 0, function* () {
let y;
try {
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;) {
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; _d = true) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
continue outer;
}
finally {
_d = true;
}
const x = _c;
continue outer;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down Expand Up @@ -294,16 +269,11 @@ function f6() {
var _a, e_1, _b, _c;
let y;
try {
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;) {
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; _d = true) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
continue outer;
}
finally {
_d = true;
}
const x = _c;
continue outer;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down Expand Up @@ -342,15 +312,10 @@ function f7() {
let y;
for (;;) {
try {
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;) {
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; _d = true) {
_c = y_1_1.value;
_d = false;
try {
const x = _c;
}
finally {
_d = true;
}
const x = _c;
}
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
Expand Down
Loading