Add number protocol functions to c-api by bschoenmaeckers · Pull Request #8005 · RustPython/RustPython

bschoenmaeckers · 2026-06-01T15:07:54Z

Summary by CodeRabbit

New Features
- Numeric operations are exposed in the C API: addition, subtraction, left/right shifts, bitwise OR, and indexing; the numeric module is publicly available.
Tests
- Unit tests for these numeric behaviors remain in the codebase but are disabled and not executed.

coderabbitai · 2026-06-01T15:08:08Z

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

📝 Walkthrough

Walkthrough

Adds a number C-API module and re-export; implements six PyNumber_* extern "C" FFI functions that call into the VM for numeric/bitwise operations. A test module with integer-operation checks exists but is disabled via #[cfg(false)].

Changes

PyNumber FFI Bindings

Layer / File(s)	Summary
FFI implementations and module wiring `crates/capi/src/abstract_.rs`, `crates/capi/src/abstract_/number.rs` (lines 1–31)	Declares the `number` module and `pub use number::*`; adds `#[no_mangle] pub unsafe extern "C"` functions `PyNumber_Add`, `PyNumber_Index`, `PyNumber_Lshift`, `PyNumber_Or`, `PyNumber_Rshift`, and `PyNumber_Subtract` that call into the VM via `with_vm` and perform unsafe pointer dereference.
Number operation tests (disabled) `crates/capi/src/abstract_/number.rs` (lines 33–86)	Test module constructs Python integers and asserts extracted `i64` results for add, lshift, bitor, rshift, sub, but the module is guarded with `#[cfg(false)]` so it is not compiled or executed.

Sequence Diagram(s)

sequenceDiagram
  participant C as C caller
  participant FFI as PyNumber_* (FFI)
  participant VM as RustPython VM
  C->>FFI: PyNumber_Add(PyObject*, PyObject*)
  FFI->>VM: with_vm -> vm._add(&*o1, &*o2)
  VM-->>FFI: *mut PyObject
  FFI-->>C: *mut PyObject

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

RustPython/RustPython#7882: Adds C-API extern "C" exports under abstract_.rs, related to adding PyNumber_* entrypoints in this PR.

Suggested Reviewers

youknowone

Poem

🐰 I nibble at bindings, bits, and sum,
I bridge raw pointers so calls may run.
Add, shift, and or now routed through,
Silent tests wait, not yet in view. 🥕

🚥 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 clearly and accurately summarizes the main change: adding number protocol functions to the C API.
Docstring Coverage	✅ Passed	Docstring coverage is 91.67% 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.}

ShaharNaveh

🚀

coderabbitai

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_/number.rs`:
- Around line 33-86: The tests currently exercise Python-level methods but not
the exported C-ABI functions, so add unit tests that call the no_mangle extern
"C" symbols directly (PyNumber_Add, PyNumber_Lshift, PyNumber_Or,
PyNumber_Rshift, PyNumber_Subtract and PyNumber_Index) via the pyo3 ffi
pointers; inside Python::with_gil (or Python::attach) obtain raw PyObject
pointers for lhs/rhs, call the corresponding pyo3::ffi::PyNumber_* function in
an unsafe block, check the returned *mut pyo3::ffi::PyObject is non-null, wrap
or convert it back to a PyObject and extract the i64/expected value, and DECREF
returned objects as appropriate to avoid leaks so these tests truly validate the
exported C-ABI entrypoints.
- Around line 3-31: The FFI entry points PyNumber_Add, PyNumber_Index,
PyNumber_Lshift, PyNumber_Or, PyNumber_Rshift, and PyNumber_Subtract perform raw
pointer derefs and must be declared unsafe; update each function signature from
the current extern "C" form to be pub unsafe extern "C" fn while preserving the
no_mangle attribute (or replacing #[no_mangle] if present) so the functions are
exported with the correct caller safety contract (adjust signatures for
PyNumber_Add, PyNumber_Index, PyNumber_Lshift, PyNumber_Or, PyNumber_Rshift,
PyNumber_Subtract).

🪄 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: f91a989d-6d34-44e5-8c47-8561f475250d

📥 Commits

Reviewing files that changed from the base of the PR and between 1ce37bf and d5643f3.

📒 Files selected for processing (2)

crates/capi/src/abstract_.rs
crates/capi/src/abstract_/number.rs

bschoenmaeckers · 2026-06-01T15:27:16Z

@coderabbitai autofix

coderabbitai · 2026-06-01T15:27:22Z

This command requires write access to the repository. Only users with write or admin permissions can trigger CodeRabbit to commit or create pull requests.

coderabbitai

Actionable comments posted: 1

🤖 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_/number.rs`:
- Around line 33-34: The test module is permanently disabled by #[cfg(false]);
change it to #[cfg(test)] so tests are compiled and run, then update the tests
in mod tests to exercise the C ABI entrypoints for the new PyNumber_* functions
(call the exported FFI wrappers rather than internal helpers) and adjust any
signatures/unsafe blocks to match the C-ABI bindings (e.g., use the public
extern "C" functions named PyNumber_*) so the tests actually invoke the FFI
layer; alternatively, if you intend to defer tests, remove the dead mod tests
block entirely.

