Skip to content

C API type slots - #8435

Merged
youknowone merged 9 commits into
RustPython:mainfrom
youknowone:c-api-type-slots
Nov 6, 2026
Merged

youknowone merged 9 commits into
RustPython:mainfrom
youknowone:c-api-type-slots

Conversation

@youknowone

@youknowone youknowone commented Aug 3, 2026 •

Copy link
Copy Markdown
Member

@bschoenmaeckers could you check if this is a working design? tried not to increase runtime cost for non-capi path

Copilot AI review requested due to automatic review settings August 3, 2026 00:07
@coderabbitai

coderabbitai Bot commented Aug 3, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

Caution

Review failed

The pull request is closed.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adds initial support for C extension–provided type slots (starting with tp_new) by storing C ABI function pointers in a per-heap-type table and routing RustPython’s slot dispatch through a shared trampoline. It also exposes a minimal C-API surface to install and query these slots, plus tests validating inheritance and safety behavior.

Changes:

  • Introduces a CSlots table (owned by heap types) and a c_new_trampoline to dispatch tp_new supplied from C.
  • Threads c_slots inheritance/identity through PyTypeSlots and uses it in the “is not safe” __new__ check.
  • Adds PyType_GetSlot / Py_tp_new support in crates/capi with tests, and reuses shared C-call utilities in methodobject.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/vm/src/types/slot.rs Adds c_slots tracking on PyTypeSlots, plus new_identity() and inheritance/override logic integration.
