Track live entries in reverse dictionary iterators by youknowdot · Pull Request #9046 · RustPython/RustPython

youknowdot · 2026-10-10T09:35:09Z

Summary

Extract the Python 3.14-compatible reverse-dictionary iterator fixes from #8954:

Count remaining live entries rather than storage slots.
Handle same-size clear/restore without a spurious size-change error.
Preserve 3.14's deferred exhaustion and mutation checks after the last yield.

Do not import the megaPR's newer extra-yield error or cursor clamp. Explicit wrapping arithmetic preserves the older unsigned hint contract without Rust underflow; existing hint consumers reject out-of-range values before allocation.

Add a concise reverse-hint/last-yield regression beside existing dictionary snippet coverage. Remove only the canonical clear/restore expected-failure decorator after a native unexpected pass. No canonical assertions or unrelated markers change.

Validation

Independent CPython 3.14.7 source/behavior review, duplicate and allocation-consumer safety inspection.
20 bounded contract combinations pass CPython 3.14.7 and patched native execution; unchanged source fails the first new hint assertion. Oversized hints were tested only through direct next()/length_hint, never an allocation consumer.
Full updated native dictionary snippet passes through pytest.
All 13 selected native canonical reverse/pickle/forward-hint tests pass after removing the proven-obsolete clear/restore marker. A final rebuilt binary also passes the complete snippet and all 20 bounded contract combinations.
Normal hooks, VM Clippy and separate CAPI Clippy passed.
Full hosted platform/workspace/CAPI CI remains pending.

Extraction, regression and validation were AI-assisted with OpenAI Codex. Draft pending hosted CI and review. README unchanged.

CI follow-up: preserve private relocation invalidation

Windows CI exposed a real interaction with the existing private _imp._dict_move_to_end contract: ordinary same-size mutation support must not allow iterators to survive an atomic relocation. The existing import_atomic_reorder.py assertion is unchanged.

Track actual private relocations with a lock-protected generation, and validate the original live count plus generation in the same read-locked reverse lookup. This removes the resnapshot retry loop. Clear, drain, resize and replacement preserve the owning dictionary's history; copies start fresh. Forward iterators retain full snapshot validation. The wrapping u64 generation adds no per-move allocation; like finite generation counters generally, it has a theoretical ABA limit after 2^64 unobserved moves.

The failing published head was reproduced locally. The correction passes the unchanged atomic-reorder and dictionary snippets, both pytest entries, all 20 bounded 3.14 contract combinations, and all 13 selected canonical tests. Independent source audit, normal hooks, VM Clippy and separate C-API Clippy pass. Updated hosted CI remains pending; the earlier failing head is not treated as green.

Summary by CodeRabbit

Bug Fixes
- Reverse iterators for dictionaries and their keys, values, and items views now report the number of remaining live entries and reduce that count as entries are returned.
- Reverse iterators continue to detect dictionary changes after exhaustion, raising a runtime error on the next iteration attempt.

Extract the Python 3.14-compatible remaining-count and same-size storage handling from RustPython#8954. Preserve the older deferred exhaustion and extra-yield contract, using explicit wrapping arithmetic rather than the Python 3.15 extra-yield error and cursor clamp. Cover deleted-slot hints and mutation after the last yield in the existing Python snippet. Assisted-by: OpenAI Codex:model identifier unavailable

The patched native interpreter unexpectedly passes the existing reverse-dict clear/restore test. Remove only its obsolete expected-failure decorator, preserving the canonical assertions and all other markers. Assisted-by: OpenAI Codex:model identifier unavailable

coderabbitai · 2026-10-10T09:35:18Z

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: 660e8667-b86b-4ed2-b8e5-fb74a7c1dd39

📥 Commits

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

⛔ Files ignored due to path filters (1)

Lib/test/test_dict.py is excluded by !Lib/**

📒 Files selected for processing (3)

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

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

📝 Walkthrough

Walkthrough

Reverse dictionary iterators now track remaining entries and use relocation history when validating changes. Tests cover length hints for dictionaries and their keys, values, and items views.

Changes

Reverse dictionary iterators

Layer / File(s)	Summary
Track dictionary relocations `crates/vm/src/dict_inner.rs`	`DictInner` and `DictSize` now record relocation history. Cloning and default construction initialize the epoch, content replacement preserves the destination epoch, and `move_to_end` increments it.
Validate and count reverse iteration `crates/vm/src/dict_inner.rs`, `crates/vm/src/builtins/dict.rs`, `extra_tests/snippets/builtin_dict.py`	Reverse checked iteration compares live-entry count and relocation history. Reverse iterators track remaining entries for length hints and decrement that count as entries are yielded. Tests check the hints and the error raised after adding an entry to an exhausted iterator.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to f31f2

No actionable merge-blocking risk is established; merge after normal checks.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 3 files.	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 primary change: tracking live entries in reverse dictionary iterators.
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.

Fix all pre-merge checks with AI

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

github-actions · 2026-10-10T09:50:54Z

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_dict.py (TODO: 2)
[x] test: cpython/Lib/test/test_dictcomps.py
[x] test: cpython/Lib/test/test_dictviews.py (TODO: 1)
[x] test: cpython/Lib/test/test_userdict.py
[x] test: cpython/Lib/test/mapping_tests.py

dependencies:

dependent tests: (no tests depend on dict)

Legend:

[+] path exists in CPython
[x] up-to-date, [ ] outdated

Assisted-by: OpenAI Codex:model identifier unavailable

chatgpt-codex-connector · 2026-10-10T10:55:41Z

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:03:10.948807Z	`f31f23b`	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: f31f23b9e8

ℹ️ 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:03:08Z

+    d = {0: 0, 1: 1, 2: 2, 3: 3}
+    del d[1]
+    it = reverse_iter(d)
+    for remaining in (3, 2, 1):
+        assert it.__length_hint__() == remaining


Remove the forbidden regression block

This block adds new test data, control flow, and assertions to extra_tests/snippets/builtin_dict.py, but the root AGENTS.md classifies test-code restrictions as critical, forbids modifying test assertions/logic/data, and permits test-file edits only for expected-failure marker changes. The decorator removal in Lib/test/test_dict.py is allowed, but this added snippet block is not.

AGENTS.md reference: AGENTS.md:L273-L280

Useful? React with 👍 / 👎.

youknowdot added 2 commits October 10, 2026 18:10

Preserve reverse iterator invalidation on atomic relocation

f31f23b

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 10:55

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

chatgpt-codex-connector Bot reviewed Oct 10, 2026

View reviewed changes

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

Navigation Menu

Repository navigation

Track live entries in reverse dictionary iterators - #9046

Track live entries in reverse dictionary iterators#9046
youknowone merged 3 commits into
RustPython:mainfrom
youknowdot:fix-dict-reverse-length-hints

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 warning)

Uh oh!

github-actions Bot commented Oct 10, 2026

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

CI follow-up: preserve private relocation invalidation

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 warning)

Uh oh!

github-actions Bot commented Oct 10, 2026

📦 Library Dependencies

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