GeezerVison

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

GeezerVison is showing github.com — raise TypeError when `__iter__` method is `None` by elmjag · Pull Request #7092 · RustPython/RustPython

Conversation

elmjag commented Feb 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Setting special __iter__ method to None is used to mark objects as non-iterable.

https://docs.python.org/3/reference/datamodel.html#special-method-names

Check if __iter__ is set to None and raise TypeError exception.

Summary by CodeRabbit

Bug Fixes
- Enhanced error reporting for iteration operations with clearer, more precise error messages when attempting to iterate over non-iterable objects
- Improved exception handling to distinguish between different iteration failure scenarios

Sorry, something went wrong.

Uh oh!

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

All reactions

coderabbitai Bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

📝 Walkthrough

Walkthrough

The iter_wrapper function in slot.rs was refactored to explicitly look up the __iter__ special method on objects rather than directly invoking it, with added error handling to distinguish between missing methods and methods explicitly set to None, raising appropriate AttributeError or TypeError exceptions.

Changes

Cohort / File(s)	Summary
Iterator wrapper enhancement `crates/vm/src/types/slot.rs`	Refactored `iter_wrapper` to perform explicit `__iter__` method lookup with None-checking logic; raises `AttributeError` when method is missing and `TypeError` when explicitly set to None; added `PyMethod` import to support method resolution.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

More slot impl #6618: Modifies slot method resolution and ownership handling logic in the same file, affecting how special methods are accessed and wrapped.
iter with slot-wrapper #6488: Changes iterator protocol implementation by replacing direct __iter__ invocation with explicit slot-based method lookup and resolution, directly related to this iterator handling enhancement.

Poem

🐰 A hop through slots, a method to find,
No more direct calls of the blind,
Explicit lookup, errors ring clear,
When None means "not iterable here!"
The rabbit approves of semantics so fine! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title clearly and specifically describes the main change: raising a TypeError when the iter method is None, which matches the core objective of implementing proper handling for non-iterable objects.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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 and usage tips.}

All reactions

Sorry, something went wrong.

Uh oh!

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

coderabbitai Bot reviewed

Feb 12, 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

🤖 Fix all issues with AI agents

In `@crates/vm/src/types/slot.rs`:
- Around line 548-551: Replace the AttributeError raised when __iter__ is
missing with a TypeError: in the block that calls vm.get_special_method(&zelf,
identifier!(vm, __iter__)), change the ok_or_else(...) from
vm.new_attribute_error(...) to vm.new_type_error(...) and construct the message
as "'<ClassName>' object is not iterable" using the object's class name (e.g.,
format!("'{}' object is not iterable", zelf.class().name())). Keep the existing
method_is_none() TypeError behavior unchanged.

🧹 Nitpick comments (2)

crates/vm/src/types/slot.rs (2)
545-551: Decorative comment separators violate coding guidelines.

Lines 545–547 and 553–556 use blank // lines as section separators. As per coding guidelines, "Do not add decorative section separators (e.g. // -----------, // ===, /* *** */). Use /// doc-comments or short // comments only when they add value."
Proposed fix: collapse to single-line comments
-    //
-    // look-up '__iter__' method
-    //
+    // Look up __iter__ method
     let method_ident = identifier!(vm, __iter__);
     let method = vm
         .get_special_method(&zelf, method_ident)?
         .ok_or_else(|| vm.new_attribute_error(method_ident.as_str().to_owned()))?;

-    //
-    // setting '__iter__' method to 'None' value is used
-    // to mark non-iterable objects, raise TypeError
-    //
+    // __iter__ = None marks non-iterable objects
     if method_is_none(&method, vm) {
537-562: Consider consistency with __hash__ = None handling.

The __hash__ = None case (lines 758–783) is handled at slot-update time by storing hash_not_implemented as the slot function. The __iter__ = None case is handled at call time inside the wrapper. Both approaches work, but the inconsistency may confuse future maintainers. If there's no strong reason to differ, consider aligning the pattern (e.g., storing a dedicated iter_not_implemented function during slot update, similar to hash_not_implemented).

That said, CPython's slot_tp_iter also checks at call time, so the current approach is arguably more faithful to CPython.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

crates/vm/src/types/slot.rs

Comment on lines +548 to +551

		let method_ident = identifier!(vm, __iter__);
		let method = vm
		.get_special_method(&zelf, method_ident)?
		.ok_or_else(\|\| vm.new_attribute_error(method_ident.as_str().to_owned()))?;

coderabbitai Bot Feb 12, 2026

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify what error CPython raises when __iter__ is dynamically deleted after being set
# Also check how get_special_method works and what it returns for None vs missing

echo "=== Check get_special_method implementation ==="
ast-grep --pattern 'fn get_special_method($$$) {
  $$$
}'

