1724123 - Update async generator implementation to reflect the latest spec steps

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

Async generator's spec steps are heavily rewritten, based on abstract closures and different set of abstract operations.
Updating our implementation based on them should make it easier to follow the future spec change

Comment 1

[Tooru Fujisawa [:arai]]

Given the current implementation and the spec is very different, I'll perform the following:

1. Remove the spec steps comments
2. Apply a bunch of minor refactoring to the implementation, to make the implementation close to the spec, step by step
3. Once the implementation matches the spec, add the spec steps comments

Comment 2

[Tooru Fujisawa [:arai]]

WIP: https://treeherder.mozilla.org/jobs?repo=try&revision=1351182b970eaa95d5bbd67bd27496338194698d
patch stack:

• https://hg.mozilla.org/try/pushloghtml?changeset=5e85fb16b3fbe0ec6beae6545ec82c71009b4a82
• https://hg.mozilla.org/try/pushloghtml?changeset=1351182b970eaa95d5bbd67bd27496338194698d

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 1: Temporarily disable debugger test that breaks the assumption in the latest spec. r?mgaudet!

Debugger allows calling async generator methods before the state is
changed from executing to suspendedStart.

That was accidentally supported in the previous spec, but the latest spec
optimizes out the behavior for such situation, and it assumes the queue is
empty if suspendedStart, and there's no correct interpretation for such
debugger interaction.

Disable the testcase that hits the issue, until it's handled in Part 43.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 2: Make PromiseHandler an enum class and move it to Promise.h. r?mgaudet!

As a preparation to move async generator functions into AsyncIteration.cpp,
expose PromiseHandler in header file.

Depends on D133228

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 3: Move AsyncGeneratorPromiseReactionJob to AsyncIteration.cpp. r?mgaudet!

Also make functions called by AsyncGeneratorPromiseReactionJob file static.

Depends on D133229

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 4: Add InternalAsyncGeneratorAwait. r?mgaudet!

To keep PromiseReactionRecord Promise.cpp-internal, while exposing InternalAwait
to AsyncIteration.cpp, add dedicate entry point and expose it in the header.

Depends on D133230

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 5: Move AsyncGeneratorAwait to AsyncIteration.cpp. r?mgaudet!

Depends on D133231

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 6: Move AsyncGeneratorResumeNext and AsyncGeneratorEnqueue to AsyncIteration.cpp. r?mgaudet!

Depends on D133232

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 7: Remove stale spec step comment, as a preparation for refactoring. r?mgaudet!

The remaining patches until Part 42 refactor the current implementation
step by step, ignoring the spec step comments, while keeping the behavior
same.

Depends on D133233

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 8: Move AsyncGeneratorCompleteStep-related part out of AsyncGeneratorResumeNext. r?mgaudet!

Those parts correspond to AsyncGeneratorCompleteStep in the latest spec.

Depends on D133234

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 9: Inline AsyncGeneratorResolve and AsyncGeneratorReject. r?mgaudet!

Depends on D133236

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 10: Move AsyncGeneratorCompleteStepNormal/AsyncGeneratorCompleteStepThrow call to the callsite of AsyncGeneratorResumeNext, and the end of loop in AsyncGeneratorResumeNext. r?mgaudet!

Depends on D133238

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 11: Split completed part of AsyncGeneratorResumeNext into AsyncGeneratorDrainQueue. r?mgaudet!

The part corresponds to AsyncGeneratorDrainQueue in the latest spec.

Depends on D133239

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 12: Move validation part out of AsyncGeneratorEnqueue. r?mgaudet!

They correspond to AsyncGeneratorValidate in the latest spec.

Depends on D133240

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 13: Move resuming part out of AsyncGeneratorResumeNext. r?mgaudet!

It corresponds to AsyncGeneratorUnwrapYieldResumption in the latest spec.

Depends on D133241

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 14: Inline AsyncGeneratorResumeNext into AsyncGeneratorYield. r?mgaudet!

Optimized out some branches based on isSuspendedYield condition.

Depends on D133242

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 15: Rename AsyncGeneratorEnqueue to AsyncGeneratorMethodCommon. r?mgaudet!

There's different AsyncGeneratorEnqueue in the latest spec.

Depends on D133243

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 16: Move enqueue part of out of AsyncGeneratorMethodCommon. r?mgaudet!

It corresponds to AsyncGeneratorEnqueue in the latest spec.

Depends on D133244

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 17: Inline AsyncGeneratorResumeNext into AsyncGeneratorMethodCommon. r?mgaudet!

