GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add more dict functions to c-api by bschoenmaeckers · Pull Request #8269 · RustPython/RustPython

Conversation

@bschoenmaeckers

@bschoenmaeckers bschoenmaeckers commented Jul 13, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Expanded dictionary support in the C API with clear, set, get, and delete operations.
    • Added UTF-8 char* key variants for string-key interop, including a reference-style lookup with a hit/miss success flag.
    • Introduced error-aware vs raw lookups, with clearer behavior for misses and propagated errors.

  • Bug Fixes

    • Improved handling of invalid UTF-8 keys by raising appropriate Python decoding exceptions.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bschoenmaeckers
Add more dict functions to c-api
03ed6cf
@coderabbitai

coderabbitai Bot commented Jul 13, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d58b2bcd-8754-4a23-b587-9523fb35baa3

📥 Commits

Reviewing files that changed from the base of the PR and between 60e37fd and 3db5877.

📒 Files selected for processing (1)
  • crates/capi/src/dictobject.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/capi/src/dictobject.rs
📝 Walkthrough

Walkthrough

The dictionary C-API gains clear, string-key set/delete, object-key lookup, and string-key lookup functions. String keys are decoded from C strings, with invalid UTF-8 handled according to each API’s return contract.

Changes

Dictionary C-API

Layer / File(s) Summary
Dictionary clearing and string-key mutation
crates/capi/src/dictobject.rs		 Adds PyDict_Clear, PyDict_SetItemString, and PyDict_DelItemString, including UTF-8 decoding for string keys.
Object-key lookup
crates/capi/src/dictobject.rs		 Adds PyDict_GetItem and PyDict_GetItemWithError with distinct missing-entry and error behavior.
String-key lookup
crates/capi/src/dictobject.rs		 Adds PyDict_GetItemString and PyDict_GetItemStringRef, including raw-pointer results, out-parameter initialization, and invalid UTF-8 handling.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

  • RustPython/RustPython#7929: Adds related dictionary C-API entry points in the same dictobject.rs module.
  • RustPython/RustPython#8202: Adds a related C-API delete-by-UTF-8-string entry point with UTF-8 decoding behavior.

Suggested reviewers: youknowone

🚥 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 accurately summarizes the main change: adding additional dict functions to the C API.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
coderabbitai[bot]
coderabbitai Bot reviewed Jul 13, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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/dictobject.rs`:
- Around line 90-110: Update PyDict_GetItemString so CStr::to_str() failures
propagate an appropriate decode exception through the existing with_vm result
instead of returning Ok(null_mut()). Preserve Ok(None) as the lookup-miss path,
but stop swallowing errors from dict.inner_getitem_opt; only lookup misses
should return a null pointer without an exception.
🪄 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: 8701ac58-1d3c-4a57-b681-83a4a95747cd

📥 Commits

Reviewing files that changed from the base of the PR and between a9c2c52 and 03ed6cf.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread crates/capi/src/dictobject.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bschoenmaeckers
Review
3db5877
@bschoenmaeckers
bschoenmaeckers force-pushed the c-api-final-dict branch from 60e37fd to 3db5877 Compare July 13, 2026 14:35
@bschoenmaeckers
bschoenmaeckers marked this pull request as draft July 14, 2026 21:24
@bschoenmaeckers
bschoenmaeckers marked this pull request as ready for review July 14, 2026 21:24
youknowone
youknowone approved these changes Jul 15, 2026
View reviewed changes

@youknowone youknowone left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@youknowone
youknowone merged commit 859516c into RustPython:main Jul 15, 2026
26 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bschoenmaeckers
bschoenmaeckers deleted the c-api-final-dict branch July 15, 2026 14:46
@coderabbitai coderabbitai Bot mentioned this pull request Jul 15, 2026
Add codecs support to c-api #8267
Merged
youknowone pushed a commit that referenced this pull request Sep 16, 2026
@bschoenmaeckers
Add more dict functions to c-api (#8269)
7f81b24 
* Add more dict functions to c-api

* Review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@youknowone youknowone youknowone approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@bschoenmaeckers @youknowone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.