Add basic dict function to c-api by bschoenmaeckers · Pull Request #7929 · RustPython/RustPython

bschoenmaeckers · 2026-05-19T19:13:49Z

Summary by CodeRabbit

New Features
- Added C-compatible dictionary support: create dictionaries, insert and retrieve items, query size, and iterate entries via the public C API.
- Exposed a runtime dictionary iteration helper to support efficient entry traversal for consumers.
- Extended the public API surface to include the dictionary module for C-compatible integrations.

coderabbitai · 2026-05-19T19:14:05Z

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: 7e4eb20b-84d7-498a-9dbe-1db5909414a0

📥 Commits

Reviewing files that changed from the base of the PR and between d407549 and a7c5bdc.

📒 Files selected for processing (2)

crates/capi/src/dictobject.rs
crates/vm/src/builtins/dict.rs

📝 Walkthrough

Walkthrough

Adds a new capi::dictobject module exposing C-API functions for dict creation, set/get, size, and iteration; exposes the module from capi::lib; and adds a PyDict::next_entry method and widens inner_setitem visibility to support iteration and C-API operations.

Changes

Dictionary C-API Surface

Layer / File(s)	Summary
PyDict iteration and visibility change `crates/vm/src/builtins/dict.rs`	Adds `pub fn next_entry(&self, position: usize) -> Option<(usize, PyObjectRef, PyObjectRef)>` and changes `inner_setitem` visibility from `pub(crate)` to `pub`.
Dictionary C-API operations and type checking `crates/capi/src/dictobject.rs`	Registers dict type-check helpers and implements exported `extern "C"` functions: `PyDict_New`, `PyDict_SetItem`, `PyDict_GetItemRef`, `PyDict_Size`, and `PyDict_Next`. Includes a disabled pyo3 test module.
Module public export `crates/capi/src/lib.rs`	Adds `pub mod dictobject;` to publicly expose the new C-API module from the crate root.

Sequence Diagram(s)

sequenceDiagram
  participant ComponentA
  participant ComponentB
  ComponentA->>ComponentB: observable interaction

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

RustPython/RustPython#7871: Adds the define_py_check! macro and related object helpers used by the new dict type-check code.

Suggested reviewers

youknowone
ShaharNaveh

Poem

🐰 I hop in code where dictionaries dwell,
Gates of C open, pointers to tell,
New, set, and get — I nibble each key,
Size and next let me bound merrily,
Hooray, I dance through entries with glee!

🚥 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 'Add basic dict function to c-api' accurately describes the main change: introducing fundamental dictionary operations to the C-API with PyDict_New, PyDict_SetItem, PyDict_GetItemRef, PyDict_Size, and PyDict_Next.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% 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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

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

Actionable comments posted: 2

🧹 Nitpick comments (1)

crates/capi/src/dictobject.rs (1)

51-53: 💤 Low value

Remove redundant null assignment.

The result pointer is already set to null on line 41, making this assignment redundant.

♻️ Simplify by removing redundant assignment

         } else {
-            unsafe {
-                *result = core::ptr::null_mut();
-            }
             Ok(false)
         }

🤖 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/dictobject.rs` around lines 51 - 53, The assignment inside
the unsafe block that sets *result = core::ptr::null_mut() is redundant because
result is already initialized to null earlier; remove the unsafe block (or at
least the single statement) that dereferences and writes to result to avoid the
duplicate null assignment and unnecessary unsafe usage, referencing the variable
name result and the unsafe block surrounding the assignment so you can locate
and delete it safely.

🤖 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/dictobject.rs`:
- Around line 96-133: The test module in dictobject.rs is disabled via
#[cfg(false)] so tests like test_create_empty_dict, test_create_dict_with_items,
and test_dict_iter never run; either remove the #[cfg(false)] and adapt these
pyo3-based tests (Python::attach, PyDict, PyInt) to the project's testing
harness (e.g., convert them to use rustpython_vm::vm::thread APIs used in
pystate.rs), or if pyo3 usage is inappropriate delete the test module or add a
clear TODO comment explaining why they remain disabled and when/how they will be
enabled; update the tests to reference the project's VM entry points and types
or remove them to avoid misleading dead code.

