Fix string addition and sequence subclass dispatch (#8822)

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

crates/vm/src/builtins/str.rs

@@ -678,16 +678,6 @@ impl PyStr {
             }
             .to_pyobject(vm))
         } else {
-            // hack to get around not distinguishing number add from seq concat
-            if let Some(radd) = vm.get_method(other.to_owned(), identifier!(vm, __radd__)) {
-                let result = radd?.call((zelf,), vm)?;
-                // CPython reaches `str`'s sq_concat once the reflected call declines, so a
-                // `__radd__` returning NotImplemented must still report the concat error
-                // rather than the generic binary-op one.
-                if !result.is(&vm.ctx.not_implemented) {
-                    return Ok(result);
-                }
-            }
             Err(vm.new_type_error(format!(
                 r#"can only concatenate str (not "{}") to str"#,
                 other.class().slot_name()
@@ -1694,20 +1684,6 @@ impl AsMapping for PyStr {
 impl AsNumber for PyStr {
     fn as_number() -> &'static PyNumberMethods {
         static AS_NUMBER: PyNumberMethods = PyNumberMethods {
-            add: Some(|a, b, vm| {
-                let Some(a) = a.downcast_ref::<PyStr>() else {
-                    return Ok(vm.ctx.not_implemented());
-                };
-                let Some(b) = b.downcast_ref::<PyStr>() else {
-                    return Ok(vm.ctx.not_implemented());
-                };
-                let bytes = a.as_wtf8().py_add(b.as_wtf8());
-                Ok(unsafe {
-                    let kind = a.kind() | b.kind();
-                    PyStr::new_str_unchecked(bytes.into(), kind)
-                }
-                .to_pyobject(vm))
-            }),
             remainder: Some(|a, b, vm| {
                 if let Some(a) = a.downcast_ref::<PyStr>() {
                     a.__mod__(b.to_owned(), vm).to_pyresult(vm)

crates/vm/src/types/slot.rs

@@ -1564,7 +1564,21 @@ impl PyType {
             SlotAccessor::SqLength => {
                 update_sub_slot!(as_sequence, length, sequence_len_wrapper, SeqLength)
             }
-            SlotAccessor::SqConcat | SlotAccessor::SqInplaceConcat if !ADD => {
+            SlotAccessor::SqConcat => {
+                // Python __add__ overrides use nb_add, not the inherited sq_concat.
+                let concat = match self.lookup_slot_in_mro(name, ctx, |sf| {
+                    if let SlotFunc::SeqConcat(f) = sf {
+                        Some(*f)
+                    } else {
+                        None
+                    }
+                }) {
+                    SlotLookupResult::NativeSlot(func) => Some(func),
+                    SlotLookupResult::PythonMethod | SlotLookupResult::NotFound => None,
+                };
+                self.slots.as_sequence.concat.store(concat);
+            }
+            SlotAccessor::SqInplaceConcat if !ADD => {
                 // Sequence concat uses sq_concat slot - no generic wrapper needed
                 // (handled by number protocol fallback)
                 accessor.inherit_from_mro(self);

extra_tests/snippets/builtin_str_add.py

@@ -0,0 +1,88 @@
+import operator
+
+from testutils import assert_raises
+
+
+class Declines(str):
+    def __add__(self, other):
+        return NotImplemented
+
+
+class Reflected:
+    def __init__(self, result):
+        self.calls = 0
+        self.result = result
+
+    def __radd__(self, other):
+        self.calls += 1
+        return self.result
+
+
+class ReflectedStr(str):
+    def __radd__(self, other):
+        return "reflected"
+
+
+def add(a, b):
+    return a + b
+
+
+def iadd(a, b):
+    a += b
+    return a
+
+
+for operation in (add, iadd, operator.add, operator.iadd):
+    assert operation("a", "b") == "ab"
+    assert operation("a", Declines("b")) == "ab"
+    assert_raises(TypeError, operation, Declines("a"), "b")
+    assert operation("a", ReflectedStr("b")) == "reflected"
+    for result in ("reflected", NotImplemented):
+        other = Reflected(result)
+        if result is NotImplemented:
+            assert_raises(TypeError, operation, "a", other)
+        else:
+            assert operation("a", other) == result
+        assert other.calls == 1
+
+assert str.__add__("a", "b") == "ab"
+assert str.__add__("a", ReflectedStr("b")) == "ab"
+assert_raises(TypeError, str.__add__, "a", 1)
+for result in ("reflected", NotImplemented):
+    other = Reflected(result)
+    assert_raises(TypeError, str.__add__, "a", other)
+    assert other.calls == 0
+
+assert not hasattr(str, "__radd__")
+
+assert str.__add__(Declines("a"), "b") == "ab"
+
+
+for base, left, right in (
+    (str, "a", "b"),
+    (bytes, b"a", b"b"),
+    (tuple, (1,), (2,)),
+    (list, [1], [2]),
+):
+
+    class Override(base):
+        def __add__(self, other):
+            return NotImplemented
+
+    class Child(Override):
+        pass
+
+    for cls in (Override, Child):
+        assert_raises(TypeError, operator.add, cls(left), right)
+
+    Override.__add__ = base.__add__
+    for cls in (Override, Child):
+        assert operator.add(cls(left), right) == left + right
+
+    Override.__add__ = lambda self, other: "override"
+    for cls in (Override, Child):
+        assert operator.add(cls(left), right) == "override"
+
+    del Override.__add__
+    for cls in (Override, Child):
+        assert operator.add(cls(left), right) == left + right