GeezerVison

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

GeezerVison is showing github.com — Add sequence protocol to the c-api by bschoenmaeckers · Pull Request #8016 · RustPython/RustPython

Conversation

bschoenmaeckers commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Summary by CodeRabbit

New Features
- Added comprehensive Python sequence operation support: sequence type checks, item and slice access/modification/deletion, concatenation and in-place concatenation, repetition and in-place repetition, length/size queries, count/index/search, membership testing, and conversion to list/tuple.

Sorry, something went wrong.

Uh oh!

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

All reactions


          Add sequence protocol to the c-api

df23d3a

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: d8f99341-f787-4404-890e-ad2b5d20b563

📥 Commits

Reviewing files that changed from the base of the PR and between df23d3a and 77b3306.

📒 Files selected for processing (1)

crates/capi/src/abstract_/sequence.rs

📝 Walkthrough

Walkthrough

Adds a new sequence module to the C API abstract layer and implements exported PySequence_* FFI wrappers that forward sequence operations into the VM; includes a disabled pyo3 test module.

Changes

PySequence C API FFI Module

Layer / File(s)	Summary
Module declaration and public re-export `crates/capi/src/abstract_.rs`	The `sequence` module is declared and re-exported via `pub use sequence::*` so its public items are available from the abstract layer.
PySequence_ FFI implementations* `crates/capi/src/abstract_/sequence.rs`	Implements exported `#[no_mangle] extern "C"` wrappers for sequence protocol operations (Check, Size, GetItem/SetItem/DelItem, GetSlice/SetSlice/DelSlice, Concat/InPlaceConcat, Repeat/InPlaceRepeat, Count, Index, List, Tuple, Contains, and membership wrapper). Each wrapper calls `with_vm`, dereferences `PyObject*` inputs, and delegates to sequence APIs.
Disabled pyo3 test module `crates/capi/src/abstract_/sequence.rs`	A `#[cfg(false)]` test module (pyo3) exercises item/slice ops, concat/repeat/in-place behaviors, count/index/contains, and dict membership; it is disabled at compile time.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

youknowone
ShaharNaveh

Poem

🐰 I hopped through code with nimble paws,
I bound the sequences to C's applause,
Sixteen calls now answer the VM's beat,
Lists and tuples all lined up neat,
RustPython sings — a rabbit's treat!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Add sequence protocol to the c-api' directly and clearly describes the main change: adding sequence protocol support to the C API. It accurately summarizes the changeset which adds a new sequence module with multiple FFI functions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

All reactions

Sorry, something went wrong.

Uh oh!

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

coderabbitai Bot reviewed

Jun 2, 2026

View reviewed changes

coderabbitai Bot left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/capi/src/abstract_/sequence.rs`:
- Around line 145-157: Add two thin no_mangle forwarders: implement an unsafe
extern "C" fn PySequence_Length that simply calls/returns PySequence_Size with
the same signature and an unsafe extern "C" fn PySequence_In that forwards to
PySequence_Contains with the same signature; annotate both with
#[unsafe(no_mangle)] and place them alongside the existing PySequence_Size and
PySequence_Contains exports so the stable-ABI symbols PySequence_Length and
PySequence_In are available.
- Around line 162-166: The function PySequence_Contains currently dereferences
caller-provided raw pointers (obj and value) but is declared as a safe extern
"C" function; change its signature to be unsafe (pub unsafe extern "C" fn
PySequence_Contains(...)) to match the other PySequence_* exports and correctly
mark that callers must uphold pointer safety, leaving the internal logic (using
&*obj, &*value and calling obj.sequence_unchecked().contains(value, vm))
unchanged.

🪄 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: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: abd8d7df-9cbc-4522-9ace-08ce34ca1b0e

📥 Commits

Reviewing files that changed from the base of the PR and between 0c9ed36 and df23d3a.

📒 Files selected for processing (2)

crates/capi/src/abstract_.rs
crates/capi/src/abstract_/sequence.rs

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

crates/capi/src/abstract_/sequence.rs Resolved

Uh oh!

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

Comment thread

crates/capi/src/abstract_/sequence.rs Resolved

Uh oh!

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


          Review

77b3306

youknowone approved these changes

Jun 2, 2026

View reviewed changes

youknowone enabled auto-merge (squash)

June 2, 2026 09:09

youknowone merged commit 5b2f6bc into RustPython:main

Jun 2, 2026

26 checks passed

Uh oh!

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

bschoenmaeckers deleted the c-api-sequence branch

June 2, 2026 09:36

coderabbitai Bot mentioned this pull request

Jun 7, 2026

Code nitpicks #8058

Merged

2 tasks

youknowone pushed a commit that referenced this pull request

Sep 16, 2026


          Add sequence protocol to the c-api (#8016)

c1aeeef

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

youknowone

youknowone approved these changes

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.

2 participants

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 01402520 — 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.

GeezerVison

Conversation

bschoenmaeckers commented Jun 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bschoenmaeckers commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

Uh oh!

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