Skip to content

Add capsule support to c-api#7940

Merged
youknowone merged 5 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-capsule
May 26, 2026
Merged

Add capsule support to c-api#7940
youknowone merged 5 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-capsule

Conversation

@bschoenmaeckers
Copy link
Copy Markdown
Contributor

@bschoenmaeckers bschoenmaeckers commented May 20, 2026

Summary by CodeRabbit

  • New Features
    • Support for Python PyCapsule objects with optional names and separate context storage.
    • FFI surface to create, validate, import, and get/set capsule pointer, name, and context.
    • pycapsule module is now publicly exposed for external use.
  • Tests
    • Unit tests for capsule creation, validation, and pointer retrieval.
  • Chores
    • Added "pycapsule" to the spell dictionary.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

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 PyCapsule support to the C-ABI: capsule objects now hold an optional name and a context pointer; Context::new_capsule accepts a name; FFI utilities for const C strings added; and a full set of PyCapsule C-ABI functions (New, Get/Set pointer/context/name, IsValid, Import) with tests are implemented.

Changes

PyCapsule C-ABI Implementation with Name and Context Support

Layer / File(s) Summary
PyCapsule Type Extension
crates/vm/src/builtins/capsule.rs
PyCapsule gains context: AtomicPtr<c_void> and name: Option<&'static CStr>; constructor stores name and initializes context; adds set_pointer, context, set_context, and name accessors.
VM Context API Update
crates/vm/src/vm/context.rs
Context::new_capsule now accepts name: Option<&'static CStr> and uses c_void/CStr imports, forwarding ptr, name, and destructor to PyCapsule::new.
CAPI FFI Utilities and Module Export
crates/capi/src/util.rs, crates/capi/src/lib.rs, .cspell.dict/python-more.txt
Implements FfiResult for *const c_char (null as error), exports the pycapsule module from the capi crate, and adds pycapsule to the spellcheck dictionary.
CAPI PyCapsule Functions and Tests
crates/capi/src/pycapsule.rs
Adds C-ABI functions: PyCapsule_New, PyCapsule_GetPointer, PyCapsule_GetName, PyCapsule_GetContext, PyCapsule_SetContext, PyCapsule_SetPointer, PyCapsule_IsValid, PyCapsule_Import; includes names_match and checked_capsule helpers and a unit test verifying creation, name validation, and pointer retrieval.

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly Related PRs

Suggested Reviewers

  • youknowone
  • ShaharNaveh

"I’m a rabbit in Rust, hiding names in tails,
Context tucked snug where the pointer prevails.
From VM to C-ABI, the capsule hops through,
Functions and tests open doors in view.
🐇✨"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add capsule support to c-api' directly and clearly describes the main change: implementing PyCapsule C-API functions and supporting infrastructure across multiple modules.
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.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (2)
crates/capi/src/util.rs (1)

79-85: 💤 Low value

Consider using null() for const pointer.

core::ptr::null() is more idiomatic for *const types than null_mut(), even though both work due to coercion.

♻️ Suggested change
 impl FfiResult for *const c_char {
-    const ERR_VALUE: *const c_char = core::ptr::null_mut();
+    const ERR_VALUE: *const c_char = core::ptr::null();

     fn into_output(self, _vm: &VirtualMachine) -> *const c_char {
         self
     }
 }
🤖 Prompt for 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.

In `@crates/capi/src/util.rs` around lines 79 - 85, The impl of FfiResult for
*const c_char uses core::ptr::null_mut() for ERR_VALUE; change it to
core::ptr::null() to be idiomatic for const pointers (update the const ERR_VALUE
in the impl and leave into_output(self, _vm: &VirtualMachine) -> *const c_char
unchanged) so the null value reflects a *const type.
crates/capi/src/pycapsule.rs (1)

134-149: ⚖️ Poor tradeoff

Test uses pyo3's PyCapsule instead of testing the C-API functions directly.

The test validates capsule behavior through pyo3's wrapper API (using PyCapsule::new_with_value(), is_valid_checked(), pointer_checked()), but does not exercise the C-API functions defined in this file (PyCapsule_New, PyCapsule_GetPointer, PyCapsule_GetName, etc.). Consider adding tests that call the C-API functions directly to verify the implementation.

🤖 Prompt for 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.

In `@crates/capi/src/pycapsule.rs` around lines 134 - 149, The test currently
exercises pyo3's PyCapsule wrapper but not the raw C-API implementations; add
unit tests that directly call the C-API functions (e.g., PyCapsule_New,
PyCapsule_GetPointer, PyCapsule_GetName, PyCapsule_IsValid) to validate your
bindings: create a capsule via PyCapsule_New with a known pointer and name,
assert PyCapsule_GetName returns the expected name, assert PyCapsule_GetPointer
returns the original pointer (and that casting yields the original value), and
test PyCapsule_IsValid for matching and non-matching names; place these tests
alongside the existing ones and use Python::attach to obtain a PyO3 GIL for
calling the unsafe C-API functions.
🤖 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/pycapsule.rs`:
- Around line 41-49: The function PyCapsule_GetContext dereferences the raw
pointer `capsule` without checking for null; update it to first check if
`capsule` is null and return a null pointer on error, then proceed to
dereference and call downcast_ref_if_exact::<PyCapsule>(vm) as before (preserve
the existing with_vm usage and error path). Ensure the function returns a *mut
c_void null when the input is null or when downcast/with_vm fails, and keep
references to symbols PyCapsule_GetContext, with_vm, and
downcast_ref_if_exact::<PyCapsule> to locate the change.
- Around line 51-59: In PyCapsule_SetContext add a null-pointer check for the
incoming capsule before performing unsafe dereference: if capsule.is_null()
return an error (e.g., propagate a failure c_int) and set a Python error via the
VM (use vm.new_value_error or similar) rather than dereferencing; then proceed
to call downcast_ref_if_exact::<PyCapsule> and capsule.set_context(context) only
after the pointer is confirmed non-null.
- Around line 31-39: PyCapsule_GetName dereferences the raw pointer without a
null check which can UB if caller passes null; modify PyCapsule_GetName to
early-return a null C string pointer when capsule.is_null() (mirroring
PyCapsule_GetPointer), i.e., check the raw capsule pointer before the unsafe {
&*capsule } dereference and return std::ptr::null() (or equivalent) on null;
keep the rest of the with_vm flow and the downcast_ref_if_exact::<PyCapsule>(vm)
handling unchanged so valid capsules still resolve to
capsule.name().map(CStr::as_ptr).unwrap_or_default().
- Around line 26-29: The function PyCapsule_GetPointer dereferences capsule
without checking for null; add a null check at the start (if capsule.is_null())
and return std::ptr::null_mut() immediately to avoid undefined behavior, then
call with_vm and checked_capsule using unsafe { &*capsule } only after the null
check; mirror the null-handling pattern used in PyCapsule_IsValid to keep
behavior consistent.
- Around line 82-102: The function PyCapsule_Import must check for a null name
pointer before calling CStr::from_ptr to avoid UB; in PyCapsule_Import add an
initial null check (if name.is_null()) that sets an appropriate Python error via
the VM (e.g., vm.new_system_error or vm.new_import_error) and returns a null
*mut c_void, then proceed with the existing logic that calls CStr::from_ptr,
vm.import, attribute traversal, and checked_capsule; ensure the null-check is
performed before any unsafe dereference and that the function consistently
returns a null pointer when an error is set.
- Around line 61-69: The PyCapsule_SetPointer implementation must validate the
incoming raw pointers before dereferencing: check that the `capsule` (raw *mut
PyObject) is not null and return an error c_int if it is, then inside with_vm
still downcast the referenced object to `PyCapsule` as currently done;
additionally, enforce that the `pointer` argument is not null and return the
CPython-compatible error code when it is (matching other capsule APIs in this
crate). Update the function handling around `PyCapsule_SetPointer`, referencing
the `PyCapsule` type, the `with_vm` wrapper, and the `capsule`/`pointer`
parameters so null checks occur before any unsafe dereference and the function
returns the appropriate c_int error on null inputs.

In `@crates/vm/src/builtins/capsule.rs`:
- Line 17: The field and accessor for the capsule name currently use
Option<&'static CStr> which is too strict; change the capsule struct field
(currently "name: Option<&'static CStr>") to store a raw pointer (Option<*const
c_char>) and adjust the getter (the method returning the name at lines ~30–33
and similar at ~61–63) to return Option<&CStr> with a lifetime tied to &self by
checking for null and performing an unsafe dereference (e.g., convert the
pointer to &CStr inside the getter), ensuring you only assume the pointer is
valid for the capsule's lifetime rather than 'static; update all places
referencing the original &'static CStr type accordingly.

---

Nitpick comments:
In `@crates/capi/src/pycapsule.rs`:
- Around line 134-149: The test currently exercises pyo3's PyCapsule wrapper but
not the raw C-API implementations; add unit tests that directly call the C-API
functions (e.g., PyCapsule_New, PyCapsule_GetPointer, PyCapsule_GetName,
PyCapsule_IsValid) to validate your bindings: create a capsule via PyCapsule_New
with a known pointer and name, assert PyCapsule_GetName returns the expected
name, assert PyCapsule_GetPointer returns the original pointer (and that casting
yields the original value), and test PyCapsule_IsValid for matching and
non-matching names; place these tests alongside the existing ones and use
Python::attach to obtain a PyO3 GIL for calling the unsafe C-API functions.

In `@crates/capi/src/util.rs`:
- Around line 79-85: The impl of FfiResult for *const c_char uses
core::ptr::null_mut() for ERR_VALUE; change it to core::ptr::null() to be
idiomatic for const pointers (update the const ERR_VALUE in the impl and leave
into_output(self, _vm: &VirtualMachine) -> *const c_char unchanged) so the null
value reflects a *const type.
🪄 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: 73238666-03e7-42c7-8749-db5d7d9f8378

📥 Commits

Reviewing files that changed from the base of the PR and between ae6c160 and 9e0f725.

📒 Files selected for processing (5)
  • crates/capi/src/lib.rs
  • crates/capi/src/pycapsule.rs
  • crates/capi/src/util.rs
  • crates/vm/src/builtins/capsule.rs
  • crates/vm/src/vm/context.rs

Comment thread crates/capi/src/pycapsule.rs
Comment thread crates/capi/src/pycapsule.rs
Comment thread crates/capi/src/pycapsule.rs
Comment thread crates/capi/src/pycapsule.rs
Comment thread crates/capi/src/pycapsule.rs
Comment thread crates/capi/src/pycapsule.rs
Comment thread crates/vm/src/builtins/capsule.rs
@bschoenmaeckers bschoenmaeckers marked this pull request as draft May 20, 2026 16:02
Comment thread crates/capi/src/lib.rs
@bschoenmaeckers
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/capi/src/pycapsule.rs (1)

139-154: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Test does not exercise the C-ABI functions defined in this module.

The test uses pyo3's PyCapsule::new_with_value and pyo3 methods (is_valid_checked, pointer_checked), not the PyCapsule_New, PyCapsule_IsValid, or PyCapsule_GetPointer functions implemented above. This leaves the actual C-ABI implementation untested.

Consider calling the exported functions directly to validate the implementation:

#[test]
fn test_capsule_new() {
    // Initialize RustPython VM and call PyCapsule_New, PyCapsule_IsValid, PyCapsule_GetPointer
    // to test the actual C-ABI surface
}
🤖 Prompt for 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.

In `@crates/capi/src/pycapsule.rs` around lines 139 - 154, The test currently uses
pyo3 helpers instead of the module's C-ABI exports; update test_capsule_new to
call the exported functions directly: call PyCapsule_New to create the capsule,
then call PyCapsule_IsValid and PyCapsule_GetPointer to validate and extract the
pointer, cast the returned void* to the original String type and assert the
value matches; ensure you call these functions through the crate's exported
symbols (PyCapsule_New, PyCapsule_IsValid, PyCapsule_GetPointer) inside the
Python::with_gil / Python::attach context and handle unsafe pointer casts
properly.
🤖 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.

Outside diff comments:
In `@crates/capi/src/pycapsule.rs`:
- Around line 139-154: The test currently uses pyo3 helpers instead of the
module's C-ABI exports; update test_capsule_new to call the exported functions
directly: call PyCapsule_New to create the capsule, then call PyCapsule_IsValid
and PyCapsule_GetPointer to validate and extract the pointer, cast the returned
void* to the original String type and assert the value matches; ensure you call
these functions through the crate's exported symbols (PyCapsule_New,
PyCapsule_IsValid, PyCapsule_GetPointer) inside the Python::with_gil /
Python::attach context and handle unsafe pointer casts properly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 94984234-5ce2-47ec-bc1d-9a78e3b61558

📥 Commits

Reviewing files that changed from the base of the PR and between 9e0f725 and 04693e6.

📒 Files selected for processing (1)
  • crates/capi/src/pycapsule.rs

@bschoenmaeckers bschoenmaeckers marked this pull request as ready for review May 22, 2026 12:20
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
crates/capi/src/pycapsule.rs (1)

145-160: ⚡ Quick win

Test exercises pyo3's PyCapsule, not the RustPython C-ABI functions.

This test creates and validates a capsule using pyo3::types::PyCapsule, but it doesn't call any of the PyCapsule_New, PyCapsule_GetPointer, PyCapsule_IsValid, etc. functions defined in this file. Consider adding tests that invoke the actual exported C-ABI functions to verify the RustPython implementation.

🤖 Prompt for 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.

In `@crates/capi/src/pycapsule.rs` around lines 145 - 160, The current test only
uses pyo3's PyCapsule wrapper; add a new test (e.g., test_capsule_c_abi) that
exercises the exported C-ABI functions: call PyCapsule_New to create a capsule
with a C name, then call PyCapsule_IsValid (or PyCapsule_IsValid) to check the
name, call PyCapsule_GetPointer to retrieve the pointer and verify the stored
value, and finally call PyCapsule_SetDestructor or
PyCapsule_SetPointer/appropriate cleanup if needed; use pyo3::ffi
(ffi::PyCapsule_New, ffi::PyCapsule_IsValid, ffi::PyCapsule_GetPointer) with
CString for the name, acquire the GIL via Python::with_gil/attach, check for
null returns and use unsafe casts to compare the pointed-to Rust value, and add
assertions mirroring the existing test to validate the RustPython C-ABI
implementation.
🤖 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.

Nitpick comments:
In `@crates/capi/src/pycapsule.rs`:
- Around line 145-160: The current test only uses pyo3's PyCapsule wrapper; add
a new test (e.g., test_capsule_c_abi) that exercises the exported C-ABI
functions: call PyCapsule_New to create a capsule with a C name, then call
PyCapsule_IsValid (or PyCapsule_IsValid) to check the name, call
PyCapsule_GetPointer to retrieve the pointer and verify the stored value, and
finally call PyCapsule_SetDestructor or PyCapsule_SetPointer/appropriate cleanup
if needed; use pyo3::ffi (ffi::PyCapsule_New, ffi::PyCapsule_IsValid,
ffi::PyCapsule_GetPointer) with CString for the name, acquire the GIL via
Python::with_gil/attach, check for null returns and use unsafe casts to compare
the pointed-to Rust value, and add assertions mirroring the existing test to
validate the RustPython C-ABI implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 6162606a-dd49-4b3f-9d34-c91288c4226f

📥 Commits

Reviewing files that changed from the base of the PR and between 6a9f1a6 and a34acf0.

📒 Files selected for processing (6)
  • .cspell.dict/python-more.txt
  • crates/capi/src/lib.rs
  • crates/capi/src/pycapsule.rs
  • crates/capi/src/util.rs
  • crates/vm/src/builtins/capsule.rs
  • crates/vm/src/vm/context.rs
✅ Files skipped from review due to trivial changes (1)
  • .cspell.dict/python-more.txt

@youknowone youknowone merged commit dcb273b into RustPython:main May 26, 2026
26 checks passed
@bschoenmaeckers bschoenmaeckers deleted the c-api-capsule branch May 27, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants