Skip to content

Preserve reflected addition for sequence subclasses - #9049

Merged
youknowone merged 2 commits into
RustPython:mainfrom
youknowdot:fix-sequence-subclass-reflected-add
Oct 10, 2026
Merged

youknowone merged 2 commits into
RustPython:mainfrom
youknowdot:fix-sequence-subclass-reflected-add

Conversation

@youknowdot

@youknowdot youknowdot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Extract the remaining sequence-subclass reflected-addition fix from #8954 (43065e79ddc46eafeca58cb39398ac007b5d25f7), adapted to the exact Python 3.14 slot contract on main 3af5f8a15314446d2b28a90e429c938dc7b8beeb.

Plain sequence subclasses inheriting a native concatenation descriptor must allow an unrelated right operand's __radd__. Preserve the distinction between inherited sequence concatenation and a numeric addition slot when methods are assigned, inherited, replaced, or deleted. Update the paired left slot when reflected addition changes, and use one stable numeric wrapper function identity.

The mega's unconditional treatment of native concatenation descriptors is insufficient for Python 3.14's stateful slot behavior. This patch preserves already-established numeric slots, exact descriptor identity/name, custom MRO membership, and unrelated numeric-subclass reflected priority. It follows CPython 3.14.7 type slot machinery.

Merged #8822 is already in the base and covers earlier override handling; this addresses the remaining inherited-native-sequence case rather than duplicating that change. No other direct duplicate appeared in the checked open/merged PR searches.

Validation

  • Exact CPython 3.14.7: the complete updated existing builtin_str_add.py snippet passes.
  • Unchanged native main fails the first new inherited-sequence regression; this head passes the entire snippet and its pytest entry.
  • Both focused canonical descriptor binary-operator tests pass. Complete test_binop, test_augassign, and test_operator modules pass: 129 tests.
  • Bounded differential probes match all 50 return/error-type outcomes and reflected call counts, plus the 35-row dynamic slot-state matrix. Some existing concatenation error strings differ from CPython; unchanged main produces those same diagnostic strings. This patch does not rewrite them.
  • VM Clippy and separate C-API Clippy with warnings denied; normal repository commit hooks.
  • Independent source review checked inheritance, exact native aliases, slot-wrapper identity, and the stored-MRO ownership check before native validation.

Coverage stays in the existing Python snippet; no Rust behavioral tests, canonical-test weakening, implementation-name guards, or README changes. Local checks are bounded Linux checks, not a complete workspace or cross-platform test claim. Full hosted CI is pending; this remains a draft for review.

AI-assisted implementation and validation with OpenAI Codex; model identifier unavailable. No human review is implied.

Summary by CodeRabbit

  • Bug Fixes
    • Corrected addition behavior for subclasses of str, bytes, tuple, list, and bytearray, including when addition methods are inherited, overridden, assigned, or deleted.
    • Improved how regular and reflected addition interact with sequence concatenation, so operations follow Python’s method dispatch rules more consistently.

Extract the Python 3.14-compatible sequence-add slot distinction from RustPython#8954 without its unconditional numeric-slot clearing. Preserve stateful duplicate-slot behavior, exact descriptor identity, inherited reflected priority and a stable wrapper function address. Extend the existing addition snippet only for the uncovered inherited-native and slot-update cases.

Assisted-by: OpenAI Codex:model identifier unavailable
Assisted-by: OpenAI Codex:model identifier unavailable
@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

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: 781d1ed3-23db-4a50-a07b-21277a160fd6

📥 Commits

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


📒 Files selected for processing (2)
  • crates/vm/src/types/slot.rs
  • extra_tests/snippets/builtin_str_add.py

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



📝 Walkthrough

Walkthrough

The NbAdd slot update now handles native sequence __add__ descriptors separately from other addition methods. New tests cover addition dispatch across native sequence and integer subclasses, including method inheritance, assignment, override, and deletion.

Changes

Numeric addition dispatch

Layer / File(s) Summary
Numeric addition slot dispatch
crates/vm/src/types/slot.rs, extra_tests/snippets/builtin_str_add.py
Adds a shared wrapper for numeric __add__ dispatch. The NbAdd update preserves native sequence concatenation and sets the numeric add slot under specified conditions. Tests cover reflected addition and changes to inherited or assigned methods.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: youknowone


Merge Risk: ⚪ Minimal · up to 24db0

The focused addition-dispatch checks are reported to pass, and no actionable issue is established. Normal CI completion remains pending.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 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 Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preserving reflected addition behavior for sequence subclasses.
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

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

@youknowone
youknowone marked this pull request as ready for review October 10, 2026 10:57
@youknowone
youknowone enabled auto-merge (squash) October 10, 2026 10:57
@chatgpt-codex-connector

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

Copy link
Copy Markdown

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:05:28.321299Z 24db067 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.

@youknowone
youknowone merged commit 02917b6 into RustPython:main Oct 10, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants