Skip to content

Add capsule support to c-api#7940

Open
bschoenmaeckers wants to merge 4 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-capsule
Open

Add capsule support to c-api#7940
bschoenmaeckers wants to merge 4 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 operations to create, validate, import, and get/set capsule pointer, name, and context.
    • Publicly exposed the pycapsule module for external use.
  • Tests
    • Added unit tests for capsule creation, validation, and pointer retrieval.
  • Chores
    • Updated spell dictionary with "pycapsule".

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

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: 097c999b-1137-429a-8b31-53baf476a2cc

📥 Commits

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

📒 Files selected for processing (2)
  • crates/capi/src/pycapsule.rs
  • crates/vm/src/builtins/capsule.rs

📝 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 57.89% 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' accurately and concisely describes the main change: adding PyCapsule FFI support to the C-API layer.
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
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.

2 participants