GeezerVison

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

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

Conversation

bschoenmaeckers commented Jun 25, 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
- Extended the C API with additional mapping helpers: mapping checks, getting/setting items by UTF-8 string keys, and computing mapping length.
- Added optional lookup functions that return a not-found result (instead of raising) when a key is missing.
- Added key-existence checks with *WithError variants for precise failure handling, including defined behavior for invalid UTF-8 input.

Sorry, something went wrong.

Uh oh!

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

All reactions

coderabbitai Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

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

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: 66c089b5-3beb-4651-9ead-33cbebd09f90

📥 Commits

Reviewing files that changed from the base of the PR and between 3b5e313 and 94cee7f.

📒 Files selected for processing (1)

crates/capi/src/abstract_/mapping.rs

🚧 Files skipped from review as they are similar to previous changes (1)

crates/capi/src/abstract_/mapping.rs

📝 Walkthrough

Walkthrough

Adds exported mapping C-API wrappers for check, length, lookup, key-existence, and string-key assignment. String-key paths decode C strings as UTF-8, convert invalid keys into VM ValueErrors, and map KeyError to not-found results in the optional and existence variants.

Changes

Mapping C-API additions

Layer / File(s)	Summary
Check and length wrappers `crates/capi/src/abstract_/mapping.rs`	Adds `PyMapping_Check` and `PyMapping_Length`, plus the `core::ffi` imports needed for the new exported wrappers.
Lookup, existence, and assignment wrappers `crates/capi/src/abstract_/mapping.rs`	Adds string-key item access, optional item retrieval, key-existence checks, and string-key assignment, including UTF-8 decoding and `KeyError` handling.

Estimated review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

RustPython/RustPython#8003: Adds earlier PyMapping_* C-API surface in the same file; this PR extends that mapping API with additional wrappers.

Suggested reviewers

youknowone

Poem

A rabbit found a mapping key 🐇
and nibbled UTF-8 with glee.
When KeyError hopped out of sight,
the wrapper said, “not found is right.”
Then check and length went bounding free.

🚥 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 clearly matches the main change by describing the addition of more mapping functions to the C API.
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.

_{✏️ 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.}

All reactions

Sorry, something went wrong.

Uh oh!

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

coderabbitai Bot reviewed

Jun 25, 2026

View reviewed changes

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/abstract_/mapping.rs`:
- Around line 137-140: PyMapping_HasKeyString currently propagates invalid UTF-8
as a ValueError via the CStr::from_ptr(...).to_str() conversion in mapping.rs,
which violates the C API contract. Update the PyMapping_HasKeyString path to
treat UTF-8 decode failures as a silent miss by returning Ok(false) immediately
instead of mapping the error into vm.new_value_error, while keeping the existing
obj.get_item(key, vm).is_ok() behavior for valid keys.

🪄 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: b25ba3c3-1271-4bac-aca3-25878e574cad

📥 Commits

Reviewing files that changed from the base of the PR and between 870ad2d and 3b5e313.

📒 Files selected for processing (1)

crates/capi/src/abstract_/mapping.rs

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

crates/capi/src/abstract_/mapping.rs Resolved

Uh oh!

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


          Add more mapping functions to c-api

94cee7f

bschoenmaeckers force-pushed the c-api-more-mapping branch from 3b5e313 to 94cee7f Compare

June 25, 2026 12:05

ShaharNaveh approved these changes

Jun 25, 2026

View reviewed changes

ShaharNaveh 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.

🚀

Sorry, something went wrong.

Uh oh!

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

All reactions

youknowone approved these changes

Jun 25, 2026

View reviewed changes

youknowone merged commit c876709 into RustPython:main

Jun 25, 2026

26 checks passed

Uh oh!

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

bschoenmaeckers deleted the c-api-more-mapping branch

June 25, 2026 13:35

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


          Add more mapping functions to c-api (#8169)

473ba73

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

youknowone

youknowone approved these changes

ShaharNaveh

ShaharNaveh 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.

3 participants

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.

GeezerVison

Conversation

bschoenmaeckers commented Jun 25, 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 Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ShaharNaveh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bschoenmaeckers commented Jun 25, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

coderabbitai Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

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