Skip to content

Add sequence protocol to the c-api#8016

Merged
youknowone merged 2 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-sequence
Jun 2, 2026
Merged

Add sequence protocol to the c-api#8016
youknowone merged 2 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-sequence

Conversation

@bschoenmaeckers

@bschoenmaeckers bschoenmaeckers commented Jun 2, 2026 •
edited by coderabbitai Bot
Loading

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.

@coderabbitai

coderabbitai Bot commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

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.

coderabbitai[bot]
coderabbitai Bot reviewed Jun 2, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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

Comment thread crates/capi/src/abstract_/sequence.rs
Comment thread crates/capi/src/abstract_/sequence.rs Outdated
@youknowone youknowone enabled auto-merge (squash) June 2, 2026 09:09
@youknowone youknowone merged commit 5b2f6bc into RustPython:main Jun 2, 2026
26 checks passed
@bschoenmaeckers bschoenmaeckers deleted the c-api-sequence branch June 2, 2026 09:36
@coderabbitai coderabbitai Bot mentioned this pull request Jun 7, 2026
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bschoenmaeckers @youknowone