Skip to content

Nesting generators compiles to incorrect JavaScript #17276

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.

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

Closed
fixermark opened this issue Jul 18, 2017 · 5 comments
Closed

Nesting generators compiles to incorrect JavaScript #17276

fixermark opened this issue Jul 18, 2017 · 5 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@fixermark
Copy link

TypeScript Version: version on typescriptlang.org/play/ on 2017-7-18 (assuming 2.4.0 / nightly (2.5.0-dev.201xxxxx)

Code

// Put this into the left-hand panel of typescriptlang.org/play/
function* infiniteRepeater(gen) {
    while (true) {
        const repeater = gen();
        for (const el of repeater) {
            yield el;
        }
    }
}

function* yieldAB () {
        yield 'a';
        yield 'b';
}

const repeater = infiniteRepeater(yieldAB);

console.log("Logging first element of repeater");
console.log(repeater.next());
console.log("Element logged.");

Expected behavior:
As seen when run in the browser's JavaScript interpreter, log output should be

Logging first element of repeater
Object {value: "A", done: false}
Element logged.

Actual behavior:
Infinite loop. It appears the generated JavaScript code (shown below) does not allow a generator to be passed as an argument to a generator.

Generated Code

Here is the output in the right-hand panel of http://typescriptlang.org/play/

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 = y[op[0] & 2 ? "return" : op[0] ? "throw" : "next"]) && !(t = t.call(y, op[1])).done) return t;
            if (y = 0, t) op = [0, 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 };
    }
};
// Put this into the left-hand panel of typescriptlang.org/play/
function infiniteRepeater(gen) {
    var repeater_1, _i, repeater_2, el;
    return __generator(this, function (_a) {
        switch (_a.label) {
            case 0:
                if (!true) return [3 /*break*/, 5];
                repeater_1 = gen();
                _i = 0, repeater_2 = repeater_1;
                _a.label = 1;
            case 1:
                if (!(_i < repeater_2.length)) return [3 /*break*/, 4];
                el = repeater_2[_i];
                return [4 /*yield*/, el];
            case 2:
                _a.sent();
                _a.label = 3;
            case 3:
                _i++;
                return [3 /*break*/, 1];
            case 4: return [3 /*break*/, 0];
            case 5: return [2 /*return*/];
        }
    });
}
function yieldAB() {
    return __generator(this, function (_a) {
        switch (_a.label) {
            case 0: return [4 /*yield*/, 'a'];
            case 1:
                _a.sent();
                return [4 /*yield*/, 'b'];
            case 2:
                _a.sent();
                return [2 /*return*/];
        }
    });
}
var repeater = infiniteRepeater(yieldAB);
console.log("Logging first element of repeater");
console.log(repeater.next());
console.log("Element logged.");
@olegdunkan
Copy link

A generator also supports yield * which means that it will delegate to another generator and yield the results that the inner generator yields.

function* infiniteRepeater(gen) {
    while (true) {
//!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
        const repeater = yield *gen(); 
//!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
        for (const el of repeater) {
            yield el;
        }
    }
}

function* yieldAB () {
        yield 'a';
        yield 'b';
}

const repeater = infiniteRepeater(yieldAB);

console.log("Logging first element of repeater");
console.log(repeater.next());
console.log("Element logged.");

@fixermark
Copy link
Author

This is true (and thank you for the suggestion olegdunkan; the JS generated by this change and removing the "for const el of repeater" block below the !!!! lines works as expected instead of infinite looping).

However, I believe this is still a bug; the code you recommended and the original should have the same result (as they do in JS), correct?

@fixermark
Copy link
Author

Note: This workaround didn't actually serve our purpose; I left out a detail in the minimum example that we needed in practice in the production code. To guard against the possibility that gen yields no values (which would make olegdunkan's suggestion infinite loop), one needs to check the count of values yielded by gen() and if it's == 0, return from infiniteRepeater instead of building a new gen that again returns 0 results, etc.
To do this, one needs to for-of across gen inside infiniteRepeater instead of delegating, so no good. :(

Both the minimum example at the top of this issue and the construction I've described show the same bad behavior of not allowing the polyfilled generator to be passed as an argument to a generator.

@rbuckton
Copy link
Member

rbuckton commented Oct 9, 2017

@fixermark While generators are allowed in ES5, we do not change the semantics of for..of statements in ES5 unless you also enable the --downlevelIteration switch. The problem is that you have not given a type to gen, so it is implicitly any. This in turn gives repeater the type of any. If you give gen the type typeof yieldAB, you would see the following error reported:

error TS2495: Type 'IterableIterator<"a" | "b">' is not an array type or a string type.

@rbuckton
Copy link
Member

rbuckton commented Oct 9, 2017

We chose not to always down-level for..of to support iterators as it significantly impacts the performance of existing code written to use for..of against only arrays.

@mhegazy mhegazy added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Bug A bug in TypeScript labels Oct 10, 2017
@mhegazy mhegazy removed this from the TypeScript 2.6 milestone Oct 10, 2017
@mhegazy mhegazy closed this as completed Oct 10, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants