fix(async): async-generator yield* return() forwarding + non-object next() (#5592) - #5751
Merged
Merged
Conversation
…() + non-object next() (#5592) Async-generator `yield *` only implemented the `next()` direction. Three spec steps were missing, so test262's `yield-star-*` class cases failed: 1. **non-object iter-result** — after `Await(? Call(next, ...))` the spec requires `If Type(innerResult) is not Object, throw a TypeError`. Perry read `.done`/`.value` off a primitive instead, so `gen.next()` fulfilled where it must reject (`yield-star-next-non-object-ignores-then`). 2. **return() forwarding** — `gen.return(v)` while suspended inside a `yield *` must forward to the delegated iterator's `return` method (spec `yield *` step 6.c): complete with `v` when there is no `return`, re-yield on a `{done:false}` result, or complete with the result value on `{done:true}`. Perry completed the outer generator directly, never touching the inner iterator (`yield-star-{sync,async}-return`). 3. **AsyncFromSync return/throw double-read** — `%AsyncFromSyncIterator Prototype%.{return,throw}` do `GetMethod` once then `Call` the captured value; Perry re-dispatched by name, firing a `get return`/`get throw` accessor twice and diverging from Node's operation order. Implementation: - linearize: enforce the Object-check on every awaited delegated pull (async generators only — sync `yield *` reads `.done`/`.value` via `GetV` and never throws), and record each `yield *` suspend-state interval so the `.return()` closure can find the delegated iterator. - lower/abrupt: in an async generator's `.return()` closure, forward to the delegated iterator's `return` (await + Object-check + done dispatch) when suspended in a `yield *` interval; otherwise fall through to the existing completion path. No-op for sync generators / generators without a `yield *`. - runtime: `async_from_sync_call_raw` invokes the already-fetched return/throw method value (with `this = iter`) instead of re-dispatching by name. Fixes 24 of 40 class `yield-star-*` cases (non-object 8, sync-return 8, async-return 8) across async-gen-method{,-static} and async-gen-private- method{,-static} (statements + expressions). The remaining `*-throw` cases need `throw()` forwarding, whose `{done:true}` branch must resume the outer body after the `yield *` (async-generator dispatch-loop continuation, analogous to #4374 for sync generators) — left as follow-up.
📝 WalkthroughWalkthroughImplements spec-compliant ChangesAsync yield* return routing and async-from-sync dispatch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-runtime/src/array/iterator.rs`:
- Around line 345-357: The async-from-sync iterator method lookup in iterator.rs
treats only TAG_UNDEFINED as “absent,” so TAG_NULL currently falls into the
non-callable error path instead of matching GetMethod behavior. Update the
method handling in the async-from-sync iterator logic around the `callable`
decision to treat `crate::value::TAG_NULL` the same as undefined, and keep the
builtin `next` fallback inside that same absent-method branch so `return: null`
and `throw: null` resolve correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d03b6e52-e628-4172-8e47-e4aef5886240
📒 Files selected for processing (5)
crates/perry-runtime/src/array/iterator.rscrates/perry-transform/src/generator/linearize.rscrates/perry-transform/src/generator/lower.rscrates/perry-transform/src/generator/lower/abrupt.rscrates/perry-transform/src/generator/mod.rs
Comment on lines
+345
to
357
| let callable = if method_value.to_bits() == crate::value::TAG_UNDEFINED { | ||
| let raw = crate::value::js_nanbox_get_pointer(iter) as usize; | ||
| if method != b"next" || !is_builtin_iterator_class_id(raw) { | ||
| return Ok(None); | ||
| } | ||
| // A builtin iterator that exposes no readable `next` property: fall back | ||
| // to method dispatch (string/typed-array iterators tower their `next` | ||
| // through the class-id method table). | ||
| false | ||
| } else if !is_callable_value(method_value) { | ||
| return Err(async_from_sync_type_error( | ||
| b"Async-from-sync iterator method is not callable", | ||
| )); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the runtime's null tag/helper name and nearby async-from-sync callers.
rg -n "TAG_NULL|is_null\\(|async_from_sync_(return|throw)|async_from_sync_call\\(" crates/perry-runtime/srcRepository: PerryTS/perry
Length of output: 50370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- iterator.rs around the changed branch ---'
sed -n '320,390p' crates/perry-runtime/src/array/iterator.rs
echo
echo '--- search for async-from-sync / iterator method handling in runtime ---'
rg -n "async_from_sync|GetMethod|TAG_NULL|TAG_UNDEFINED|is_callable_value|method_value.to_bits\\(\\)" \
crates/perry-runtime/src/array crates/perry-runtime/src/object crates/perry-runtime/src/value.rsRepository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- iterator.rs: @@asyncIterator GetMethod handling ---'
sed -n '590,610p' crates/perry-runtime/src/array/iterator.rs
echo
echo '--- iterator.rs: nearby cached-method branch ---'
sed -n '738,752p' crates/perry-runtime/src/array/iterator.rs
echo
echo '--- iterator.rs: async-from-sync branch under review ---'
sed -n '336,360p' crates/perry-runtime/src/array/iterator.rsRepository: PerryTS/perry
Length of output: 3651
Treat TAG_NULL as absent here too. GetMethod treats null the same as undefined, so return: null / throw: null currently hits the non-callable TypeError path instead of the absent-method path. Add crate::value::TAG_NULL to the check and keep the builtin next fallback inside that branch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/array/iterator.rs` around lines 345 - 357, The
async-from-sync iterator method lookup in iterator.rs treats only TAG_UNDEFINED
as “absent,” so TAG_NULL currently falls into the non-callable error path
instead of matching GetMethod behavior. Update the method handling in the
async-from-sync iterator logic around the `callable` decision to treat
`crate::value::TAG_NULL` the same as undefined, and keep the builtin `next`
fallback inside that same absent-method branch so `return: null` and `throw:
null` resolve correctly.
Contributor
Author
VerificationBuild: All 8
Regression surface (the change only executes on the async-generator Note: the full |
proggeramlug
added a commit
that referenced
this pull request
Jun 28, 2026
…me (#5761) * fix(async): #5745 — async-generator yield* .throw() delegation + resume When an async generator is suspended inside a `yield *` and `gen.throw(e)` is called, spec `yield *` step 6.b requires forwarding the error to the delegated iterator's `throw` method. Perry already forwarded `.return()` (#5751) but `.throw()` fell through to the outer generator's own catch routing, so the inner iterator's `throw` never fired, the post-resume yielded value was dropped, and the delegation loop could not continue. Unlike `return`, a `throw` whose inner result is `done` does NOT complete the outer generator — it resumes execution *after* the `yield *` (which may yield again or run to completion). This needs the async-generator abrupt-resume continuation machinery that sync generators have (#4374) but the async path deliberately omitted. Implementation: - DelegationRoute records the drive loop's iter-result local (`result_id`) and its condition-check state (`resume_state` = the `while` cond state). - build_yield_star_throw_routes (async only): GetMethod(iterator,"throw"); if undefined, AsyncIteratorClose the inner iterator then throw a TypeError; else await throw.call(iter, e), enforce the Object result, store it into `result_id`, set state to `resume_state`, and re-drive a clone of the state-dispatch loop. The condition state then reads `result.done` and either exits the loop (resuming the outer body past the `yield *`) or re-yields `result.value` — reproducing spec operation order exactly. - Injected into the `.throw()` closure before the catch-routing fallback, so it only fires while suspended in a delegation. Strictly additive and async-only: `delegations` is recorded solely under `linearize_async_generator()`, so the routes are empty (a no-op) for sync generators and for async generators without a `yield *`. test262 language/{expressions,statements}/class/async-gen-method-static: 96 pass / 1 diff each (was ~10 failing); the remaining diff (`yield-star-sync-next`, a `[Symbol.iterator]` thisValue gap) is unrelated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(async): route delegated yield* throw-protocol errors to outer catch Address CodeRabbit review on #5761: when the delegation protocol itself completes abruptly — `iterator.throw` rejecting, a non-object inner result, or the `throw`-undefined TypeError — the error occurs at the `yield *` site, so an outer `try/catch` around the delegation must be able to handle it (spec `?`/ReturnIfAbrupt semantics). Wrap the protocol work (read `throw`, close-on-undefined, await call, object check) in a generated `try/catch`. The handler routes the caught error into the enclosing try's linearized catch states via build_abrupt_routing, then re-drives the dispatch loop so a `yield` inside that catch suspends; when no catch matches it runs pending non-yielding finallys and re-throws to reject the generator. `state` is still the delegation suspend state in the handler (the success-path `state = resume_state` runs last, inside the try), so it falls inside the outer try's protected interval and matches correctly. Validated: inner `throw` that throws is now caught by an outer `try { yield* inner } catch (e) { yield ... }`; a missing inner `throw` closes the inner iterator via `return` and the TypeError is likewise caught. No test262 regressions across language/{expressions,statements}/{class/ async-gen-method-static,async-generator} + AsyncFromSyncIteratorPrototype. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: note async yielding-finally limitation in yield* throw fallback Clarify (CodeRabbit minor) that the no-catch fallback running only non-yielding finallys before rethrowing matches build_async_throw_body's own async fallback. Routing into a yielding finally needs the pending-completion re-raise machinery gated !is_async_generator; doing so without it would swallow the error. Out of scope; comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ralph <ralph@skelpo.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Subcluster: async-generator
yield *delegation (#5592)The class-tail ticket's single largest root is the async-generator
yield *family — 40 cases across
async-gen-method{,-static}andasync-gen-private-method{,-static}(statements + expressions), all failingbecause Perry only implemented the
next()direction ofyield *delegation.This PR makes three spec steps correct and fixes 24 of those 40.
What was wrong
Non-object iterator result — after
Await(? Call(next, …))the specrequires
If Type(innerResult) is not Object, throw a TypeError. Perry read.done/.valueoff the primitive, sogen.next()fulfilled where it mustreject. (
yield-star-next-non-object-ignores-then)return()not forwarded —gen.return(v)while suspended inside ayield *must forward to the delegated iterator'sreturnmethod(
yield *step 6.c): complete withvwhen there is noreturn, re-yieldon
{done:false}, or complete with the result's value on{done:true}.Perry completed the outer generator directly and never touched the inner
iterator, so the test logs stayed empty.
(
yield-star-{sync,async}-return)AsyncFromSync
return/throwdouble-read —%AsyncFromSyncIteratorPrototype%.{return,throw}doGetMethodonce thenCallthe captured value. Perry re-dispatched by name, firing aget return/get throwaccessor twice and diverging from Node'soperation order. (the sync-iterator half of the
returncases)Fix
perry-transformlinearize — enforce the Object-check on every awaiteddelegated pull (async generators only; sync
yield *reads.done/.valuevia
GetVand never throws), and record eachyield *suspend-state interval.perry-transformlower/abrupt — in an async generator's.return()closure, when suspended inside a recorded
yield *interval, forward to thedelegated iterator's
return(await + Object-check + done dispatch / re-yield);otherwise fall through to the existing completion path. No-op for sync
generators and for generators with no
yield *.perry-runtime—async_from_sync_call_rawnow invokes thealready-fetched
return/throwmethod value (this = iter) instead ofre-dispatching by name.
Before / after (8
yield-star-*class dirs)rt-fail = 0,compile-fail = 0across all 8 dirs; non-class async-generatorand
for await…ofslices show no new runtime/compile failures.What remains
The 16
*-throwcases needthrow()forwarding. Its{done:true}branch mustresume the outer body after the
yield *, which requires theasync-generator dispatch-loop continuation that sync generators already have
(#4374) — async
.throw()/.return()closures don't run the dispatch loop, sothis is a larger state-machine change left as follow-up.
Related
yield *delegation drops.return()/.throw()abrupt resume) — this PR implements the
.return()half of that core feature;.throw()forwarding is the remaining work noted above.async_from_sync_call_rawdouble-read fix isin the
AsyncFromSyncIteratorcluster called out there.Refs #5592, #5745, #5591.
Summary by CodeRabbit
New Features
return()duringyield*handling.Bug Fixes