Check `enumerate()` arguments before calling `__iter__` by jseop-lim · Pull Request #9241 · RustPython/RustPython

jseop-lim · 2026-10-20T05:21:11Z

This PR follows our AI policy

Summary

EnumerateArgs binds the iterable as PyIter, so argument binding calls __iter__ before start is converted and before surplus or unknown arguments are rejected. An exception from __iter__ then replaces the TypeError CPython raises. CPython's enum_new_impl() converts start with PyNumber_Index before it calls PyObject_GetIter.

AS-IS

>>> class FailingIter:
...     def __iter__(self): raise RuntimeError
...
>>> enumerate(FailingIter(), {})
RuntimeError
>>> enumerate(FailingIter(), 0, 0)
RuntimeError

TO-BE

>>> enumerate(FailingIter(), {})
TypeError: 'dict' object cannot be interpreted as an integer
>>> enumerate(FailingIter(), 0, 0)
TypeError: enumerate expected at most 2 arguments, got 3

(matching CPython's exception types; the wording of the second message is the shared argument-count message and still differs from CPython's enumerate-specific enumerate() takes at most 2 arguments (3 given))

Changes

Bind iterable as PyObjectRef and call get_iter() in py_new, after start is converted, as itertools.batched already does for its iterable.
Extend extra_tests/snippets/builtin_enumerate.py with a bad start by position and by keyword, a third argument, an unknown keyword, and a valid call that still raises from __iter__.

Test plan

Built and run on macOS aarch64 against upstream/main 398dd70, using the CI feature set --no-default-features --features stdlib,importlib,stdio,encodings,sqlite,ssl-rustls-aws-lc,host_env,doc,ucd_3_2_0, plus threading, compared with CPython 3.14.4.

A 20-case probe over bad and valid start values (by position and keyword), missing, surplus and unknown arguments, __index__, bool, 2**70 and __reduce__ differs from CPython in 7 cases on an unpatched build of that commit and matches in all of them with the patch. It also checks that a rejected call no longer runs __iter__.
rustpython -m test test_enumerate test_builtin test_itertools test_iter test_list: 524 run, 56 skipped, all pass.
pytest -k "rustpython and builtin" in extra_tests: 71 pass. The extended snippet fails on the unpatched build and passes under CPython 3.14.4.
cargo fmt --check clean, cargo clippy -p rustpython-vm --all-targets reports nothing, and pre-commit run --files passes on both changed files.

I used Claude Code (claude-opus-5-5) to draft the analysis, the patch and this description; I reviewed the change and ran every command above myself.

Summary by CodeRabbit

Bug Fixes
- Improved enumerate argument validation so invalid argument counts, keywords, and starting values are reported before iteration begins.
- Preserved proper propagation of iterator errors when iteration is attempted.

EnumerateArgs bound the iterable as PyIter, so argument binding called __iter__ before start was converted and before surplus positional arguments were rejected. An exception from __iter__ then replaced the TypeError CPython raises for a bad start or a third argument. Bind the iterable as an object and call get_iter() in py_new, after start is converted, which is the order of CPython's enum_new_impl. Assisted-by: Claude Code:claude-opus-5-5

chatgpt-codex-connector · 2026-10-20T05:21:27Z

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-20T05:26:23.335999Z	`bae548e`	PR opened

ℹ️ 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.

coderabbitai · 2026-10-20T05:21:58Z

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: 88a37536-b628-4a26-8f60-737ecbff6154

📥 Commits

Reviewing files that changed from the base of the PR and between 398dd70 and bae548e.

📒 Files selected for processing: 2

crates/vm/src/builtins/enumerate.rs
extra_tests/snippets/builtin_enumerate.py

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

📝 Walkthrough

enumerate now obtains an iterator after argument binding. Added tests check that invalid arguments raise TypeError before iterator access, and that iterator errors propagate for valid arguments.

Changes: Enumerate argument validation

Layer / File(s)	Summary
Bind arguments before creating the iterator `crates/vm/src/builtins/enumerate.rs`, `extra_tests/snippets/builtin_enumerate.py`	`EnumerateArgs.iterable` now stores the original object. `PyEnumerate::py_new` calls `get_iter(vm)` after argument binding. Tests check argument validation before iterator access and error propagation for a valid start.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to bae54

Argument validation now precedes iterator access, and valid enumeration behavior remains preserved. The change is mergeable.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 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 main change: validating enumerate() arguments before invoking iter.
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.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bae548e432

ℹ️ 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-20T05:26:20Z

+    // Bound as an object so the other arguments are checked before `iter()` runs, as in CPython.
    #[pyarg(any)]
-    iterable: PyIter,
+    iterable: PyObjectRef,


Defer start conversion until binding validates the call

After only iterable is made raw, a malformed call such as enumerate(BadIter(), RaisingIndex(), extra) reaches ArgIndex::try_from_object for start before FuncArgs::bind_for notices the extra positional argument. It therefore runs and may propagate RaisingIndex.__index__ instead of raising the argument-count TypeError; CPython rejects this call before invoking either __index__ or __iter__. Binding start as an object too, then converting it in py_new immediately before get_iter, preserves the intended valid-call order without this side effect.

Useful? React with 👍 / 👎.

chatgpt-codex-connector Bot reviewed Oct 20, 2026

View reviewed changes

youknowone approved these changes Oct 20, 2026

View reviewed changes

youknowone merged commit d634f26 into RustPython:main Oct 20, 2026
20 of 21 checks passed

Navigation Menu

Repository navigation

Check `enumerate()` arguments before calling `iter` - #9241

Check `enumerate()` arguments before calling `iter`#9241
youknowone merged 1 commit into
RustPython:mainfrom
jseop-lim:enumerate-iter-order

jseop-lim commented Oct 20, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 20, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Oct 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Summary

AS-IS

TO-BE

Changes

Test plan

Summary by CodeRabbit

Uh oh!

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

Uh oh!

Codex Review Summary

Uh oh!

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

Uh oh!

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 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jseop-lim commented Oct 20, 2026 •

edited by coderabbitai Bot

Loading

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

edited

Loading

coderabbitai Bot commented Oct 20, 2026 •

edited

Loading