Skip to content

Add more abstract functions to c-api#8202

Open
bschoenmaeckers wants to merge 2 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-more-abstract
Open

Add more abstract functions to c-api#8202
bschoenmaeckers wants to merge 2 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-more-abstract

Conversation

@bschoenmaeckers

@bschoenmaeckers bschoenmaeckers commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Expanded C API support for invoking Python callables with positional and keyword arguments, including vectorcall-style dispatch.
    • Added C API helpers for deleting items by UTF-8 string keys and formatting objects.
    • Exposed object length and type lookup via the C API.
  • Bug Fixes
    • Improved handling of UTF-8 key strings during item deletion, returning errors for invalid text.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Six new C-ABI exports are added in crates/capi/src/abstract_.rs: callable invocation, vectorcall dispatch, UTF-8 key deletion, formatting, length, and type retrieval. The file also gains core::ffi imports for C string and integer handling.

Changes

C-ABI abstract protocol exports

Layer / File(s) Summary
FFI imports and call wrappers
crates/capi/src/abstract_.rs
Adds CStr, c_char, and c_int imports. Implements PyObject_CallObject and PyVectorcall_Call for callable dispatch from the C-ABI.
DelItemString, Format, Length, and Type exports
crates/capi/src/abstract_.rs
Adds PyObject_DelItemString, PyObject_Format, PyObject_Length, and PyObject_Type exports.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • youknowone

Poem

\o/ A bunny peeks through ABI doors,
Callables hop and vectorcall soars.
UTF-8 keys and formats gleam,
Length and type join the stream.
Thump-thump — six exports on the floors!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adding additional abstract C-API functions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
✨ 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.

@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: 2

🤖 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 191-199: PyObject_Format currently dereferences format_spec
unconditionally, which crashes when callers pass NULL. Update the
PyObject_Format function to detect a null format_spec and treat it as an empty
PyStr before calling try_downcast_ref::<PyStr>(vm), while leaving the obj
handling and vm.format call unchanged.
- Around line 139-147: PyVectorcall_Call is currently routed through
PyObject_Call, which bypasses the vectorcall fast path and can recurse when used
as the callable’s call bridge. Update PyVectorcall_Call in abstract_.rs to
dispatch via the vectorcall entrypoint instead of the normal call slot, using
the existing vectorcall-related helpers/types in this module so the tp_call
bridge preserves vectorcall behavior.
🪄 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: fc3c192e-d650-46b7-b380-4fa319477111

📥 Commits

Reviewing files that changed from the base of the PR and between 33c0cf7 and 662e26b.

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

Comment thread crates/capi/src/abstract_.rs
Comment thread crates/capi/src/abstract_.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.

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 225-228: In PyObject_Format within abstract_.rs, the current
try_downcast_ref::<PyStr> path returns a generic downcast error for non-string
format_spec values, but this C API should map that case to SystemError to match
CPython. Update the handling around format_spec.as_ref() so non-str specs are
explicitly translated into a SystemError before continuing, while keeping the
existing empty_str fallback for None.
🪄 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: 273e1473-5959-4bf5-9667-93d87a2fb932

📥 Commits

Reviewing files that changed from the base of the PR and between 662e26b and 4c4cc0d.

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

Comment thread crates/capi/src/abstract_.rs
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.

1 participant