Repository navigation
Fix string addition and sequence subclass dispatch - #8822
Conversation
Use sequence concatenation for str and clear inherited concat slots when Python subclasses override __add__. Cover direct calls, reflected dispatch, and dynamic overrides against CPython. Codex assisted with investigation, implementation, tests, validation, and the PR description. Assisted-by: Codex:gpt-6
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
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 configurationConfiguration used: Repository: RustPython/RustPython/.coderabbit.yml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughString addition now uses sequence-slot dispatch. The slot update resolves ChangesString Addition Dispatch
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue remains; the changed addition path preserves single-call and reflected-dispatch behavior. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The changed behavior affects addition across built-in sequences and their subclasses. The inspected dispatch paths show no new privileged operation or trust-boundary bypass, but security coverage of the affected code is incomplete. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Thanks! |
Summary
A
strsubclass that declines addition still gets concatenated:CPython raises
TypeError; RustPython returns"ab". Direct calls also differ:str.__add__("a", 1)returnsNotImplementedinstead of raisingTypeError.Use the sequence concatenation slot for strings and leave reflected calls to the operator dispatcher. This removes the extra
str.__radd__method and prevents a declining right operand from being called twice. Clear inherited sequence concatenation when a subclass overrides__add__, and restore it when the native method becomes visible again.The regression covers direct calls,
operator.add,operator.iadd, reflected-call counts, and inherited or dynamically replaced overrides for strings, bytes, tuples, and lists. The existing exact-string bytecode fast paths remain unchanged.Related: #7293 introduced the numeric string-addition slot; #8652 fixed concatenation errors when a reflected call declines. This preserves those error messages while correcting dispatch.
Validation
393689d), and passes with this patch.test_str,test_operator,test_descr,test_exceptions,test_list,test_tuple, andtest_bytes: 941 tests run, 59 skipped; all modules pass.extra_tests: 464 passed. Four failures also reproduce without the patch: the default build lacks_sqlite3, and three host-side multiprocessing tests return non-Nonevalues, which pytest rejects.cargo test --release --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher --exclude rustpython-capi). The local macOS run needs the installed library directories inDYLD_FALLBACK_LIBRARY_PATH.AI assistance
Codex assisted with investigation, implementation, regression tests, validation, and this description.
Summary by CodeRabbit
NotImplemented.