echo ""
echo "=== Look for get_special_method definition ==="
rg -n 'fn get_special_method' crates/vm/src/ -A 20 | head -50

Repository: RustPython/RustPython

Length of output: 1467

🏁 Script executed:

#!/bin/bash
echo "=== Check iter_wrapper and surrounding context in slot.rs ==="
rg -n 'iter_wrapper\|__iter__' crates/vm/src/types/slot.rs -B 5 -A 10

Repository: RustPython/RustPython

Length of output: 126

🏁 Script executed:

#!/bin/bash
echo "=== Check how other slot wrappers handle missing methods ==="
rg -n 'new_attribute_error|new_type_error' crates/vm/src/types/slot.rs -B 3 -A 1 | head -100

Repository: RustPython/RustPython

Length of output: 4724

🏁 Script executed:

#!/bin/bash
echo "=== Check if there are references to 'not iterable' error messages ==="
rg -n 'not iterable' crates/vm/src/ -B 2 -A 2

Repository: RustPython/RustPython

Length of output: 1234

🏁 Script executed:

#!/bin/bash
echo "=== Check get_special_method behavior in the codebase ==="
rg -n 'get_special_method.*ok_or' crates/vm/src/ -B 3 -A 3 | head -60

Repository: RustPython/RustPython

Length of output: 122

Change error type from AttributeError to TypeError when __iter__ is missing.

The code at line 551 currently raises AttributeError when get_special_method returns None (i.e., when __iter__ is not found). This is inconsistent with CPython, which raises TypeError: '<ClassName>' object is not iterable. This error type also aligns with how other slot wrappers in the codebase handle missing methods—len_wrapper, repr_wrapper, str_wrapper, and others all use new_type_error() for similar cases.

The method_is_none() check at line 557 handles the case where __iter__ exists but returns None, which should also raise the same TypeError (already correct).

Proposed fix

     let method_ident = identifier!(vm, __iter__);
     let method = vm
         .get_special_method(&zelf, method_ident)?
-        .ok_or_else(|| vm.new_attribute_error(method_ident.as_str().to_owned()))?;
+        .ok_or_else(|| vm.new_type_error(format!("'{}' object is not iterable", zelf.class().name())))?;

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	let method_ident = identifier!(vm, __iter__);
	let method = vm
	.get_special_method(&zelf, method_ident)?
	.ok_or_else(\|\| vm.new_attribute_error(method_ident.as_str().to_owned()))?;
	let method_ident = identifier!(vm, __iter__);
	let method = vm
	.get_special_method(&zelf, method_ident)?
	.ok_or_else(\|\| vm.new_type_error(format!("'{}' object is not iterable", zelf.class().name())))?;

🤖 Prompt for AI Agents

In `@crates/vm/src/types/slot.rs` around lines 548 - 551, Replace the
AttributeError raised when __iter__ is missing with a TypeError: in the block
that calls vm.get_special_method(&zelf, identifier!(vm, __iter__)), change the
ok_or_else(...) from vm.new_attribute_error(...) to vm.new_type_error(...) and
construct the message as "'<ClassName>' object is not iterable" using the
object's class name (e.g., format!("'{}' object is not iterable",
zelf.class().name())). Keep the existing method_is_none() TypeError behavior
unchanged.

Sorry, something went wrong.

Uh oh!

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

All reactions

ShaharNaveh commented Feb 12, 2026

Copy link

Copy Markdown

Contributor

Thanks for the PR!
I don't think that the failed CI is related to your change, I'll investigate