Depends on D133245

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 18: Move await on return out of AsyncGeneratorDrainQueue. r?mgaudet!

It corresponds to AsyncGeneratorAwaitReturn in the latest spec.

Depends on D133246

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 19: Move realm/wrap handling out of AsyncGeneratorMethodCommon. r?mgaudet!

AsyncGeneratorMethodCommon is going to be inlined into the callers.
Factor out the common code.

Depends on D133247

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 20: Remove redundant queue handling for suspended case. r?mgaudet!

The latest spec assumes the queue is empty if the generator is suspended,
and that is what the previous spec also guaranteed, but our debugger allows
breaking the assumption, by calling async generator methods and enqueueing
requests, before it becomes suspendedStart state.

The debugger case is handled in Part 43.

Depends on D133248

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 21: Move suspended start to completed transition. r?mgaudet!

Depends on D133249

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 22: Move AsyncGenerator methods after AsyncGeneratorMethodCommon. r?mgaudet!

Depends on D133250

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 23: Inline AsyncGeneratorMethodCommon into each method. r?mgaudet!

Optimization for each method is applied in later patches.

Depends on D133252

Comment 26

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 24: Optimize AsyncGenerator.prototype.next with completed case by inlining AsyncGeneratorDrainQueue and AsyncGeneratorCompleteStepNormal, skipping queue handling. r?mgaudet!

The queue must be empty also after the async generator becomes completed state
(except for the case the debugger breaks the assumption).

Optimize out the queue handling (enqueue + dequeue), and directly pass the
completion to ResolvePromiseInternal.

Again, the debugger case is handled in Part 43.

Depends on D133253

Comment 27

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 25: Refactor AsyncGeneratorResume callsites as a preparation to move setExecuting. r?mgaudet!

setExecuting is moved into AsyncGeneratorResume in the next part,
so move setExecuting just before AsyncGeneratorResume call.

Depends on D133254

Comment 28

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 26: Move setExecuting into AsyncGeneratorResume. r?mgaudet!

Depends on D133255

Comment 29

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 27: Inline AsyncGeneratorResume into AsyncGeneratorUnwrapYieldResumptionAndResume callsites with completionKind != CompletionKind::Return. r?mgaudet!

Depends on D133256

Comment 30

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 28: Move AsyncGeneratorPromiseReactionJob to the end of the file to avoid forward declaration in later patches. r?mgaudet!

Depends on D133257

Comment 31

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 29: Move functions called by AsyncGeneratorPromiseReactionJob above each reference to the PromiseHandler enum. r?mgaudet!

Depends on D133258

Comment 32

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 30: Move AsyncGeneratorResumeNextReturn{Fulfilled,Rejected} code into separte function. r?mgaudet!

Depends on D133259

Comment 33

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 31: Rename AsyncGeneratorResumeNextReturn{Fulfilled,Rejected} to AsyncGeneratorAwaitReturn{Fulfilled,Rejected}. r?mgaudet!

Depends on D133260

Comment 34

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 32: Move AsyncGeneratorUnwrapYieldResumptionAndResume before the first reference to avoid forward declaration. r?mgaudet!

Depends on D133261

Comment 35

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 33: Remove redundant isCompleted check from AsyncGeneratorDrainQueue. r?mgaudet!

isCompleted() is asserted at the beginning of the function, and the
state doesn't change.

Depends on D133262

Comment 36

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 34: Tweak isQueueEmpty check in AsyncGeneratorDrainQueue. r?mgaudet!

Depends on D133263

Comment 37

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 35: Inline AsyncGeneratorDrainQueue into AsyncGeneratorReturn. r?mgaudet!

Depends on D133264

Comment 38

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 36: Inline AsyncGeneratorDrainQueue and AsyncGeneratorCompleteStepThrow into AsyncGeneratorThrow. r?mgaudet!

Depends on D133265

Comment 39

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 37: Consistently use generator variable name. r?mgaudet!

Depends on D133266

Comment 40

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 38: Remove unused ResumeNextKind enum class. r?mgaudet!

Depends on D133267

Comment 41

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 39: Reorder AsyncIteration.cpp entries to categorize. r?mgaudet!

Depends on D133268

Comment 42

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 40: Flip return condition in AsyncGeneratorUnwrapYieldResumptionAndResume. r?mgaudet!

Depends on D133269

Comment 43

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 41: Move setSuspendedYield call into empty queue case in AsyncGeneratorYield. r?mgaudet!

Depends on D133271

Comment 44

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 42: Add spec steps comment to async generator. r?mgaudet!

Also tweaked some variable names, and added assertions.

