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

[niceandneat]

After #51297, early exit(e.g. throw) from for await...of statement body does not trigger .return() of the asyncIterator. This PR removes try/finally block to prevent a nonUserCode variable from being set true when code exit early.

Before

for (...) {
    _nonUserCode = false;
    try {
        const x = _value;
        // for statement body comes here
    }
    finally {
        // early exit at try block always reach this
        _nonUserCode = true;
    }
}

After

for (...) {
    _nonUserCode = false;
    {
        const x = _value;
        // for statement body comes here
    }
    // early exit at try block never reach this
    _nonUserCode = true;
}

It makes iterator call .return() after for loop exits early and fits more with the specification.

Fixes #53106
Fixes #52936

[typescript-bot]

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

[niceandneat]

@microsoft-github-policy-service agree

[kbridge]

I encountered the same problem today. The simplest workarounds include

• changing compilerOptions.target in tsconfig.json to "ES2018" or higher.
• changing TypeScript compiler version in package.json to 4.8.4

[kbridge]

An MCVE:

package.json

{
  "devDependencies": {
    "@types/node": "18.14.0",
    "typescript": "4.9.5"
  },
  "type": "module"
}

tsconfig.json

{
  "compilerOptions": {
    "target": "ES2017",
    "rootDir": ".",
    "outDir": "dist",
    "module": "Node16"
  }
}

index.ts

const asyncIterable = {
  [Symbol.asyncIterator]() {
    return {
      next() {
        console.log('next()');
        return { value: '!', done: false };
      },
      return(value) {
        console.log('return()');
        return { value, done: true };
      },
    };
  }
};

for await (const x of asyncIterable) {
  console.log(x);
  break;
}

code generated:

var __asyncValues = (this && this.__asyncValues) || function (o) {
    if (!Symbol.asyncIterator) throw new TypeError("Symbol.asyncIterator is not defined.");
    var m = o[Symbol.asyncIterator], i;
    return m ? m.call(o) : (o = typeof __values === "function" ? __values(o) : o[Symbol.iterator](), i = {}, verb("next"), verb("throw"), verb("return"), i[Symbol.asyncIterator] = function () { return this; }, i);
    function verb(n) { i[n] = o[n] && function (v) { return new Promise(function (resolve, reject) { v = o[n](v), settle(resolve, reject, v.done, v.value); }); }; }
    function settle(resolve, reject, d, v) { Promise.resolve(v).then(function(v) { resolve({ value: v, done: d }); }, reject); }
};
var _a, e_1, _b, _c;
const asyncIterable = {
    [Symbol.asyncIterator]() {
        return {
            next() {
                console.log('next()');
                return { value: '!', done: false };
            },
            return(value) {
                console.log('return()');
                return { value, done: true };
            },
        };
    }
};
try {
    for (var _d = true, asyncIterable_1 = __asyncValues(asyncIterable), asyncIterable_1_1; asyncIterable_1_1 = await asyncIterable_1.next(), _a = asyncIterable_1_1.done, !_a;) {
        _c = asyncIterable_1_1.value;
        _d = false;
        try {
            const x = _c;
            console.log(x);
            break;
        }
        finally {
            _d = true;
        }
    }
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
    try {
        // _d is always true
        if (!_d && !_a && (_b = asyncIterable_1.return)) await _b.call(asyncIterable_1);
    }
    finally { if (e_1) throw e_1.error; }
}
export {};

[sandersn]

@niceandneat Can you please open a bug first? Please include @kbridge 's repro if it matches your problem.

[niceandneat]

@sandersn Thanks for the guide! I made a #53106 and add it on the PR comment. Please check the issue.

[rbuckton]

Unfortunately, the proposed changes break continue. I have an alternative solution I'm investigating, however.

[rbuckton]

I experimented a bit, and we can definitely drop the inner try-finally and instead switch back to non-user code in the incrementor of the for loop we emit. For example:

// source
for await (const x of y) {
  console.log(x);
}

// generated
var _a, e_1, _b, _c;
try {
    for (
      // start in non-user code
      /*initializer*/ var _d = true, y_1 = __asyncValues(y), y_1_1;
      /*condition*/   y_1_1 = await y_1.next(), _a = y_1_1.done, !_a;
      /*incrementor*/ _d = true // <- switch back to non-user code
    ) {
        _c = y_1_1.value;
        _d = false; // <- switch to user code
        const x = _c;

        // we can remove the inner try..finally and just inline the original body:
        console.log(x);
    }
}
catch (e_1_1) { e_1 = { error: e_1_1 }; }
finally {
    try {
        if (!_d && !_a && (_b = y_1.return)) await _b.call(y_1);
    }
    finally { if (e_1) throw e_1.error; }
}

This will allow local continue to still work, while treating other abrupt completions as user code.

@niceandneat: do you want to update this PR with this change, or should I create a new one?

[niceandneat]

@rbuckton Thanks a lot! I will update this PR with your changes and let you know.

[niceandneat]

@rbuckton I applied some changes to inline the original body. If It has any problem, please let me know.

[rbuckton]

We should preserve bodyLocation and avoid setting these flags.

[rbuckton]

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

[niceandneat]

I also added local label continue case.