🪄 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: a841889b-bf28-4038-bf28-ea5973f3bbcd

📥 Commits

Reviewing files that changed from the base of the PR and between 4f64690 and 82c8f92.

📒 Files selected for processing (2)

crates/capi/src/abstract_.rs
crates/capi/src/abstract_/number.rs

🚧 Files skipped from review as they are similar to previous changes (1)

crates/capi/src/abstract_.rs

youknowone

👍 please rebase this

bschoenmaeckers · 2026-06-02T07:35:48Z

👍 please rebase this

Done

bschoenmaeckers · 2026-06-02T09:36:12Z

Rebased, please try to merge again 🙏

bschoenmaeckers changed the title ~~Add number protocol functions~~ Add number protocol functions to c-api Jun 1, 2026

ShaharNaveh approved these changes Jun 1, 2026

View reviewed changes

coderabbitai Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread crates/capi/src/abstract_/number.rs

Comment thread crates/capi/src/abstract_/number.rs Outdated

bschoenmaeckers marked this pull request as draft June 1, 2026 15:31

bschoenmaeckers force-pushed the c-api-number branch from d5643f3 to 4f64690 Compare June 1, 2026 16:51

bschoenmaeckers marked this pull request as ready for review June 1, 2026 16:51

bschoenmaeckers force-pushed the c-api-number branch from 4f64690 to 82c8f92 Compare June 1, 2026 17:25

coderabbitai Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread crates/capi/src/abstract_/number.rs

youknowone approved these changes Jun 2, 2026

View reviewed changes

bschoenmaeckers force-pushed the c-api-number branch from 82c8f92 to 6f43fe6 Compare June 2, 2026 07:25

bschoenmaeckers force-pushed the c-api-number branch from 6f43fe6 to 9cb27ef Compare June 2, 2026 08:31

youknowone enabled auto-merge (squash) June 2, 2026 09:08

Add number protocol functions

d9c591b

auto-merge was automatically disabled June 2, 2026 09:35
Head branch was pushed to by a user without write access

bschoenmaeckers force-pushed the c-api-number branch from 9cb27ef to d9c591b Compare June 2, 2026 09:35

youknowone merged commit 60b1346 into RustPython:main Jun 3, 2026
26 checks passed

bschoenmaeckers deleted the c-api-number branch June 3, 2026 09:38

Conversation

bschoenmaeckers commented Jun 1, 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 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Sequence Diagram(s)

Estimated Code Review Effort

Possibly related PRs

Suggested Reviewers

Poem

Uh oh!

ShaharNaveh left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bschoenmaeckers commented Jun 1, 2026

Uh oh!

coderabbitai Bot commented Jun 1, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

youknowone left a comment

Choose a reason for hiding this comment

Uh oh!

bschoenmaeckers commented Jun 2, 2026

Uh oh!

bschoenmaeckers commented Jun 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bschoenmaeckers commented Jun 1, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 1, 2026 •

edited

Loading