Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 0 additions & 24 deletions crates/vm/src/builtins/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 15 additions & 1 deletion crates/vm/src/types/slot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
88 changes: 88 additions & 0 deletions extra_tests/snippets/builtin_str_add.py
Original file line number Diff line number Diff line change
@@ -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
Loading