Depends on D133272

Comment 45

[Tooru Fujisawa [:arai]]

Attachment: Bug 1724123 - Part 43: Detect async generator in invalid state due to debugger interaction and throw error. r?mgaudet!

As noted in the Part 1, debugger can break the assumption used in the spec,
by calling async generator methods before it becomes suspendedStart,
and there's no correct interpretation for such method call and method calls
after that, because the request queue contains unexpected entry.

Make the async generator methods throw if it's already in such invalid state.

Depends on D133273

Comment 46

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85035e07a46f
Part 1: Temporarily disable debugger test that breaks the assumption in the latest spec. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/32be9ee6760e
Part 2: Make PromiseHandler an enum class and move it to Promise.h. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/3f9f786e9efb
Part 3: Move AsyncGeneratorPromiseReactionJob to AsyncIteration.cpp. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e1d371cbb78f
Part 4: Add InternalAsyncGeneratorAwait. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/2477fb97fafc
Part 5: Move AsyncGeneratorAwait to AsyncIteration.cpp. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/46951c1eed5f
Part 6: Move AsyncGeneratorResumeNext and AsyncGeneratorEnqueue to AsyncIteration.cpp. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/f9e742e82fd0
Part 7: Remove stale spec step comment, as a preparation for refactoring. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/8a5f8ca4af15
Part 8: Move AsyncGeneratorCompleteStep-related part out of AsyncGeneratorResumeNext. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/1fa1faf9b198
Part 9: Inline AsyncGeneratorResolve and AsyncGeneratorReject. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/6df8942df583
Part 10: Move AsyncGeneratorCompleteStepNormal/AsyncGeneratorCompleteStepThrow call to the callsite of AsyncGeneratorResumeNext, and the end of loop in AsyncGeneratorResumeNext. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/be4a39bb8a67
Part 11: Split completed part of AsyncGeneratorResumeNext into AsyncGeneratorDrainQueue. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/da51ef36d048
Part 12: Move validation part out of AsyncGeneratorEnqueue. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/a2b5535e166c
Part 13: Move resuming part out of AsyncGeneratorResumeNext. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/b35debc6633d
Part 14: Inline AsyncGeneratorResumeNext into AsyncGeneratorYield. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/5633e13203e3
Part 15: Rename AsyncGeneratorEnqueue to AsyncGeneratorMethodCommon. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/71e7ca9ea4cf
Part 16: Move enqueue part of out of AsyncGeneratorMethodCommon. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/cb48a1f029d6
Part 17: Inline AsyncGeneratorResumeNext into AsyncGeneratorMethodCommon. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/78344977f6b9
Part 18: Move await on return out of AsyncGeneratorDrainQueue. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/22bc4f406d24
Part 19: Move realm/wrap handling out of AsyncGeneratorMethodCommon. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/fcc4d2ed0e71
Part 20: Remove redundant queue handling for suspended case. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/a0d1f20fea59
Part 21: Move suspended start to completed transition. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/2e0a1c43ee35
Part 22: Move AsyncGenerator methods after AsyncGeneratorMethodCommon. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/db9b6b3ed734
Part 23: Inline AsyncGeneratorMethodCommon into each method. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/192fd6189f1f
Part 24: Optimize AsyncGenerator.prototype.next with completed case by inlining AsyncGeneratorDrainQueue and AsyncGeneratorCompleteStepNormal, skipping queue handling. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/3adfd839f628
Part 25: Refactor AsyncGeneratorResume callsites as a preparation to move setExecuting. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/2fa82bfad4c4
Part 26: Move setExecuting into AsyncGeneratorResume. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/c1cd4b9204e4
Part 27: Inline AsyncGeneratorResume into AsyncGeneratorUnwrapYieldResumptionAndResume callsites with completionKind != CompletionKind::Return. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/096247ef83af
Part 28: Move AsyncGeneratorPromiseReactionJob to the end of the file to avoid forward declaration in later patches. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/7f4f59a149c9
Part 29: Move functions called by AsyncGeneratorPromiseReactionJob above each reference to the PromiseHandler enum. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/f3da2da99748
Part 30: Move AsyncGeneratorResumeNextReturn{Fulfilled,Rejected} code into separte function. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/76381320de77
Part 31: Rename AsyncGeneratorResumeNextReturn{Fulfilled,Rejected} to AsyncGeneratorAwaitReturn{Fulfilled,Rejected}. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/c2d43eaa4a0e
Part 32: Move AsyncGeneratorUnwrapYieldResumptionAndResume before the first reference to avoid forward declaration. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d7e10f507c65
Part 33: Remove redundant isCompleted check from AsyncGeneratorDrainQueue. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/a4b60264d467
Part 34: Tweak isQueueEmpty check in AsyncGeneratorDrainQueue. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/3a50a0a5fd83
Part 35: Inline AsyncGeneratorDrainQueue into AsyncGeneratorReturn. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/82d2e0736efa
Part 36: Inline AsyncGeneratorDrainQueue and AsyncGeneratorCompleteStepThrow into AsyncGeneratorThrow. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e60cfb44c44d
Part 37: Consistently use generator variable name. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/471e291fadc9
Part 38: Remove unused ResumeNextKind enum class. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/554ccce91dd5
Part 39: Reorder AsyncIteration.cpp entries to categorize. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/0e13d5548fce
Part 40: Flip return condition in AsyncGeneratorUnwrapYieldResumptionAndResume. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/a809dff7324e
Part 41: Move setSuspendedYield call into empty queue case in AsyncGeneratorYield. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/d0570f34b3c6
Part 42: Add spec steps comment to async generator. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/4243f988e94a
Part 43: Detect async generator in invalid state due to debugger interaction and throw error. r=mgaudet