crates/vm/src/types/mod.rs Exposes the new c_slots module and re-exports selected C-slot types/trampolines.
crates/vm/src/types/c_slots.rs New module implementing C slot tables, trampolines, and shared argument/return conversion utilities.
crates/vm/src/builtins/type.rs Stores owned C-slot tables on heap types and adjusts tp_new identity comparisons for safety checks.
crates/capi/src/typeobject.rs New C-API glue for installing/querying tp_new via CSlots, with unit tests.
crates/capi/src/methodobject.rs Reuses the shared (args, kwds) split + return conversion helpers for C function calls.
crates/capi/src/lib.rs Registers the new typeobject module.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/vm/src/types/slot.rs Outdated
Comment on lines +911 to +914
let owns_table = self.slots.c_slots.load().is_some()
&& self.slots.c_slots.load()
!= self.base.deref().and_then(|base| base.slots.c_slots.load());
if owns_table {
Comment on lines +296 to 299
/// The C slot table this type owns, once an extension has filled one in.
/// Types that inherit from it point at this same table.
pub c_slots: PyRwLock<Option<OwnedCSlots>>,
pub specialization_cache: TypeSpecializationCache,
Comment thread crates/capi/src/typeobject.rs Outdated
Comment on lines +26 to +29
/// Install a C `newfunc` as the type's tp_new.
///
/// `ty` must be a heap type that does not already define `__new__` itself.
pub fn set_tp_new(vm: &VirtualMachine, ty: &Py<PyType>, tp_new: newfunc) -> PyResult<()> {
Comment thread crates/capi/src/typeobject.rs Outdated
Comment on lines +50 to +62
pub unsafe extern "C" fn PyType_GetSlot(ty: *const PyTypeObject, slot: c_int) -> *mut c_void {
let ty = unsafe { &*ty };
let Some(c_slots) = ty.slots.c_slots() else {
return ptr::null_mut();
};
match CSlotId::from_raw(slot) {
Some(CSlotId::TpNew) => c_slots
.new
.load()
.map_or(ptr::null_mut(), |f| f as *mut c_void),
None => ptr::null_mut(),
}
}

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.

This will not work. As PyType_GetSlot may be called on all types, so this will fail for builtin types. This is also required for extending classes because it requires fetching tp_new of the base type, which is almost always builtins.object.

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

I like the approach but this unfortunately does not work. When constructing heaptypes using the c-api, a type requires the tp_new of the base type. This is most of the time just object which we can fill in a c-slot trivially. To support extending all RustPython defined types we need to define the C tp_new slot for all types. Which might defeats the purpose separate slot definitions. Is there a way to make both C and Rust slots interchangeable?

I tested your approach in this commit bc481ed. Which fails this assert in pyo3: https://github.com/PyO3/pyo3/blob/4df5ac6296fa9c9a375d874082a8ad90e43f9b2d/src/internal/pyclass_init.rs#L50

@youknowone

Copy link
Copy Markdown
Member Author

Thank you for the review!

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

CPython semantics are “call base tp_new with subtype as argument”, not “call subtype’s current tp_new”. So we need a extern "C" tp_new for all types. We might be able to fill in the c slot automatically for all types even when defined in rust?

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

After some experimenting with this, it makes me want FnStatic. This allow us to create a generic C trampoline for all rust defined type slots.

fn get_c_tp_new<F>(
    _: F,
) -> extern "C" fn(*mut PyTypeObject, *mut PyObject, *mut PyObject) -> *mut PyObject
where
    F: FnStatic(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
{
    extern "C" fn trampoline<F>(
        subtype: *mut PyTypeObject,
        args: *mut PyObject,
        kwargs: *mut PyObject,
    ) -> *mut PyObject
    where
        F: FnStatic(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
    {
        unsafe { F::call_static(&*subtype.to_owned(), tuple_to_args(args), dict_to_kwargs(kwargs)) }
    }

    trampoline::<F>
}

 let c_tp_new = ty.slots.new.load().map(|rust_tp_new| get_c_tp_new(rust_tp_new)

I'm not sure how we can do this in todays ✨ rust.

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

In todays rust we may have to use a static dispatch table. What do you think of this solution?

fn get_c_tp_new<F: Any + Send + Sync>(
    rust_tp_new: F,
) -> extern "C" fn(*mut PyTypeObject, *mut PyObject, *mut PyObject) -> *mut PyObject
where
    F: Fn(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
{
    extern "C" fn trampoline<F: Any + Send + Sync>(
        subtype: *mut PyTypeObject,
        args: *mut PyObject,
        kwargs: *mut PyObject,
    ) -> *mut PyObject
    where
        F: Fn(PyTypeRef, FuncArgs, &VirtualMachine) -> PyResult,
    {
        let entry = SLOT_CACHE
            .get(&TypeId::of::<F>())
            .expect("could not find tp_new trampoline");
        let func = entry.downcast_ref::<F>().expect("trampoline type mismatch");
        with_vm(|vm| unsafe {
            let args = FuncArgs::new(
                tuple_to_args((&*args).try_downcast_ref(vm)?),
                kwargs
                    .as_ref()
                    .map(|kwargs| dict_to_kwargs(vm, kwargs.try_downcast_ref::<PyDict>(vm)?))
                    .transpose()?
                    .unwrap_or_default(),
            );
            func((&*subtype).to_owned(), args, vm)
        })
    }

    static SLOT_CACHE: LazyLock<DashMap<TypeId, Box<dyn Any + Send + Sync>>> =
        LazyLock::new(DashMap::new);
    SLOT_CACHE
        .entry(TypeId::of::<F>())
        .or_insert_with(|| Box::new(rust_tp_new));
    trampoline::<F>
}

@youknowone

Copy link
Copy Markdown
Member Author

@bschoenmaeckers Thank you so much! And I am sorry for late response. I am going to try your suggestion without table

@codspeed

codspeed Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing youknowone:c-api-type-slots (2fc0aca) with main (7f314dd)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Comment thread crates/capi/src/typeobject.rs Outdated

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.

Could you move this to the object/pytype.rs file?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Moved crates/capi/src/typeobject.rs into crates/capi/src/object/pytype.rs and dropped the extra typeobject module. Slot APIs are re-exported through object as before.

— commented by Grok 4.6

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

Please limit this to PyType_GetSlot only. I already have code for PySlot & PyType_Slot handling.

Comment thread crates/vm/src/object/c_body.rs Outdated
const ALIGN: usize = 16;

/// Instance payload whose body is a C-owned byte buffer of `basicsize` bytes.
pub struct PyCBody {

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.

I don't think this will work. The last time I checked it is only possible for a type to have 1 Payload type with data in it. So for a C class extending a PyList, it needs both PyCBody & PyList holding data.

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

What is the status of this?

@bschoenmaeckers

Copy link
Copy Markdown
Contributor

@youknowone Do you have time to finish this? Or should I take it over?

youknowone and others added 2 commits November 6, 2026 00:32
A METH_VARARGS | METH_KEYWORDS function received an empty dict when the
call had no keywords, where the calling convention passes NULL. A callee
that rejects keywords by testing the pointer saw a non-NULL kwargs.
The METH_FASTCALL | METH_KEYWORDS path already passed a NULL kwnames.

Name the four function pointer types the PyMethodPointer union holds
instead of spelling out each signature inline.

Assisted-by: Claude
The tp_new slot holds a Rust fn pointer, which a C `newfunc` cannot be.
Put the C function in a `CSlots` table owned by the heap type it belongs
to, and store `c_new_trampoline` in `new`; the trampoline reads the table
off the type it is called with, so a subclass that inherited both reaches
the same function and passes itself as `subtype`.

`PyTypeSlots` holds one 8-byte pointer to the table rather than a field
per C-provided slot, so further slots are a field in `CSlots` and a
trampoline beside this one. The pointer is inherited with `new` by
`set_new` and by `update_one_slot`, and dropped when a Python-level
`__new__` replaces the slot.

Compare what tp_new dispatches to, not the slot, in the "is not safe"
check: every C type shares one trampoline, so comparing `new` alone let
`CBase.__new__(CSub)` run CSub's tp_new.

Move the C call marshalling from capi to `types::c_slots` so the
trampoline and the METH_* call paths share it.

Assisted-by: Claude
PyType_GetSlot reported Py_tp_new only for a function installed from C.
A Rust slot such as int's or object's read as NULL, so an extension that
reads its base type's tp_new to call it with its own subtype had nothing
to call.

Pair each Rust tp_new with a statically instantiated extern "C"
function. `c_new_for::<S>` marshals `(subtype, args, kwds)`, calls
`S::call`, and returns the object or NULL with the exception pending.
`S` is `ViaConstructor<T>` for a `#[pyclass(with(Constructor))]` payload
and `PythonNew` for `new_wrapper`; the target is fixed by the type
parameter, so a subtype passed in runs the base's body.

The derive macro emits a `HasStaticCSlots::TABLE` const per Constructor
class holding the (Rust fn, C fn) pair, and stores a reference to it in
the new `PyTypeSlots::static_c_slots` field on the defining type only.
`PyType::c_tp_new` resolves a C-installed slot from `CSlots`, and a Rust
slot by matching the slot's address against the tables along the MRO,
then against the `new_wrapper` pair.

An empty heap subclass of object now reports object's tp_new rather
than NULL; the round-trip test is updated for that.

Assisted-by: Claude
Assisted-by: Grok
Reproduce what pyo3's PyNativeTypeInitializer does: a heap type whose C
tp_new fetches the base type's tp_new through PyType_GetSlot and calls
it with the subtype it was given. Cover object and int as bases, a
Python-level subclass of the extension type, the slot each type reports,
and a ValueError raised by the base crossing both trampolines.

Replace a map/unwrap_or_else pair in a test helper that clippy flags.

Assisted-by: Claude
Assisted-by: Grok
pyresult_to_ret_ptr stored the error with set_exception, which is the
exc_info slot PUSH_EXC_INFO and POP_EXCEPT maintain. The C side reads the
pending error with raised_exception and take_raised_exception, so a Rust
tp_new reached through c_new_for returned NULL with nothing raised.
Use set_raised_exception, and the same in the test helper that reports a
missing base tp_new.

Assisted-by: Claude
PyType_FromSlots left the Py_tp_new entry as a TODO. Hand it to
set_tp_new once the type exists, so the type instantiates through the
C function, gets the checked __new__ wrapper, and reports the function
from PyType_GetSlot.

A spec test covers those three paths. The pyo3 tests zero_sized_class
and subclass_with_property were ignored; they pass now that
PyType_GetSlot answers Py_tp_new for object and list, so the ignore
markers are removed. rust_class and rust_class_with_member stay ignored
because the spec asks for a non-zero basicsize.

Assisted-by: Claude
set_tp_new now returns TypeError when the type's own dict already has
__new__, matching its documented precondition. HeapTypeExt::c_slots is
narrowed to pub(crate), and update_one_slot loads the type's C slot
table once instead of twice.

Add a capi test for the refusal and for the second installation, and a
pyo3 test that a #[new] taking arguments runs from Python code, raises
its error, and refuses a Python subclass.

Assisted-by: Claude
@youknowone
youknowone marked this pull request as ready for review November 6, 2026 03:31
@youknowone
youknowone merged commit 1dbe198 into RustPython:main Nov 6, 2026
19 of 20 checks passed
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-11-06T03:31:48.512226Z
We couldn’t complete this review. Comment @codex review to try again.
e5d33e4 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@youknowone
youknowone deleted the c-api-type-slots branch November 6, 2026 03:31
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

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