Fix recursive sequence repetition fallback by 1ndahous3 · Pull Request #8865 · RustPython/RustPython

1ndahous3 · 2026-09-27T13:47:44Z

This PR follows our AI policy

Summary

Raise TypeError instead of overflowing the native stack when *= is used with a sequence that does not support repetition, including range and classes defining only __getitem__.

The presence of an item slot made _imul call PySequence::inplace_repeat, which called _imul again when no repeat slot existed. Correct the numeric and sequence fallbacks:

Multiplication and in-place multiplication use native repeat slots after numeric methods decline. Invalid repeat counts raise the sequence-specific TypeError or index-sized OverflowError, including the original count type in overflow messages. In-place multiplication only tries the right sequence when the left type has no sequence table, and never mutates that right operand.
Direct in-place sequence operations fall back to numeric dispatch without re-entering the full operator implementation, preserving their sequence-specific errors.
Python __mul__, __rmul__ and __imul__ populate numeric slots without redundant sequence wrappers; native repeat slots remain inherited. This prevents duplicate method calls and leaked NotImplemented results after numeric dispatch declines an operation.

Known limitations

A native list subclass still receives numeric wrappers for inherited sequence methods. Invalid-count messages, inherited __imul__ precedence and the result type after deleting an __imul__ override can differ from CPython; these pre-existing differences remain unchanged.

AI assistance

Written with Claude Code (claude-opus-5-5) and Codex (GPT-6), reviewed by a human before submission.

Summary by CodeRabbit

Bug Fixes
- Improved sequence concatenation and repetition operator behavior, including consistent fallback to regular operators when in-place operations are unsupported.
- Corrected repetition handling across built-in sequences and subclasses, including support for index-based repeat counts and numeric fallback.
- Standardized errors for unsupported repetition operands and counts that exceed the supported range.

Assisted-by: Claude Code:claude-opus-5-5 Assisted-by: Codex:GPT-6

chatgpt-codex-connector · 2026-09-27T13:47:48Z

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

coderabbitai · 2026-09-27T13:48:14Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 133bc813-9d2f-4593-9a55-34eda60d595f

📥 Commits

Reviewing files that changed from the base of the PR and between f871707 and 193afdb.

📒 Files selected for processing (4)

crates/vm/src/protocol/sequence.rs
crates/vm/src/types/slot.rs
crates/vm/src/vm/vm_ops.rs
extra_tests/snippets/operator_arithmetic.py

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

📝 Walkthrough

Walkthrough

Sequence repetition now uses sequence slots and shared binary-operator dispatch. In-place concatenation and repetition use binary operator fallbacks. New tests cover unsupported operands, count conversion, method fallback, and error messages.

Changes

Sequence operator dispatch

Layer / File(s)	Summary
Resolve sequence repeat slots `crates/vm/src/types/slot.rs`	Sequence repeat slots resolve Python multiplication methods through MRO lookup. Native repeat slots remain when no matching Python method is found.
Route sequence operations through slots `crates/vm/src/vm/vm_ops.rs`, `crates/vm/src/protocol/sequence.rs`, `extra_tests/snippets/operator_arithmetic.py`	Multiplication uses sequence repeat slots and shared count conversion. In-place operations use binary operator fallbacks. Tests cover supported and unsupported operands, count conversion, method fallback, and sequence-specific errors.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: youknowone

Merge Risk: ⚪ Minimal · up to 193af

No actionable merge-blocking issue remains; the reviewed multiplication and direct sequence-operation paths preserve their supported behavior.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 193af

The changed dispatch appears to keep repetition within the existing VM object model, without adding a privilege path. It changes broadly used operator behavior, however, and security coverage is incomplete.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The independently controllable surface is the operands and their Python special methods in VM operator evaluation. The observed effect is confined to selecting a callback, its result or error, and any behavior that callback itself performs; broader host-capability exposure was not established.

Trust Boundaries and Controls

observed — Numeric dispatch must decline before sequence repetition is attempted. Count conversion occurs before the repeat callback, and right-operand fallback does not select its in-place slot.

Resilience and Maintainability Implications

inferred — Conversion failures do not reach a repeat callback. Once an arbitrary callback runs, its partial side effects and retry behavior remain callback-owned rather than guaranteed by dispatch; the inspected change adds no retry or rollback transition.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	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 20 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: fixing recursive fallback behavior for sequence repetition.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	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

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

codspeed · 2026-09-27T14:50:01Z

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 4 skipped benchmarks¹

_{Comparing 1ndahous3:sequence_repeat_recursion (193afdb) with main (f871707)}

4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Fix recursive sequence repetition fallback

193afdb

Assisted-by: Claude Code:claude-opus-5-5 Assisted-by: Codex:GPT-6

youknowone approved these changes Sep 27, 2026

View reviewed changes

youknowone enabled auto-merge (squash) September 27, 2026 15:11

youknowone merged commit 8c928c2 into RustPython:main Sep 27, 2026
22 checks passed

1ndahous3 deleted the sequence_repeat_recursion branch September 27, 2026 17:40

1ndahous3 mentioned this pull request Oct 19, 2026

Preserve reflected multiplication for sequence subclasses #9228

Merged

1 task done

Navigation Menu

Repository navigation

Fix recursive sequence repetition fallback - #8865

Fix recursive sequence repetition fallback#8865
youknowone merged 1 commit into
RustPython:mainfrom
1ndahous3:sequence_repeat_recursion

1ndahous3 commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 27, 2026

Uh oh!

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

codspeed Bot commented Sep 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

1ndahous3 commented Sep 27, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Known limitations

AI assistance

Summary by CodeRabbit

Uh oh!

chatgpt-codex-connector Bot commented Sep 27, 2026

Uh oh!

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

Uh oh!

Walkthrough

Changes

Security Architecture Review

❌ Failed checks (1 warning)

Uh oh!

codspeed Bot commented Sep 27, 2026

Merging this PR will not alter performance

Footnotes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

1ndahous3 commented Sep 27, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 27, 2026 •

edited

Loading