Comment 47

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/85035e07a46f
https://hg.mozilla.org/mozilla-central/rev/32be9ee6760e
https://hg.mozilla.org/mozilla-central/rev/3f9f786e9efb
https://hg.mozilla.org/mozilla-central/rev/e1d371cbb78f
https://hg.mozilla.org/mozilla-central/rev/2477fb97fafc
https://hg.mozilla.org/mozilla-central/rev/46951c1eed5f
https://hg.mozilla.org/mozilla-central/rev/f9e742e82fd0
https://hg.mozilla.org/mozilla-central/rev/8a5f8ca4af15
https://hg.mozilla.org/mozilla-central/rev/1fa1faf9b198
https://hg.mozilla.org/mozilla-central/rev/6df8942df583
https://hg.mozilla.org/mozilla-central/rev/be4a39bb8a67
https://hg.mozilla.org/mozilla-central/rev/da51ef36d048
https://hg.mozilla.org/mozilla-central/rev/a2b5535e166c
https://hg.mozilla.org/mozilla-central/rev/b35debc6633d
https://hg.mozilla.org/mozilla-central/rev/5633e13203e3
https://hg.mozilla.org/mozilla-central/rev/71e7ca9ea4cf
https://hg.mozilla.org/mozilla-central/rev/cb48a1f029d6
https://hg.mozilla.org/mozilla-central/rev/78344977f6b9
https://hg.mozilla.org/mozilla-central/rev/22bc4f406d24
https://hg.mozilla.org/mozilla-central/rev/fcc4d2ed0e71
https://hg.mozilla.org/mozilla-central/rev/a0d1f20fea59
https://hg.mozilla.org/mozilla-central/rev/2e0a1c43ee35
https://hg.mozilla.org/mozilla-central/rev/db9b6b3ed734
https://hg.mozilla.org/mozilla-central/rev/192fd6189f1f
https://hg.mozilla.org/mozilla-central/rev/3adfd839f628
https://hg.mozilla.org/mozilla-central/rev/2fa82bfad4c4
https://hg.mozilla.org/mozilla-central/rev/c1cd4b9204e4
https://hg.mozilla.org/mozilla-central/rev/096247ef83af
https://hg.mozilla.org/mozilla-central/rev/7f4f59a149c9
https://hg.mozilla.org/mozilla-central/rev/f3da2da99748
https://hg.mozilla.org/mozilla-central/rev/76381320de77
https://hg.mozilla.org/mozilla-central/rev/c2d43eaa4a0e
https://hg.mozilla.org/mozilla-central/rev/d7e10f507c65
https://hg.mozilla.org/mozilla-central/rev/a4b60264d467
https://hg.mozilla.org/mozilla-central/rev/3a50a0a5fd83
https://hg.mozilla.org/mozilla-central/rev/82d2e0736efa
https://hg.mozilla.org/mozilla-central/rev/e60cfb44c44d
https://hg.mozilla.org/mozilla-central/rev/471e291fadc9
https://hg.mozilla.org/mozilla-central/rev/554ccce91dd5
https://hg.mozilla.org/mozilla-central/rev/0e13d5548fce
https://hg.mozilla.org/mozilla-central/rev/a809dff7324e
https://hg.mozilla.org/mozilla-central/rev/d0570f34b3c6
https://hg.mozilla.org/mozilla-central/rev/4243f988e94a