GeezerVison is showing github.com — Move PyList Python methods and operators to Py<PyList> by youknowone · Pull Request #8848 · RustPython/RustPython
Conversation
youknowone
commented
Sep 27, 2026
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Follow-up to #8819, same rules applied to list. Four commits.
Move PyList Python methods and operators to Py
-
Py<PyList>::borrow_vec() / borrow_vec_mut() are the views. PyList::__len__() is removed; its callers read borrow_vec().len().
-
The Python interface of list moves from the PyList payload to a #[pyclass] impl Py<PyList> block, registered through with(Py, ...):
- every
#[pymethod] (append, extend, insert, clear, copy, __sizeof__, reverse, __reversed__, __getitem__, count, index, pop, remove, sort) and #[pyclassmethod] __class_getitem__
- the operators
__add__ (which replaces concat()), __iadd__, __mul__, __imul__, __setitem__, __delitem__, __contains__ and inplace_concat
The #[pyclass] impl PyList block keeps only with(...) and flags(...).
-
PyList keeps the helpers that work on the payload: _getitem, _setitem, _delitem, repeat, irepeat and sort, which sorted() and dir() also call on a PyList value before it becomes an object. Py<PyList> reaches them through the payload field. The mapping and sequence slots call the Py<PyList> operators.
-
GenericAlias.__dir__ works on the PyList value returned by vm.dir(), so it appends through borrow_vec_mut() and checks membership with mut_contains().
-
Deref is unchanged and behavior is unchanged.
Rename Traverse::clear to clear_refs
Traverse::clear becomes clear_refs, and MaybeTraverse::try_clear becomes try_clear_refs. Before this, list.clear() on a PyRef<PyList> resolved to the Traverse trait method instead of Py<PyList>::clear. The derive macros and every existing impl are renamed. try_clear_obj, HAS_CLEAR and #[pyclass(clear)] are unchanged.
Expose the PyList_* operations on Py
Only the operations that have a PyList_* counterpart are pub on Py<PyList>. Names follow pyo3's PyListMethods.
Py<PyList> |
C API |
len(), is_empty() |
PyList_Size |
get_item(index, vm) |
PyList_GetItemRef |
set_item(index, item, vm) |
PyList_SetItem |
get_slice(low, high, vm) |
PyList_GetSlice |
set_slice(low, high, items, vm) |
PyList_SetSlice |
del_slice(low, high) |
PyList_SetSlice with NULL |
append, extend, insert, clear, reverse (now pub) |
PyList_Append, PyList_Extend, PyList_Insert, PyList_Clear, PyList_Reverse |
sort(vm) |
PyList_Sort |
to_tuple(vm) |
PyList_AsTuple |
- The
sort pymethod is now implemented by sort_with_options().
get_slice, set_slice and del_slice clamp both bounds to the list.
set_item, set_slice and del_slice drop the replaced elements after the list lock is released.
- The capi
PyList_Size, PyList_Append, PyList_Insert, PyList_Reverse and PyList_AsTuple call these methods.
- A unit test covers the new methods.
Call the Py methods from the remaining capi PyList_* functions
PyList_GetItemRef, PyList_SetItem, PyList_GetSlice, PyList_SetSlice and PyList_Sort now call the Py<PyList> methods. Their behavior changes to match the C API:
PyList_GetSlice and PyList_SetSlice clamp negative bounds to 0 instead of counting them from the end of the list.
PyList_SetSlice collects the new items before taking the list lock, so a list can be assigned into itself.
PyList_GetItemRef and PyList_SetItem raise IndexError without the index in the message.
PyList_Sort sorts the list directly instead of calling its sort method.
PyList_SetItem still appends when the index equals the length and the list has spare capacity, as after PyList_New.
New capi tests cover negative bounds, deleting a slice, and assigning a list into itself.
🤖 Generated with Claude Code
Summary by CodeRabbit
- Bug Fixes
- Corrected C API list indexing so negative indices are treated as out of range.
- Improved list slicing behavior, including deletion and replacing a list’s contents with itself.
- Improvements
- Updated list operations to provide consistent behavior across Python-facing and C API usage, with expanded coverage for list mutations and sorting.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Sep 27, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
|

