Skip to content

__length_hint__ - #6636

Merged
youknowone merged 4 commits into
RustPython:mainfrom
youknowone:__length_hint__
Aug 2, 2026
Merged

__length_hint__#6636
youknowone merged 4 commits into
RustPython:mainfrom
youknowone:__length_hint__

Conversation

@youknowone

@youknowone youknowone commented Jan 3, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

fix #6590

Summary by CodeRabbit

  • Bug Fixes
    • Improved sequence iterator length hints to safely handle recursive calls and exhausted iterators.
    • Iterators now correctly report remaining items, return zero when exhausted, and use NotImplemented when sequence length is unavailable.
  • Tests
    • Added coverage for recursion handling, unsupported length queries, and decreasing remaining-length results.

@coderabbitai

coderabbitai Bot commented Jan 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 106eec8c-f6c5-44ea-912e-8a1a07dd2b59

📥 Commits

Reviewing files that changed from the base of the PR and between 6131363 and d22855e.

📒 Files selected for processing (2)
  • crates/vm/src/builtins/iter.rs
  • extra_tests/snippets/builtin_iter.py
📝 Walkthrough

Walkthrough

PySequenceIterator::__length_hint__ now protects recursive calls, releases its lock before invoking sequence length logic, handles exhausted and unsized sequences, and returns saturated remaining lengths. Tests cover recursion errors, unsized sequences, and decreasing length hints.

Changes

Sequence iterator length hints

Layer / File(s) Summary
Recursion-safe length-hint implementation
crates/vm/src/builtins/iter.rs		 __length_hint__ returns PyResult<PyObjectRef>, snapshots state under the lock, queries sequence length after unlocking, and handles recursion, exhaustion, unavailable lengths, and saturation.
Length-hint behavior validation
extra_tests/snippets/builtin_iter.py		 Tests verify recursive calls raise RecursionError without deadlock, unsized sequences return NotImplemented, and sized sequences report the remaining item count.

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

Suggested reviewers: shaharnaveh

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 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.

@github-actions

github-actions Bot commented Jan 14, 2026

Copy link
Copy Markdown
Contributor

Code has been automatically formatted

The code in this PR has been formatted using:

  • cargo fmt --all
    Please pull the latest changes before pushing again:
git pull origin __length_hint__

Copilot AI and others added 4 commits August 2, 2026 16:24
Copilot AI review requested due to automatic review settings August 2, 2026 07:39
Copilot AI reviewed Aug 2, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes the deadlock described in #6590 by ensuring iterator.__length_hint__ does not hold the iterator’s internal PyMutex across user callbacks (like __len__). While addressing that, it also corrects the hint calculation to account for the iterator’s current position (so the hint decreases after next()), and adds a regression test to cover the deadlock scenario.

Changes:

  • Refactors PySequenceIterator::__length_hint__ to drop the internal lock before invoking __len__, preventing re-entrant deadlocks.
  • Updates __length_hint__ behavior to return len - position (saturating at 0) and return NotImplemented when a length slot is unavailable.
  • Adds a snippet regression test that would previously hang, and verifies correct __length_hint__ behavior for sequences with/without __len__.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
extra_tests/snippets/builtin_iter.py Adds regression coverage for the deadlock PoC and verifies correct __length_hint__ semantics.
crates/vm/src/builtins/iter.rs Releases the iterator lock before calling into user __len__ and computes hint as remaining items (len - position).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@youknowone
youknowone marked this pull request as ready for review August 2, 2026 12:20
@youknowone
youknowone enabled auto-merge (squash) August 2, 2026 12:21
@youknowone
youknowone disabled auto-merge August 2, 2026 12:21
@youknowone
youknowone merged commit 1306b71 into RustPython:main Aug 2, 2026
25 of 27 checks passed
@youknowone
youknowone deleted the __length_hint__ branch August 2, 2026 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deadlock when __len__ re-enters __length_hint__ via list iterator

3 participants

@youknowone