PyAnextAwaitable #6427
Conversation
Sorry, something went wrong.
WalkthroughAdds qualname storage/accessors for Coro-derived objects, introduces a new PyAnextAwaitable type and registers it, updates builtins.anext to wrap awaitables when a default is provided, and tightens async-iterator protocol checks in get_aiter(). Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller
participant Builtin as builtins.anext
participant Obj as AsyncIterableObj
participant Awaitable as Awaitable (from __anext__)
participant Wrapper as PyAnextAwaitable
Caller->>Builtin: anext(obj, default?)
Builtin->>Obj: check __anext__ exists
alt no __anext__
Builtin-->>Caller: TypeError (not async iterator)
else has __anext__
Builtin->>Obj: call __anext__()
Obj-->>Awaitable: returns awaitable
alt default provided
Builtin->>Wrapper: PyAnextAwaitable::new(awaitable, default)
Builtin-->>Caller: return Wrapper
Caller->>Wrapper: await (calls __await__/send/throw)
Wrapper->>Awaitable: delegate send/throw/close
alt Awaitable raises StopAsyncIteration
Wrapper-->>Caller: raise StopIteration(default)
else Awaitable yields value
Wrapper-->>Caller: return value
end
else no default
Builtin-->>Caller: return Awaitable
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
Sorry, something went wrong.
|
Code has been automatically formatted The code in this PR has been formatted using git pull origin asyncgen |
Sorry, something went wrong.
youknowone
force-pushed
the
asyncgen
branch
3 times, most recently
from
7fe0cdb to
0199879
Compare
December 13, 2025 02:11
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
crates/vm/src/protocol/object.rs (1)
106-110: Consider including the actual type name in the error message.The error message uses a hardcoded string
'async_iterator'rather than the actual type name. CPython typically includes the actual type in such messages.// Check that __aiter__ did not return a coroutine if iterator.downcast_ref::<PyCoroutine>().is_some() { - return Err(vm.new_type_error( - "'async_iterator' object cannot be interpreted as an async iterable; \ - perhaps you forgot to call aiter()?", - )); + return Err(vm.new_type_error(format!( + "'{}' object cannot be interpreted as an async iterable; \ + perhaps you forgot to call aiter()?", + iterator.class().name() + ))); }crates/vm/src/builtins/asyncgenerator.rs (2)
469-522: Redundant coroutine check at line 509.The
downcast_ref::<PyCoroutine>()check on line 509 is unreachable for actual coroutines because line 499 already handles thecoroutine_typecase and returns early. This check would only catch objects that have a__await__method returning aPyCoroutine, which is already an error condition.Consider clarifying the intent or removing the redundant check:
- // Check the result is an iterator, not a coroutine - if awaitable.downcast_ref::<PyCoroutine>().is_some() { - return Err(vm.new_type_error("__await__() returned a coroutine")); - } + // Check the __await__ result is not a coroutine (invalid usage) + if awaitable.class().is(vm.ctx.types.coroutine_type) { + return Err(vm.new_type_error("__await__() returned a coroutine")); + }
552-558: Consider propagating close errors instead of silently ignoring them.The
close()method silently ignores both errors fromget_awaitable_iter()and from the innerclose()call. While ignoringget_awaitable_itererrors may be intentional (the awaitable may already be exhausted), silently ignoringclose()errors could hide legitimate issues.#[pymethod] fn close(&self, vm: &VirtualMachine) -> PyResult<()> { if let Ok(awaitable) = self.get_awaitable_iter(vm) { - let _ = vm.call_method(&awaitable, "close", ()); + // Ignore errors from close() as is typical for cleanup methods, + // but the awaitable may not have a close() method + if let Some(close_method) = vm.get_method(awaitable.clone(), identifier!(vm, close)) { + let _ = close_method?.call((), vm); + } } Ok(()) }
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Lib/test/test_asyncgen.pyis excluded by!Lib/**Lib/test/test_generators.pyis excluded by!Lib/**Lib/test/test_types.pyis excluded by!Lib/**
📒 Files selected for processing (9)
.cspell.dict/python-more.txt(1 hunks)crates/vm/src/builtins/asyncgenerator.rs(3 hunks)crates/vm/src/builtins/coroutine.rs(3 hunks)crates/vm/src/builtins/function.rs(1 hunks)crates/vm/src/builtins/generator.rs(2 hunks)crates/vm/src/coroutine.rs(3 hunks)crates/vm/src/protocol/object.rs(2 hunks)crates/vm/src/stdlib/builtins.rs(1 hunks)crates/vm/src/types/zoo.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/stdlib/builtins.rscrates/vm/src/types/zoo.rscrates/vm/src/builtins/coroutine.rscrates/vm/src/builtins/function.rscrates/vm/src/builtins/generator.rscrates/vm/src/coroutine.rscrates/vm/src/protocol/object.rscrates/vm/src/builtins/asyncgenerator.rs
🧬 Code graph analysis (5)
crates/vm/src/types/zoo.rs (1)
crates/vm/src/class.rs (1)
init_builtin_type(39-48)
crates/vm/src/builtins/coroutine.rs (2)
crates/vm/src/builtins/generator.rs (4)
new(35-39)__qualname__(52-54)set___qualname__(57-59)close(112-114)crates/vm/src/coroutine.rs (5)
new(51-60)frame(180-182)name(184-186)qualname(192-194)close(150-170)
crates/vm/src/builtins/function.rs (2)
crates/vm/src/builtins/coroutine.rs (1)
new(32-36)crates/vm/src/builtins/generator.rs (1)
new(35-39)
crates/vm/src/builtins/generator.rs (3)
crates/vm/src/builtins/asyncgenerator.rs (4)
new(36-41)new(455-460)__qualname__(54-56)set___qualname__(59-61)crates/vm/src/builtins/coroutine.rs (3)
new(32-36)__qualname__(49-51)set___qualname__(54-56)crates/vm/src/builtins/property.rs (1)
setter(249-255)
crates/vm/src/protocol/object.rs (1)
crates/vm/src/builtins/asyncgenerator.rs (5)
__aiter__(89-91)class(25-27)class(156-158)class(206-208)__anext__(94-96)
🔇 Additional comments (21)
.cspell.dict/python-more.txt (1)
7-7: LGTM!The dictionary entry is correctly alphabetized and corresponds to the new
PyAnextAwaitabletype introduced in this PR.crates/vm/src/types/zoo.rs (2)
24-24: LGTM!The
anext_awaitablefield is properly added toTypeZooand follows the established pattern for type registration, grouped appropriately with other async generator related types.
143-143: LGTM!Initialization correctly uses
init_builtin_type()consistent with other type registrations in this file.crates/vm/src/stdlib/builtins.rs (1)
543-561: LGTM!The
anextfunction implementation correctly:
- Validates that the object is an async iterator by checking for
__anext__- Calls
__anext__to get the awaitable- Wraps the result in
PyAnextAwaitablewhen a default value is provided- Returns the raw awaitable when no default is given
The error message appropriately mirrors CPython's behavior for non-async-iterator objects.
crates/vm/src/builtins/function.rs (1)
428-436: LGTM!The updated constructor calls correctly pass both
__name__()and__qualname__()toPyGenerator,PyCoroutine, andPyAsyncGen, aligning with the PR objective to propagate qualified names throughout the generator/coroutine hierarchy.crates/vm/src/protocol/object.rs (1)
90-120: Enhanced async iterable protocol validation looks good.The implementation properly:
- Checks for
__aiter__presence before calling- Detects incorrectly defined
async def __aiter__methods that return coroutines- Validates the result is an async iterator by checking for
__anext__crates/vm/src/builtins/coroutine.rs (3)
32-36: LGTM!The constructor update correctly propagates the new
qualnameparameter toCoro::new, maintaining consistency withPyGenerator::newandPyAsyncGen::new.
48-56: LGTM!The
__qualname__getter and setter follow the same pattern as the existing__name__accessors and correctly delegate to the innerCoromethods.
169-173: LGTM!The
closemethod correctly delegates to the inner coroutine'sclosemethod, completing the wrapper's lifecycle management API alongside the existingsendandthrowmethods.crates/vm/src/coroutine.rs (3)
35-35: LGTM!The new
qualnamefield appropriately usesPyMutex<PyStrRef>for thread-safe access, consistent with the existingnamefield.
51-58: LGTM!The constructor properly initializes the new
qualnamefield alongside existing fields.
192-198: LGTM!The
qualname()getter andset_qualname()setter follow the established pattern of thename()andset_name()methods.crates/vm/src/builtins/generator.rs (2)
35-39: LGTM!The constructor update correctly propagates
qualnametoCoro::new, consistent with the parallel changes inPyCoroutineandPyAsyncGen.
51-59: LGTM!The
__qualname__getter and setter correctly delegate to the innerCoromethods, following the same pattern as the__name__accessors.crates/vm/src/builtins/asyncgenerator.rs (7)
36-41: LGTM!The constructor update correctly propagates
qualnametoCoro::new, consistent with the parallel changes inPyCoroutineandPyGenerator.
53-61: LGTM!The
__qualname__getter and setter correctly delegate to the innerCoromethods, maintaining consistency with the other generator types.
437-460: LGTM!The
PyAnextAwaitablestruct is well-documented and follows the established pattern for implementing Python types in RustPython.
561-569: LGTM!The
handle_resultmethod correctly convertsStopAsyncIterationtoStopIteration(default_value), which is the core functionality needed foranext()with a default value.
572-577: LGTM!The
SelfIterandIterNextimplementations follow the established pattern used by other async generator helper types.
583-583: LGTM!The new
PyAnextAwaitabletype is properly registered in theinitfunction.
524-549:get_awaitable_iter()is called on everysend()/throw()invocation, but this is correct and necessary.
PyAnextAwaitablewraps arbitrary async iterators (unlikePyAsyncGenASendwhich handles only async generators directly). For the common case of wrapping async generators,get_awaitable_iter()returns immediately via an early check. For coroutines and other awaitables, the method must call__await__()fresh on each invocation per PEP 492 requirements — caching the result would violate the protocol by reusing an exhausted iterator. The current design correctly handles all awaitable types without unnecessary overhead.Likely an incorrect or invalid review comment.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
crates/vm/src/coroutine.rs (1)
200-207: Consider usingqualname(notname) inCoro::repr()for CPython parity.If RustPython aims to match CPython’s
<coroutine object outer.<locals>.inner at ...>style,self.qualnameis typically the right display name (and would respect__qualname__mutation as well).
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
Lib/test/test_asyncgen.pyis excluded by!Lib/**Lib/test/test_generators.pyis excluded by!Lib/**Lib/test/test_types.pyis excluded by!Lib/**
📒 Files selected for processing (9)
.cspell.dict/python-more.txt(1 hunks)crates/vm/src/builtins/asyncgenerator.rs(3 hunks)crates/vm/src/builtins/coroutine.rs(3 hunks)crates/vm/src/builtins/function.rs(1 hunks)crates/vm/src/builtins/generator.rs(2 hunks)crates/vm/src/coroutine.rs(3 hunks)crates/vm/src/protocol/object.rs(2 hunks)crates/vm/src/stdlib/builtins.rs(1 hunks)crates/vm/src/types/zoo.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/vm/src/types/zoo.rs
- .cspell.dict/python-more.txt
- crates/vm/src/builtins/function.rs
- crates/vm/src/stdlib/builtins.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/protocol/object.rscrates/vm/src/builtins/coroutine.rscrates/vm/src/builtins/asyncgenerator.rscrates/vm/src/builtins/generator.rscrates/vm/src/coroutine.rs
🧬 Code graph analysis (3)
crates/vm/src/protocol/object.rs (2)
crates/vm/src/builtins/asyncgenerator.rs (5)
__aiter__(89-91)class(25-27)class(156-158)class(206-208)__anext__(94-96)crates/vm/src/coroutine.rs (1)
name(184-186)
crates/vm/src/builtins/coroutine.rs (4)
crates/vm/src/builtins/asyncgenerator.rs (7)
new(36-41)new(455-460)__qualname__(54-56)set___qualname__(59-61)close(277-279)close(401-403)close(553-558)crates/vm/src/builtins/function.rs (6)
new(58-91)new(789-791)new(911-915)__qualname__(567-569)__qualname__(836-855)set___qualname__(572-585)crates/vm/src/builtins/generator.rs (4)
new(35-39)__qualname__(52-54)set___qualname__(57-59)close(112-114)crates/vm/src/coroutine.rs (5)
new(51-60)frame(180-182)name(184-186)qualname(192-194)close(150-170)
crates/vm/src/coroutine.rs (4)
crates/vm/src/builtins/asyncgenerator.rs (2)
new(36-41)new(455-460)crates/vm/src/builtins/coroutine.rs (1)
new(32-36)crates/vm/src/builtins/function.rs (3)
new(58-91)new(789-791)new(911-915)crates/vm/src/builtins/generator.rs (1)
new(35-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Check Rust code with clippy
- GitHub Check: Check the WASM package and demo
🔇 Additional comments (9)
crates/vm/src/builtins/coroutine.rs (3)
32-56: Qualname plumbing on PyCoroutine looks consistent with Coro and other generator-like types.
169-173: Wrapperclose()delegation is a good parity improvement with generator wrappers.
32-36: All call sites have been properly updated to the new constructor signature.The search for any remaining calls to
PyCoroutine::newwith the old 2-parameter signature found no matches, confirming that all call sites throughout the codebase (including tests and stdlib) have been updated to use the new 3-parameter signature(frame: FrameRef, name: PyStrRef, qualname: PyStrRef).crates/vm/src/coroutine.rs (1)
27-60: Coro qualname storage/accessors are straightforward and match the intended propagation pattern.Also applies to: 192-198
crates/vm/src/builtins/generator.rs (2)
35-60: Generator qualname API mirrors coroutine changes cleanly.
35-39: All call sites ofPyGenerator::neware properly updated. The constructor at lines 35-39 accepts three parameters (frame,name,qualname), and the single call site incrates/vm/src/builtins/function.rs:429correctly passes all three arguments.crates/vm/src/builtins/asyncgenerator.rs (3)
36-61: Async generator qualname changes align with the generator/coroutine pattern.
579-584: Type registration wiring looks correct (extend_class for anext_awaitable).
1-12: Please ensurecargo fmt+cargo clippyare clean for these protocol/runtime changes.Also applies to: 579-584
Sorry, something went wrong.
youknowone
merged commit b3c2aa6
into
RustPython:main
Dec 14, 2025
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.