GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Move PyList Python methods and operators to Py<PyList> by youknowone · Pull Request #8848 · RustPython/RustPython

Conversation

@youknowone

@youknowone youknowone commented Sep 27, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

All reactions
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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.

  • Last reviewed commit: d4112a2
  • Latest commit, not reviewed: 8e4561e

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)

  • Create a new PR
  • 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

  • X
  • Mastodon
  • Reddit
  • LinkedIn
ℹ️ 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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone force-pushed the explicit-payload branch from e82b749 to e758e5b Compare September 27, 2026 01:27
@youknowone youknowone changed the title Move PyList object operations to Py<PyList> Move PyList Python methods and operators to Py<PyList> Sep 27, 2026
@youknowone
youknowone force-pushed the explicit-payload branch 3 times, most recently from a90cbe6 to d4112a2 Compare September 27, 2026 15:12
This was referenced Nov 3, 2026
Drop replaced exception and traceback fields outside locks #9600
Merged
Align list, tuple and slice controls #9696
Merged
Align object reduction and state callback controls #9716
Merged
youknowone added 2 commits November 7, 2026 08:03
@youknowone
Reconcile explicit PyList APIs with current main
8ea5d22
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
@youknowone
Update GC storage regression test for clear_refs
eb35587
Assisted-by: Codex:model-version-unavailable
@youknowone
youknowone force-pushed the explicit-payload branch from ca56b94 to eb35587 Compare November 6, 2026 23:04
@youknowone
Test PyList_GetItemRef independently of host Python version
8e4561e
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
@youknowdot youknowdot mentioned this pull request Nov 7, 2026
Match sequence and timestamp signature metadata to Python 3.15 #9742
Merged
6 of 7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

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.

1 participant

@youknowone
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.