All reactions

Sorry, something went wrong.

Uh oh!

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

ShaharNaveh commented Feb 12, 2026

Copy link

Copy Markdown

Contributor

yup: rust-lang/miri#4855

All reactions

Sorry, something went wrong.

Uh oh!

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

ShaharNaveh mentioned this pull request

Feb 12, 2026

fix miri ci #7093

Merged

elmjag commented Feb 12, 2026

Copy link

Copy Markdown

Contributor Author

yup: rust-lang/miri#4855

Thanks for investigating!

ShaharNaveh reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

ShaharNaveh commented Feb 12, 2026

Copy link

Copy Markdown

Contributor

@elmjag please merge main, it should pass now

All reactions

Sorry, something went wrong.

Uh oh!

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

elmjag commented Feb 12, 2026

Copy link

Copy Markdown

Contributor Author

537-562: Consider consistency with __hash__ = None handling.
The __hash__ = None case (lines 758–783) is handled at slot-update time by storing hash_not_implemented as the slot function. The __iter__ = None case is handled at call time inside the wrapper. Both approaches work, but the inconsistency may confuse future maintainers. If there's no strong reason to differ, consider aligning the pattern (e.g., storing a dedicated iter_not_implemented function during slot update, similar to hash_not_implemented).
That said, CPython's slot_tp_iter also checks at call time, so the current approach is arguably more faithful to CPython.

I'll look into using this approach for __iter__ = None case. Sounds like a better way to implement it, marking this PR as draft for now.

All reactions

Sorry, something went wrong.

Uh oh!

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

elmjag marked this pull request as draft

February 12, 2026 15:20


          raise TypeError when __iter__ method is None

bdcc047

Setting special `__iter__` method to `None` is used to mark
objects as non-iterable.

https://docs.python.org/3/reference/datamodel.html#special-method-names

Check if `__iter__` is set to `None` and raise TypeError exception.

elmjag force-pushed the iter-meth-none branch from e131b7c to bdcc047 Compare

February 12, 2026 16:34

github-actions Bot commented Feb 12, 2026

Copy link

Copy Markdown

Contributor

📦 Library Dependencies

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

[x] lib: cpython/Lib/typing.py
[ ] test: cpython/Lib/test/test_typing.py (TODO: 6)
[x] test: cpython/Lib/test/test_type_aliases.py
[x] test: cpython/Lib/test/test_type_annotations.py (TODO: 1)
[x] test: cpython/Lib/test/test_type_params.py (TODO: 10)
[x] test: cpython/Lib/test/test_genericalias.py

dependencies:

typing

dependent tests: (12 tests)

typing: test_annotationlib test_builtin test_enum test_fractions test_functools test_genericalias test_grammar test_isinstance test_type_aliases test_type_params test_types test_typing

Legend:

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

All reactions

Sorry, something went wrong.

Uh oh!

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

ShaharNaveh commented Feb 22, 2026

Copy link

Copy Markdown

Contributor

@elmjag can you please resolve the conflicts? I think it can be merged

All reactions

Sorry, something went wrong.

Uh oh!

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

elmjag commented Feb 23, 2026

Copy link

Copy Markdown

Contributor Author

@elmjag can you please resolve the conflicts? I think it can be merged

This issue is now fixed, via #7091. Closing this PR.

ShaharNaveh reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

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

elmjag closed this

Feb 23, 2026

Reviewers

coderabbitai[bot]

coderabbitai[bot] left review comments

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.

2 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

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

Uh oh!

Review skipped

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh commented Feb 12, 2026

Uh oh!

ShaharNaveh commented Feb 12, 2026

Uh oh!

elmjag commented Feb 12, 2026

Uh oh!

ShaharNaveh commented Feb 12, 2026

Uh oh!

elmjag commented Feb 12, 2026

Uh oh!

github-actions Bot commented Feb 12, 2026

📦 Library Dependencies

Uh oh!

ShaharNaveh commented Feb 22, 2026

Uh oh!

elmjag commented Feb 23, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

elmjag commented Feb 12, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

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

coderabbitai Bot commented Feb 12, 2026 •

edited

Loading

Uh oh!

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