Draft: explore a RustPython-owned ABI facade for pristine PyO3#7647
Draft: explore a RustPython-owned ABI facade for pristine PyO3#7647smarcd wants to merge 83 commits intoRustPython:mainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Why are you sidestepping my MR. Without participating in the api discussion there? |
This is a PR taking your ideas and running with them. |
| #[unsafe(export_name = "PyBaseObject_Type")] | ||
| pub static mut PYBASEOBJECT_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyBool_Type")] | ||
| pub static mut PYBOOL_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyByteArray_Type")] | ||
| pub static mut PYBYTEARRAY_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyBytes_Type")] | ||
| pub static mut PYBYTES_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyDict_Type")] | ||
| pub static mut PYDICT_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyList_Type")] | ||
| pub static mut PYLIST_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyLong_Type")] | ||
| pub static mut PYLONG_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyModule_Type")] | ||
| pub static mut PYMODULE_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyTuple_Type")] | ||
| pub static mut PYTUPLE_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyType_Type")] | ||
| pub static mut PYTYPE_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); | ||
| #[unsafe(export_name = "PyUnicode_Type")] | ||
| pub static mut PYUNICODE_TYPE_HANDLE: *mut PyTypeObject = ptr::null_mut(); |
There was a problem hiding this comment.
As discussed in my PR, this will not work.
There was a problem hiding this comment.
Removed, replaced with f67a934#diff-6cb426843cb08f1943a128f6df3e3e46e3862a5eae416e33d45a079529eab6b2
smarcd
left a comment
There was a problem hiding this comment.
Two high-confidence issues from a focused review of the new crates/capi facade (the rest looks consistent with the draft/exploration intent).
| #[unsafe(no_mangle)] | ||
| pub extern "C" fn PyObject_CallMethodObjArgs( | ||
| receiver: *mut PyObject, | ||
| name: *mut PyObject, | ||
| arg1: *mut PyObject, | ||
| arg2: *mut PyObject, | ||
| arg3: *mut PyObject, | ||
| arg4: *mut PyObject, | ||
| arg5: *mut PyObject, | ||
| arg6: *mut PyObject, | ||
| arg7: *mut PyObject, | ||
| arg8: *mut PyObject, | ||
| ) -> *mut PyObject { | ||
| let raw = [arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8]; | ||
| let nargs = raw.iter().position(|arg| arg.is_null()).unwrap_or(raw.len()); | ||
| RustPython_PyObject_CallMethodObjArgsArray(receiver, name, raw.as_ptr(), nargs) | ||
| } |
There was a problem hiding this comment.
Duplicate exported symbol PyObject_CallMethodObjArgs — link error / ABI corruption.
This Rust definition collides with the variadic C definition in crates/capi/csrc/pyobject_callmethodobjargs.c#L13-L20. Both have external linkage with the same name (Rust uses #[unsafe(no_mangle)], C is non-static), both end up in the same rustpython-capi rlib (the C file is compiled by cc::Build in crates/capi/build.rs), and lib.rs even adds #[used] static KEEP_PYOBJECT_CALL_METHOD_OBJ_ARGS to force this Rust version to be retained.
Most linkers (gnu-ld with --whole-archive, lld in cdylib mode, etc.) will fail with a multiple-definition error. On macOS the linker can silently pick one — and the two signatures are not interchangeable: the C entry is variadic with a NULL sentinel, this Rust entry takes 8 fixed *mut PyObject slots. If the Rust version wins, real CPython callers that pass more than 8 args (or that pass the NULL terminator) get garbage in the trailing slots; if the C version wins, this Rust function is dead code.
Since the C wrapper already implements the variadic ABI and just dispatches to RustPython_PyObject_CallMethodObjArgsArray, the simplest fix is to delete this Rust function (and the matching #[used] static in lib.rs).
| let args = unsafe { slice::from_raw_parts(args, args_len) } | ||
| .iter() | ||
| .map(|arg| unsafe { &*resolve_object_handle(*arg) }.to_owned()) | ||
| .collect::<Vec<_>>(); | ||
|
|
There was a problem hiding this comment.
Vectorcall drops keyword-argument values — wrong result / runtime panic when any kwargs are passed.
This slice reads only args_len (positional count) entries from the C-supplied args buffer. CPython's vectorcall ABI requires that when kwnames is non-NULL the buffer holds nargs + len(kwnames) entries — positional values at args[0..nargs] and keyword values at args[nargs..nargs+nkwargs]. See Python docs / vectorcall:
If kwnames is not NULL, it must be a tuple of strings naming the keyword arguments. The values of these arguments are stored in args after the positional arguments.
Downstream, FuncArgs::from_vectorcall_owned does args.drain(nargs..nargs + kw_count) (see crates/vm/src/function/argument.rs#L177-L186), which will index past the end of this truncated Vec whenever kwnames is non-empty (debug-asserts in debug, OOB drain panic in release). Any vectorcall through this facade with keyword arguments is broken.
PyObject_VectorcallMethod at L167-L172 builds its own slice the same way, but since it then re-enters this function through args.as_ptr() (still pointing into the original C buffer where the kwarg values live), fixing only PyObject_Vectorcall is enough: resolve kwnames length first and slice args_len + kwnames_len from the raw pointer.

Summary
This PR explores a RustPython-owned ABI facade in
crates/capiso that pristine or near-pristine PyO3 can work against RustPython without requiring a large RustPython-specific backend split inside PyO3.This direction is explicitly informed by:
Current verified status
pyo3 0.28.xcompiles againstcrates/capirpds: package-owned tests greenjiter: package-owned tests greenblake3: package-owned tests green except thenumpy-dependent caseScope and claims
This is not claiming:
What it does claim is:
Reviewer asks
crates/capibe the preferred direction relative to#7562?