Reuse stored dict hashes when merging call keywords by youknowdot · Pull Request #9053 · RustPython/RustPython

youknowdot · 2026-10-10T10:27:23Z

Summary

Extract the exact-dict keyword-merge cached-hash path from #8954 (43065e79ddc46eafeca58cb39398ac007b5d25f7) onto Python 3.14 main (3af5f8a15314446d2b28a90e429c938dc7b8beeb).

Snapshot each key, value and stored hash under one read lock, then use the existing fallible known-hash containment/insertion helpers after unlocking. Expanding an exact dictionary into a call must not invoke its keys' __hash__ again during this merge. Equality errors such as RuntimeError also reach Python instead of the old containment unwrap.

The exact-dict guard, mapping fallback, snapshot order and duplicate detection stay in place. Only crate-private snapshot accessors are added. References: CPython 3.14.7 DICT_MERGE and stored-hash dictionary merge.

Merged #8858 covers other dictionary hash-reuse paths, not frame.rs. Merged #8957 covers native keyword dispatch. Both remain intact, and open #8951 does not contain this extraction.

Validation

Exact CPython 3.14.7 and this native head pass the complete updated syntax_function_args.py snippet; its native pytest entry passes.
A harmless baseline counter records two extra keyword hashes on unchanged main, versus zero on CPython and this head. Raising callbacks were deliberately not run against the old unwrap path.
Four focused existing-snippet scenarios cover one-shot string-key hashing, duplicate detection, non-string validation, and equality exception identity.
Existing canonical test_extcall doctest passes unchanged.
Independent source audit checked key/hash identity, owned snapshots, lock release and private API scope.
Normal repository hooks, VM Clippy and separate C-API Clippy pass. The pre-existing snippet's AST is unchanged; only the reviewed function and invocation were appended.

This is stored-hash reuse, not a rewrite of every kwargs error or mutation rule. Existing snapshot-versus-live-source differences remain. CPython specially formats equality-raised AttributeError and single-argument KeyError; this narrow path propagates them directly instead of the previous panic, and complete formatter parity remains separate work. No canonical tests, README wording, implementation-name guards or Rust behavioral tests changed.

Local validation is bounded Linux validation, not a complete workspace or cross-platform claim. Full hosted CI is pending; this remains a draft for review.

AI-assisted implementation and validation with OpenAI Codex; model identifier unavailable. No human review is implied.

Summary by CodeRabbit

Bug Fixes
- Improved keyword-argument dictionary expansion: duplicate keywords and non-string keys continue to raise TypeError, while errors raised during key comparison are preserved.
- Avoided unnecessarily rehashing string-subclass keys during expansion.

Assisted-by: OpenAI Codex:model identifier unavailable

coderabbitai · 2026-10-10T10:27:31Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: RustPython/RustPython/.coderabbit.yml
Review profile: CHILL
Plan: Advanced
Run ID: 4f3c9ae5-d434-449f-9d8f-675043a74ee1

📥 Commits

Reviewing files that changed from the base of the PR and between 3af5f8a and 749e4ca.

📒 Files selected for processing (4)

crates/vm/src/builtins/dict.rs
crates/vm/src/dict_inner.rs
crates/vm/src/frame.rs
extra_tests/snippets/syntax_function_args.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

Exact-dictionary keyword merging now uses stored key hashes for duplicate checks and insertion. New dictionary helpers return entries with their stored hashes. Tests cover string-subclass keys, duplicate keywords, non-string keys, and exceptions during key comparison.

Changes

Keyword dictionary merging

Layer / File(s)	Summary
Dictionary snapshots with stored hashes `crates/vm/src/dict_inner.rs`, `crates/vm/src/builtins/dict.rs`	Added methods that return live dictionary entries with their stored hashes. The `Dict::insert` documentation lists `items_with_hashes` as a source of key/hash pairs.
Use stored hashes during keyword merging `crates/vm/src/frame.rs`, `extra_tests/snippets/syntax_function_args.py`	The exact-dictionary `DICT_MERGE` path uses stored hashes for duplicate checks and insertion. Tests cover hash preservation and error handling.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 749e4

No actionable merge-blocking issue is identified. Complete the pending CI checks before merging.

Pre-merge checks | 4 | 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: reusing stored dictionary hashes during call keyword merging.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 3 files. (1 skipped: 1 too large.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

chatgpt-codex-connector · 2026-10-10T11:08:50Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-10T11:12:10.426180Z	`749e4ca`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 749e4ca244

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-10T11:12:07Z

    f(1,2,3,4,5,6)
+
+
+def test_keyword_dict_stored_hashes():


Remove the newly added test body

This function adds test logic, test data, and assertions to an existing test file, but this repository explicitly limits test-file edits to adding expected-failure decorators or removing those decorators and their TODOs. This addition therefore cannot land under the repository's test-modification policy; remove it or obtain an explicit policy exception before landing.

AGENTS.md reference: AGENTS.md:L273-L279

Useful? React with 👍 / 👎.

Reuse exact dictionary hashes when merging call keywords

749e4ca

Assisted-by: OpenAI Codex:model identifier unavailable

youknowone approved these changes Oct 10, 2026

View reviewed changes

youknowone marked this pull request as ready for review October 10, 2026 11:08

youknowone enabled auto-merge (squash) October 10, 2026 11:08

chatgpt-codex-connector Bot reviewed Oct 10, 2026

View reviewed changes

youknowone merged commit b63c3b0 into RustPython:main Oct 10, 2026
21 checks passed

Navigation Menu

Repository navigation

Reuse stored dict hashes when merging call keywords - #9053

Reuse stored dict hashes when merging call keywords#9053
youknowone merged 1 commit into
RustPython:mainfrom
youknowdot:fix-keyword-dict-cached-hashes

youknowdot commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 inconclusive)

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

youknowdot commented Oct 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 inconclusive)

Uh oh!

chatgpt-codex-connector Bot commented Oct 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Oct 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

youknowdot commented Oct 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 10, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 10, 2026 •

edited

Loading