---

Nitpick comments:
In `@crates/capi/src/dictobject.rs`:
- Around line 51-53: The assignment inside the unsafe block that sets *result =
core::ptr::null_mut() is redundant because result is already initialized to null
earlier; remove the unsafe block (or at least the single statement) that
dereferences and writes to result to avoid the duplicate null assignment and
unnecessary unsafe usage, referencing the variable name result and the unsafe
block surrounding the assignment so you can locate and delete it safely.

🪄 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: db25f9b1-5cfb-43e0-b57f-a7caaa293249

📥 Commits

Reviewing files that changed from the base of the PR and between 62b081b and cdd45cf.

📒 Files selected for processing (2)

crates/capi/src/dictobject.rs
crates/capi/src/lib.rs

bschoenmaeckers · 2026-05-20T07:58:38Z

@coderabbitai resume

coderabbitai · 2026-05-20T07:59:05Z

✅ Actions performed

Reviews resumed.

coderabbitai

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/dictobject.rs`:
- Around line 26-30: The dict operations currently call the generic mapping
hooks via PyDict::set_item and get_item_opt inside with_vm (symbols: with_vm,
PyDict, set_item, get_item_opt), which can trigger subclass
__setitem__/__missing__; change these to call the dict-exact-storage helpers
that mirror CPython C-API semantics (PyDict_SetItem / PyDict_GetItemRef) on
rustpython_vm::builtins::PyDict so they bypass subclass hooks; locate the call
sites in dictobject.rs (the blocks that dereference dict/key/val and call
dict.set_item(...) and the analogous get_item_opt usages) and replace them with
the helper functions on PyDict that operate on underlying storage directly,
preserving error handling and VM context.

🪄 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: 36a3dc17-183f-4dcd-8d87-82e3e1d175ec

📥 Commits

Reviewing files that changed from the base of the PR and between da1777d and d407549.

📒 Files selected for processing (3)

crates/capi/src/dictobject.rs
crates/capi/src/lib.rs
crates/vm/src/builtins/dict.rs

✅ Files skipped from review due to trivial changes (1)

crates/capi/src/lib.rs

coderabbitai Bot reviewed May 19, 2026

View reviewed changes

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

Comment thread crates/capi/src/dictobject.rs

bschoenmaeckers marked this pull request as draft May 19, 2026 19:25

bschoenmaeckers marked this pull request as ready for review May 20, 2026 07:45

bschoenmaeckers added 2 commits May 20, 2026 15:20

Add basic dict function to c-api

c6a0a23

Fix iter

d407549

bschoenmaeckers force-pushed the c-api-dict branch from da1777d to d407549 Compare May 20, 2026 13:20

coderabbitai Bot reviewed May 20, 2026

View reviewed changes

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

youknowone approved these changes May 20, 2026

View reviewed changes

Do not use mapping protocol

a7c5bdc

youknowone merged commit a136f90 into RustPython:main May 21, 2026
26 checks passed

bschoenmaeckers deleted the c-api-dict branch May 21, 2026 06:29

coderabbitai Bot mentioned this pull request Jun 1, 2026

Use PyO3 main & enable c-api tests #8000

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic dict function to c-api#7929

Add basic dict function to c-api#7929
youknowone merged 3 commits into
RustPython:mainfrom
bschoenmaeckers:c-api-dict

bschoenmaeckers commented May 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 19, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Review ran into problems

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

bschoenmaeckers commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bschoenmaeckers commented May 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Review ran into problems

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

bschoenmaeckers commented May 20, 2026

Uh oh!

coderabbitai Bot commented May 20, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bschoenmaeckers commented May 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 19, 2026 •

edited

Loading