Editorial: enable built-in async and sync generator functions to be specified

[devsnek]

This is a precursor to https://github.com/tc39/proposal-iterator-helpers, and probably can be integrated into that proposal at some point, but I wanted to get some feedback on this approach before then. I think this change is definitely worth exploring for the purpose of the clarity and maintainability of the specification. You can compare the current iterator methods (https://tc39.es/proposal-iterator-helpers) with what the spec text looks like with this change:

<h1>%IteratorPrototype%.map ( _mapper_ )</h1>
<p>%IteratorPrototype%.map is a built-in generator function which, when called, performs the following prelude steps:</p>
<emu-alg>
  1. Let _iterated_ be ? GetIteratorDirect(*this* value).
  1. If IsCallable(_mapper_) is *false*, throw a *TypeError* exception.
</emu-alg>
<p>The body of %IteratorPrototype%.map is composed of the following steps:</p>
<emu-alg>
  1. Repeat,
    1. Let _next_ be ? IteratorStep(_iterated_).
    1. If _next_ is *false*, return *undefined*.
    1. Let _value_ be ? IteratorValue(_next_).
    1. Let _mapped_ be ? Call(_mapper_, *undefined*, &laquo; _value_ &raquo;).
    1. Perform ? Yield(_mapped_).
</emu-alg>

[ljharb]

Would it be perhaps helpful in demonstrating the change to refactor the matchAll iterator algorithm steps to be a built-in generator function?

[devsnek]

@ljharb here's what i think RegExp.prototype[@@matchAll] would look like. Obviously though we can't do this now because RegExpStringIteratorPrototype is observable.

<p>RegExp.prototype[@@matchAll] is a built-in generator function which, when called, performs the following prelude steps:</p>
<emu-alg>
  1. Let _R_ be the *this* value.
  1. If Type(_R_) is not Object, throw a *TypeError* exception.
  1. Let _S_ be ? ToString(_string_).
  1. Let _C_ be ? SpeciesConstructor(_R_, %RegExp%).
  1. Let _flags_ be ? ToString(? Get(_R_, `"flags"`)).
  1. Let _matcher_ be ? Construct(_C_, &laquo; _R_, _flags_ &raquo;).
  1. Let _lastIndex_ be ? ToLength(? Get(_R_, `"lastIndex"`)).
  1. Perform ? Set(_matcher_, `"lastIndex"`, _lastIndex_, *true*).
<emu-alg>
<p>The body of RegExp.prototype[@@matchAll] is composed of the following steps:</p>
<emu-alg>
  1. If _flags_ contains `"g"`, let _global_ be *true*.
  1. Else, let _global_ be *false*.
  1. If _flags_ contains `"u"`, let _fullUnicode_ be *true*.
  1. Else, let _fullUnicode_ be *false*.
  1. Repeat,
    1. Let _match_ be ? RegExpExec(_matcher_, _S_).
    1. If _match_ is *null*, return *undefined*.
    1. If _global_ is *true*, then
      1. Let _matchStr_ be ? ToString(? Get(_match_, *"0"*)).
      1. If _matchStr_ is the empty string, then
        1. Let _thisIndex_ be ? ToLength(? Get(_matcher_, *"lastIndex"*)).
        1. Let _nextIndex_ be ! AdvanceStringIndex(_S_, _thisIndex_, _fullUnicode_).
        1. Perform ? Set(_matcher_, *"lastIndex"*, ).
      1. Perform ? Yield(_match_).
    1. Else,
      1. Perform ? Yield(_match_).
      1. Return *undefined*.
</emu-alg>

[devsnek]

Actually, we might need to add a GeneratorResume before Return _generator_. to allow the generator to have an initial time to do things like check its argument types. The steps of a builtin function would Yield() once after initialization. Alternatively, we could split the steps into a "prelude" and "body" where the prelude is eagerly evaluated, and the body is evaluated in the generator context.

[ljharb]

Why couldn't we do it now? The only observable part is the toString's native code, which wouldn't change.

[devsnek]

@ljharb because RegExpStringIteratorPrototype is observable, we can't stop having the function return it.

[ljharb]

ahhh right, so to make this PR maximally useful, we'd have to have a way to control the [[Prototype]] of the returned iterator (since we'd want to use this approach on all the iterators in the spec, i'd think)

[bakkot]

I am not convinced this is an improvement in clarity, personally.

[devsnek]

To me the pros are:

• less spec text (fewer possible mistakes)
• less memory usage (without this the iterator method proposal adds an additional 22 prototypes and accompanying methods to every engine)
• easier to optimize (fewer invariants about objects having certain structures and calling patterns need to be fulfilled for fast paths)
• exposes a cohesive api from the standard library

And the cons are:

• individual iterator instances aren't identifiable by @@toStringTag

[bakkot]

Since this is strictly editorial, how could it impact optimizability or memory usage?

