Skip to content

Add call functions to c-api#7937

Merged
youknowone merged 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-call
May 20, 2026
Merged

Add call functions to c-api#7937
youknowone merged 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-call

Conversation

@bschoenmaeckers

@bschoenmaeckers bschoenmaeckers commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Expanded C API object calling interface with new functions supporting multiple invocation patterns. These include standard calls with arguments and keyword arguments, efficient no-argument calls, and optimized vectorcall variants. These additions enable improved performance, better compatibility, and provide developers with flexible mechanisms for object invocation in C-based code.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

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: 559dbb75-ba64-4411-a89f-38b8ef83c027

📥 Commits

Reviewing files that changed from the base of the PR and between 4241c83 and 9530575.

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

📝 Walkthrough

Walkthrough

This PR adds four new C-ABI exported functions (PyObject_Call, PyObject_CallNoArgs, PyObject_Vectorcall, PyObject_VectorcallMethod) to RustPython's C-API layer. These functions enable external C code to invoke Python objects with various calling conventions, converting raw C pointers and argument arrays into VM-compatible argument structures before dispatching through the Rust VM.

Changes

C-API object calling

Layer / File(s) Summary
Argument conversion helpers
crates/capi/src/abstract_.rs
Adds PY_VECTORCALL_ARGUMENTS_OFFSET constant and helper functions that convert Python PyTuple pointers to VM PosArgs and PyDict pointers to KwArgs, including validation that dictionary keys are strings.
Basic object call entry points
crates/capi/src/abstract_.rs
Exports PyObject_Call to invoke callables with tuple args and optional dict kwargs, and PyObject_CallNoArgs to invoke callables with no arguments, both delegating to the VM call interface.
Vectorcall protocol entry points
crates/capi/src/abstract_.rs, .cspell.dict/cpython.txt
Exports PyObject_Vectorcall to dispatch via the vectorcall protocol (decoding nargsf and kwnames into positional/keyword counts) and PyObject_VectorcallMethod to perform method calls (validating receiver presence and extracting it from the argument array before forwarding to vectorcall). Adds nargsf to the spell-check dictionary.

Sequence Diagram

sequenceDiagram
  participant CCaller as External C Code
  participant CallVec as PyObject_Vectorcall
  participant CallMethod as PyObject_VectorcallMethod
  participant VM as Rust VM

  CCaller->>CallVec: callable, args_ptr, nargsf, kwnames
  CallVec->>CallVec: extract pos_count from nargsf (nargsf - offset)
  CallVec->>CallVec: optionally decode kwnames into keyword slice
  CallVec->>CallVec: materialize args_ptr into Rust vector
  CallVec->>VM: vectorcall(callable, posargs, kwnames)
  VM-->>CallVec: PyObject* result
  CallVec-->>CCaller: result

  CCaller->>CallMethod: name, args_ptr, nargsf, kwnames
  CallMethod->>CallMethod: validate args_len > 0 (receiver present)
  CallMethod->>CallMethod: extract receiver from args[0]
  CallMethod->>CallMethod: resolve attribute by name to callable
  CallMethod->>CallVec: call with receiver removed from args
  CallVec->>VM: vectorcall(method, adjusted_args, kwnames)
  VM-->>CallVec: PyObject* result
  CallVec-->>CallMethod: result
  CallMethod-->>CCaller: result
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested Reviewers

  • youknowone
  • ShaharNaveh

Poem

