Return completions when yield* is used

[anba]

var it = Iterator.concat({
  [Symbol.iterator]() { return this; },
  next() { console.log("next"); return {done: false}; },
  return() { console.log("return"); return {done: false}; },
});

Before #18:

js> it.next()
next
({value:(void 0), done:false})
js> it.return()
return
({value:(void 0), done:true})
js> it.return()
({value:(void 0), done:true})

When using yield* semantics:

js> it.next()
next
({done:false})
js> it.return()
return
({done:false})
js> it.return()
return
({done:false})

But the current spec requires something in between, because per spec return completions must always close the iterator, whereas yield* allows to continue after return completions.

After #18:

s> it.next()
next
({done:false})
js> it.return()
return
({value:(void 0), done:true})
js> it.return()
({value:(void 0), done:true})

[anba]

SpiderMonkey implementation before #18:

for (var innerValue of allowContentIterWith(item, method)) {
  yield innerValue;
}

Implementation after #18:

// Step 3.a.i.
var iter = callContentFunction(method, item);

// Step 3.a.ii.
if (!IsObject(iter)) {
  ThrowTypeError(JSMSG_GET_ITER_RETURNED_PRIMITIVE);
}

// Step 3.a.iii.
var iterNext = iter.next;

// Steps 3.a.iv-v.
var wrapped = {
  next() {
    return callContentFunction(iterNext, iter);
  },
  return() {
    IteratorClose(item);
    return {done: true, value: undefined};
  },
};
yield* allowContentIterWithNext(wrapped, wrapped.next);

[michaelficarra]

After #18:

s> it.next()
next
({done:false})
js> it.return()
return
({value:(void 0), done:true})
js> it.return()
({value:(void 0), done:true})

This seems like the correct behaviour. What are you saying is undesirable about it? That it can't be implemented as literally

function* concat(...iterables) {
  for (const it of iterables) yield* it;
}

?

[anba]

Yes, it seems undesirable to add another wrapper to be able to use yield*.

[michaelficarra]

@anba Do you think this design needs to be re-raised with committee? You can review the previous discussion we had here: https://github.com/tc39/notes/blob/main/meetings/2024-12/december-02.md#re-using-iteratorresult-objects-in-iterator-helpers

I'm hoping to bring this for Stage 3 at the next meeting, and would like to have Mozilla's support.

[bakkot]

@anba yield* also forwards arguments passed to to .next() and .return(), which this method does not do (the same as all iterator helpers). So you can't use yield* without a wrapper anyway. Right?

[dminor]

It would be great to see this issue resolved prior to Stage 3, but we would support conditional advancement if that's not possible.

[bakkot]

@dminor Do you have a comment on the observation that even with this change yield* would not be usable because of the difference in whether arguments to .next() and .return() are forwarded to the underlying iterator?

[anba]

I did some performance testing and at least for SpiderMonkey, the additional wrapper approach outlined in #20 (comment) gives a noticeable performance regression when compared to a plain `yield* without the wrapper. But as mentioned in tc39/ecma262#3489 (comment), SpiderMonkey isn't a great target for high performance testing of generators.

To resolve the SpiderMonkey performance concerns, we could add a special function to our self-hosting environment which compiles to a yield*-like expressions which fully implements the special Iterator.concat semantics: https://hg-edge.mozilla.org/try/rev/f2af8c4ccebb85f295ff2df53ae3639642168702. When using this approach, #19 isn't even needed anymore for SpiderMonkey, because we can just compile to different bytecode which doesn't perform IteratorValue on the final yield value.

JSC also uses self-hosting JavaScript to implement Iterator helpers, maybe worth checking with them how they plan to implement Iterator.concat? Possibly also tricky for them because of https://bugs.webkit.org/show_bug.cgi?id=174756.

[anba]

@anba yield* also forwards arguments passed to to .next() and .return(), which this method does not do (the same as all iterator helpers). So you can't use yield* without a wrapper anyway. Right?

Yes, I forgot about argument passing. Thankfully test262 has coverage for this in https://github.com/tc39/test262/blob/main/test/built-ins/Iterator/concat/next-method-called-with-zero-arguments.js and https://github.com/tc39/test262/blob/main/test/built-ins/Iterator/concat/return-method-called-with-zero-arguments.js. Coverage for other Iterator helpers will be added in tc39/test262#4496.

[michaelficarra]

Closing, as we agreed to not try to further pursue matching yield * behaviour at the last plenary.