[devsnek]

@bakkot because without this we create additional visible methods and prototypes for every iterator method we add (22 in the current proposal), every one of which takes up a non-zero amount of memory and which requires its own checks to make sure its safe to be optimized and lowered in whatever implementation-specific way it will be.

---

Since its also come up twice, i'm also going to rename this pr to normative, even though the diff has no normative changes.

[bakkot]

@devsnek If you want to discuss whether the iterators added by your proposal should be specified as creating generator functions in a user-observable way, that's something which should be discussed on your proposal, since it's about the normative details of your proposal. If you want to discuss whether it's clearer to write iterators in the current style or with a Yield macro, this repo is probably the right place. Those questions need not have anything to do with each other; either normative approach can be specified using either editorial style.

[bakkot]

I think there's conceptually two parts here:

1. Add a Yield macro, and associated machinery, which allows specifying iterator-producing functions in a more compact style (just as the Await macro allows specifying promise-producing functions).
2. Machinery to support built-in generator functions.

There is not necessarily any dependency between the two, and the first makes sense now while the second only makes sense if we actually add a built-in generator function later.

Is it possible change this PR to define Yield in such a way that it could be used for the current iterator-producing built-in functions (and, ideally, do that refactoring), and move the built-in generator functions part to the iterator helpers proposal?

(Talked about this with @ljharb, @syg, and @michaelficarra, and I believe this is their position as well.)

[devsnek]

@bakkot my plan is to make this pr a commit that is part of merging iterator methods into the spec. are you saying you want a separate pr that just refactors yield?

[bakkot]

@devsnek I would like the Yield macro, if it is ever introduced, to be useable for most or all of the functions which produce iterators currently. That part could be productively done now.

[devsnek]

@bakkot you mean use Yield in StringIteratorPrototype and such? That doesn't make sense to me, those don't ever perform yield operations, they're just functions.

[bakkot]

There is no primitive "yield operation". Built-in iterators and the iterators returned by generators are both just a combination of an object which stores some state and a next method invoked on that object which reads and manipulates that state, maybe reads its arguments, and returns some value.

[devsnek]

@bakkot yielding is suspending a context to be resumed later. The iterators currently in the spec are just normal functions. I don't understand how those could be combined.

[bakkot]

It is not observable that (e.g.) string iterators keep track of state by keeping a [[StringIteratorNextIndex]] field on the iterator object holding an index as opposed to keeping a [[GeneratorContext]] field on the iterator object holding an execution context. It's purely an editorial choice.

[devsnek]

@bakkot so you're saying, make an internal generator and then do this.[[InternalGenerator]].next() from IteratorPrototype.next?

That would depend on the built-in generators in this pr, so i couldn't really separate out just the Yield for that.

[bakkot]

Something like that, I guess, except that it would just call GeneratorResume directly (or otherwise directly manipulate an execution context). And presumably that call would be behind a Yield macro, most of the time.

Of course, I'm not sure that capturing an entire execution context is actually clearer than the current approach of just being explicit about the state being tracked. But if that's true, we should not introduce a Yield macro either here or in the iterator helpers proposal.

[devsnek]

I'm really not sure what you're trying to say, and I'm not planning to address how our current iterators are written (at least not right now, maybe after iterator helpers is merged I'll look at ways to simplify them).

[ljharb]

imo the current iterator-producing functions should be written in the same way as the new iterator helpers - whether that's the current style, or an alternative style.

[devsnek]

I guess that would be the hidden built-in generator approach? I wouldn't object to anyone doing such a refactoring.

[michaelficarra]

@devsnek We talked about this in the editor call yesterday. Since this PR includes observable changes, we will need committee consensus before it can be incorporated. The observable changes would need to be rationalised on their own merit, regardless of the impact they would have on spec authorship/readability. Alternatively, you could make this PR strictly editorial.

[devsnek]

@michaelficarra the editor team also said this change was purely editorial before and made me rename it. can you expand specifically on what makes it normative so we're on the same page.

[bakkot]

@devsnek The PR as-is is editorial because it doesn't do anything, because, per discussion above, it can't be used for anything currently in the spec without making observable changes. Using built-in generators for iterator helpers, rather than specifying the helpers the way the spec currently describes all iterators, would also be observable. So either

• Those observable differences should be incorporated on their own merit, at least for iterator helpers, or
• Those observable differences should be rejected, in which case this PR is only useful if it can be refactored to be used for the current iterators in the spec (which would then be the same kind of thing as the iterators in the iterator helpers proposal, so it would be useful for them too), i.e. this PR could be changed to be strictly editorial and also useful for the current spec.

[devsnek]

got it. the intention here is to be merged with iterator helpers anyway so it seems like we're in a good place.

[ExE-Boss]

This has been superseded by #2045, which uses Abstract Closures for this.