🐰 A vectorcall bloom in the C-API spring,
Four functions now dance and freely ring—
PyObjects called from across the wall,
With args materialized for one and all.
nargsf decoded, the method's receiver found,
RustPython's bindings make not a sound! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.46% 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 call functions to c-api' directly and specifically describes the main change: introducing new C API call functions (PyObject_Call, PyObject_CallNoArgs, PyObject_Vectorcall, PyObject_VectorcallMethod).
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.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 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/abstract_.rs`:
- Around line 50-54: The code constructs a slice from a potentially NULL pointer
when computing args: change the logic in the vectorcall handler so that if
args_len == 0 you directly return an empty Vec, otherwise call unsafe
slice::from_raw_parts(args, args_len) and map/collect as before; specifically
update the block that references args_len, num_positional_args, kwnames and args
to short-circuit on args_len == 0 (mirror the pattern used by
PyObject_VectorcallMethod) to avoid undefined behavior from
slice::from_raw_parts on a NULL pointer.
🪄 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: 3028c37c-9c9d-4595-9896-6b445ccdc4cf

📥 Commits

Reviewing files that changed from the base of the PR and between ae3804f and a07d3dd.

📒 Files selected for processing (2)
  • crates/capi/src/abstract_.rs
  • crates/vm/src/function/argument.rs

Comment thread crates/capi/src/abstract_.rs Outdated
Comment thread crates/vm/src/function/argument.rs Outdated

@coderabbitai coderabbitai Bot left a comment

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.

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/abstract_.rs (1)

91-101: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Missing keyword argument value count in args_len calculation causes out-of-bounds read.

The args_len at line 91 extracts only the positional argument count (nargsf & !PY_VECTORCALL_ARGUMENTS_OFFSET), but per the vectorcall protocol, the raw args array contains both positional arguments and keyword argument values in sequence: [receiver, pos_arg_1, ..., pos_arg_N, kwarg_val_1, ..., kwarg_val_M].

Creating the slice with only the positional count omits the keyword argument values. When the slice pointer is passed to PyObject_Vectorcall along with the original kwnames, that function attempts to read keyword argument values beyond the slice boundary, causing undefined behavior.

The fix is to compute args_len to include both positional and keyword argument value counts, mirroring the calculation in PyObject_Vectorcall at line 68.

Proposed fix: include kwarg values in args_len
-        let args_len = nargsf & !PY_VECTORCALL_ARGUMENTS_OFFSET;
+        let num_positional_args = nargsf & !PY_VECTORCALL_ARGUMENTS_OFFSET;
+
+        let kwnames_len = unsafe {
+            kwnames
+                .as_ref()
+                .map(|tuple| tuple.try_downcast_ref::<PyTuple>(vm).map(|t| t.len()))
+                .transpose()?
+                .unwrap_or(0)
+        };
+
+        let args_len = num_positional_args + kwnames_len;
🤖 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/abstract_.rs` around lines 91 - 101, The slice length
calculation for args_len wrongly uses only positional count (nargsf &
!PY_VECTORCALL_ARGUMENTS_OFFSET), which omits keyword argument values and can
cause out-of-bounds reads when PyObject_Vectorcall expects them; update the
args_len computation to add the keyword value count when kwnames is non-null
(mirror the same calculation used by PyObject_Vectorcall), i.e. derive the
keyword count from nargsf (using PY_VECTORCALL_ARGUMENTS_OFFSET mask) and
include it in args_len before creating the slice for receiver/args so the full
[receiver, pos..., kw_vals...] region is covered.
🤖 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/abstract_.rs`:
- Around line 91-101: The slice length calculation for args_len wrongly uses
only positional count (nargsf & !PY_VECTORCALL_ARGUMENTS_OFFSET), which omits
keyword argument values and can cause out-of-bounds reads when
PyObject_Vectorcall expects them; update the args_len computation to add the
keyword value count when kwnames is non-null (mirror the same calculation used
by PyObject_Vectorcall), i.e. derive the keyword count from nargsf (using
PY_VECTORCALL_ARGUMENTS_OFFSET mask) and include it in args_len before creating
the slice for receiver/args so the full [receiver, pos..., kw_vals...] region is
covered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: ae630f7a-1a5a-4707-8032-1e0a144ef418

📥 Commits

Reviewing files that changed from the base of the PR and between 6d47176 and 4241c83.

📒 Files selected for processing (2)
  • crates/capi/src/abstract_.rs
  • crates/vm/src/function/argument.rs
✅ Files skipped from review due to trivial changes (1)
  • crates/vm/src/function/argument.rs

@bschoenmaeckers

Copy link
Copy Markdown
Contributor Author

@coderabbitai your last comment is not correct. Because the pointer to pos_arg_1 points to the start of the array, and the array still contains the keywords arguments after it. It does not patter we do not count the kwargs in the slice.

@coderabbitai

coderabbitai Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat crates/capi/src/abstract_.rs

Length of output: 5025


@bschoenmaeckers You're absolutely right, and I apologize for the incorrect comment. Looking at the code again:

In PyObject_VectorcallMethod, the slice is only created over the positional-arg count in order to call .split_first() and peel off the receiver:

let (receiver, args) = unsafe { slice::from_raw_parts(args, args_len) }
    .split_first()
    .expect("args_len > 0 should guarantee a receiver");

The resulting args.as_ptr() still points into the original contiguous buffer — keyword argument values are sitting right after the positional ones in that same allocation (as mandated by the vectorcall convention). When that pointer is forwarded to PyObject_Vectorcall, it recomputes the full length as num_positional_args + kwnames.len() and reads the complete buffer (positional + kwarg values) correctly. The intermediate short slice is only ever used for the split, so nothing is lost. The code is correct as written.

(ミ・ω・ミ)


✏️ Learnings added
Learnt from: bschoenmaeckers
URL: https://github.com/RustPython/RustPython/pull/7937

Timestamp: 2026-05-20T14:27:24.682Z
Learning: In RustPython's C-API vectorcall implementation (`crates/capi/src/abstract_.rs`), `PyObject_VectorcallMethod` creates a slice over only the positional-argument count solely to extract the receiver via `split_first()`. The resulting pointer (`args.as_ptr()`) still addresses the full contiguous buffer (positional + kwarg values). When forwarded to `PyObject_Vectorcall`, the full `args_len` (positional + kwnames count) is recomputed there and the complete buffer is read correctly. The short intermediate slice is intentional and correct.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@youknowone youknowone merged commit ae6c160 into RustPython:main May 20, 2026
26 checks passed
@bschoenmaeckers bschoenmaeckers deleted the c-api-call branch May 20, 2026 15:23
@coderabbitai coderabbitai Bot mentioned this pull request Jun 11, 2026
2 tasks
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