ecma262 icon indicating copy to clipboard operation
ecma262 copied to clipboard

Normative: avoid mostly-redundant `await` in async `yield*`

Open bakkot opened this issue 2 years ago • 4 comments

This is an alternative to #2818. See that PR for more context - this implements option 3 of the options listed there. Fixes https://github.com/tc39/ecma262/issues/2813.

The main effect of this PR would be that if you had

let done = false;
let inner = {
  [Symbol.asyncIterator]: () => ({
    next() {
      if (done) {
        return Promise.resolve({ done: true });
      }
      done = true;
      return Promise.resolve({ done: false, value: Promise.resolve(0) });
    },
  }),
};

async function* outer() {
  yield* inner;
}

(async () => {
  for await (let x of outer()) {
    console.log(x);
  }
})().catch(e => {
  console.log('threw', e);
});

then you would see printed Promise.resolve(0) instead of, as currently, 0. In other words, it would behave as if you'd written for await (let x of inner) instead of of outer().

The other effect is a reduction in the number of promise ticks.

No difference (except in timing) would be observable without manually implementing Symbol.asyncIterator because async generators (and the async-from-sync wrapper, see step 6) already await values before yielding or returning them.

bakkot avatar Jul 07 '22 19:07 bakkot

Tests: https://github.com/tc39/test262/pull/3619

bakkot avatar Jul 28 '22 04:07 bakkot

My understanding was that yield* inner in async generators was intended to work as

for await (let x of inner) {
    yield x;
}

and I understand this changes that. I think this is a footgun. Imagine you had the for-await version because you had a bit of logic before yielding and then you removed that logic. One might think that you would be able to simplify the code by replacing the for-await with yield*, which was working before, but I understand this will not work anymore. You would need to know how inner was implemented which is not a good idea.

raulsebastianmihaila avatar Jul 28 '22 06:07 raulsebastianmihaila

My understanding was that yield* inner in async generators was intended to work as

No, they're different in other ways - yield* forwards the entire generator protocol (next, return, and throw, including arguments), whereas yield does not. So you already need to know the contract of the inner iterator to know if that's a semantics-preserving rewrite.

More importantly, though, this basically should not come up. Syntactic async generators, as well as the async-from-sync wrapper, will prevent you from having a Promise in the value slot at all. The only way you can observe a different value as a result of this change is by manually writing an async iterator which goes out of its way to put a Promise in the value field, which is a contract violation. Having all async generators using yield* pay a cost (the extra await) for well-behaved iterators just to handle that contract violation seems silly, and as far as I can tell was not particularly intended.

By a similar token, you might think that using for await (x of y) { ... } means you never need to await x. This is almost true, except that, just as in the above case, that assumption can fail with a manually implemented async iterator. Should for await then also unconditionally await the value field? We explicitly decided not to do that, with the rationale that it's the producer's responsibility not to do that, not the consumer. The same rationale applies here.


Also, even if you are just reasoning by analogy to other code, yield* is supposed to be deferring the entire protocol to another generator, not doing a transformation on part of it. Consuming (async function*(){ yield* inner })() should work exactly like consuming inner directly. Right now that principle is violated, and I think that's a more important analogy than the analogy to for await (x of y) yield.

bakkot avatar Jul 28 '22 06:07 bakkot

Nice explanation! I think it's unfortunate that we use the same keyword ('yield') for these 2 different ways of producing the resulting value.

raulsebastianmihaila avatar Jul 28 '22 07:07 raulsebastianmihaila