Add integer c-api support by bschoenmaeckers · Pull Request #7905 · RustPython/RustPython
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: b0b52f12-8315-46af-b738-d8cb087d8887
📒 Files selected for processing (2)
crates/capi/src/longobject.rscrates/capi/src/object.rs
✅ Files skipped from review due to trivial changes (1)
- crates/capi/src/object.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/capi/src/longobject.rs
📝 Walkthrough
Walkthrough
Adds a new longobject cAPI module: extern "C" PyLong constructors and extractors for multiple C integer types, plus an FfiResult implementation for c_ulonglong; includes a disabled test module.
Changes
Python Long Integer FFI Bindings
| Layer / File(s) | Summary |
|---|---|
Module declaration and imports crates/capi/src/lib.rs, crates/capi/src/object.rs, crates/capi/src/longobject.rs |
Exposes longobject as a public module, re-exports define_py_check crate-wide, and declares PyLong_Check/PyLong_CheckExact plus top-level imports. |
PyLong constructors from C integers crates/capi/src/longobject.rs |
Six PyLong_From* extern "C" functions allocate Python ints from c_long, c_longlong, isize, usize, c_ulong, and c_ulonglong via `with_vm( |
PyLong extraction to C integers crates/capi/src/longobject.rs |
Unsafe PyLong_AsLong and PyLong_AsUnsignedLongLong dereference *mut PyObject, coerce to bigint/PyInt, attempt downcast to requested C type, and map failures to VM overflow errors with fixed messages. |
Disabled test validation crates/capi/src/longobject.rs |
#[cfg(false)] test module (pyo3) with disabled checks for i32 and u64 extraction; not compiled in normal builds. |
FfiResult impl for c_ulonglong crates/capi/src/util.rs |
Implements FfiResult for c_ulonglong with ERR_VALUE = Self::MAX and into_output returning the input value. |
Sequence Diagram(s)
sequenceDiagram
participant C_Caller
participant FFI_Constructor as PyLong_From*
participant VM_Context as with_vm
participant VM_CTX as vm.ctx
participant PyInt as Python_int
C_Caller->>FFI_Constructor: call PyLong_From*(C integer)
FFI_Constructor->>VM_Context: enter VM context
VM_Context->>VM_CTX: vm.ctx.new_int(value)
VM_CTX->>PyInt: create Python int object
PyInt-->>FFI_Constructor: PyObject* returned
FFI_Constructor-->>C_Caller: PyObject*
sequenceDiagram
participant C_Caller
participant FFI_Extractor as PyLong_As*
participant PyObject_Ptr
participant BigInt_Conversion
participant VM_Context
C_Caller->>FFI_Extractor: call PyLong_As*(PyObject*)
FFI_Extractor->>PyObject_Ptr: dereference & coerce
PyObject_Ptr->>BigInt_Conversion: extract bigint value
BigInt_Conversion->>BigInt_Conversion: try downcast to C type
alt success
BigInt_Conversion-->>FFI_Extractor: numeric value
FFI_Extractor-->>C_Caller: return C integer
else overflow/failure
BigInt_Conversion->>VM_Context: set overflow error
VM_Context-->>FFI_Extractor: error indicator
FFI_Extractor-->>C_Caller: error sentinel (ERR_VALUE)
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- RustPython/RustPython#7787: Prior cAPI/FFI error and FfiResult plumbing related to capi util and VM-exception mapping.
- RustPython/RustPython#7871: Introduced the
define_py_check!macro usage for generating PyLong check functions.
Suggested reviewers
- youknowone
- ShaharNaveh
Poem
🐰 I hop from C to Python, numbers in my paw,
I stitch longs with care and check for overflow,
From Rusty roots I craft each tiny int,
A FFI bridge where rabbits dream and sprint,
Hooray — the bindings sing, and onward we go.
🚥 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 integer c-api support' directly and accurately summarizes the main purpose of the pull request: adding C-API support for Python integer objects through multiple FFI functions. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 83.33% 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.
Comment @coderabbitai help to get the list of available commands and usage tips.