Note
Reviews paused
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
Use the following commands to manage reviews:
@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.
Use the checkboxes below for quick actions:
- ▶️ Resume reviews
- 🔍 Trigger review
No actionable comments were generated in the recent review.
Walkthrough
-
PyList operations now use payload-level methods, and C API list operations delegate to them. Related runtime paths use borrowed list lengths or mutable list operations. Traversal cleanup hooks use renamed methods across runtime types and generated implementations.
Changes: List data access
| Layer / File(s) |
Summary |
PyList operations and C API integration crates/vm/src/builtins/list.rs, crates/capi/src/listobject.rs |
List item access, mutation, slicing, sorting, and conversion use payload-level methods. Python protocol methods and C API operations delegate to these methods. Tests cover list operations and C API cases. |
Borrowed lengths and mutable list consumers crates/stdlib/src/_heapq.rs, crates/vm/src/builtins/object.rs, crates/vm/src/builtins/genericalias.rs |
Heap algorithms and object slot processing read list lengths from borrowed vectors. Generic-alias directory construction checks and adds entries through mutable list operations. |
Traversal cleanup hooks
| Layer / File(s) |
Summary |
Traversal hooks and generated implementations crates/vm/src/object/traverse.rs, crates/derive-impl/src/pyclass.rs, crates/derive-impl/src/pystructseq.rs |
Traversal traits and generated implementations use clear_refs and try_clear_refs in place of the prior hook names. |
Runtime traversal implementations crates/vm/src/builtins/*, crates/vm/src/stdlib/*, crates/stdlib/src/elementtree.rs, crates/vm/src/exceptions.rs, crates/vm/src/frame.rs, crates/vm/src/object/core.rs |
Runtime traversal implementations use the renamed cleanup hooks. Existing reference-draining and cleanup behavior remains unchanged. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Refactor
Suggested reviewers: shaharnaveh
Merge Risk: Minimal · previous review d4112
-
No identified issue currently blocks merging after normal checks.
Security architecture risk: Low · up to d4112
-
The list and cleanup interfaces have a broad compatibility surface, but the inspected paths preserve list type checks, bounded access, and reference cleanup. No introduced security bypass was established.
Retained concerns
No architecture-level concerns identified.
Security review details
-
Security Blast Radius
- inferred — The inspected attacker-influenced inputs are list objects, indices, slice bounds, and replacement iterables supplied through Python or native list APIs; their demonstrated effects are on the addressed interpreter list, not a newly identified privileged service or store.
Trust Boundaries and Controls
- observed — At the C-to-VM boundary, the inspected wrappers cast the target to PyList before delegation. Negative item indices reach checked access and produce an index error; slice replacement gathers iterable contents before taking mutable list access.
Resilience and Maintainability Implications
- observed — List garbage-collection cleanup still extracts child references, and generated clear dispatch calls the renamed hook rather than the Python-facing list clear method.
🚥 Pre-merge checks | ✅ 4 | ❓ 1
-
❌ Failed checks (1 inconclusive)
| Check name |
Status |
Explanation |
Resolution |
| Docstring Coverage |
❓ Inconclusive |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 114 functions across 22 files. (1 skipped… |
Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
| Check name |
Status |
Explanation |
| Linked Issues check |
✅ Passed |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check |
✅ Passed |
Check skipped because no linked issues were found for this pull request. |
| Description Check |
✅ Passed |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check |
✅ Passed |
The title clearly and concisely describes the primary change: moving PyList Python methods and operators to Py. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 114 functions across 22 files. (1 skipped: 1 too large.)
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Share
-
ℹ️ Recent review info
-
⚙️ Run configuration
-
Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: 0471d9e0-20a5-45ec-a3e2-cb03be74a30a
📥 Commits
-
Reviewing files that changed from the base of the PR and between a90cbe6 and d4112a2.
📒 Files selected for processing (20)
-
crates/capi/src/listobject.rs
crates/derive-impl/src/pyclass.rs
crates/derive-impl/src/pystructseq.rs
crates/stdlib/src/elementtree.rs
crates/vm/src/builtins/dict.rs
crates/vm/src/builtins/function.rs
crates/vm/src/builtins/list.rs
crates/vm/src/builtins/slice.rs
crates/vm/src/builtins/str.rs
crates/vm/src/builtins/tuple.rs
crates/vm/src/builtins/type.rs
crates/vm/src/exceptions.rs
crates/vm/src/frame.rs
crates/vm/src/object/core.rs
crates/vm/src/object/traverse.rs
crates/vm/src/stdlib/_collections.rs
crates/vm/src/stdlib/_collections/ordered_dict.rs
crates/vm/src/stdlib/_functools.rs
crates/vm/src/stdlib/_testinternalcapi.rs
crates/vm/src/stdlib/_thread.rs
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
youknowone
changed the title
Move PyList object operations to Py<PyList>
Move PyList Python methods and operators to Py<PyList>
Sep 27, 2026
This was referenced Nov 3, 2026
Keep list operations on Py<PyList> and route the C API through them. Reuse current checked slice assignment and finalizer ordering, retaining the Traverse method rename so list.clear() is not shadowed. Keep replaced items alive through the write guard scope without a redundant terminal drop.
Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Codex:model-version-unavailable
Assisted-by: Codex:model-version-unavailable
Call the RustPython C-API implementation directly so the negative-index regression test also compiles when workspace Clippy selects pre-3.13 PyO3 bindings. Keep the null result and IndexError assertions intact.
Assisted-by: Codex:model-version-unavailable
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|