Binding arithmetics-assign operators with non-standard C++ signature lead to surprising results

[lyskov]

Looks like bindings for __iadd__/__isub__ operators uses return value of C++ function to change value of current object. So if corresponding C++ operator+= return void (instead of standard T&) - then value of object became Python None. This behavior is quite surprising especially since corresponding C++ code will behave normally.
Code to reproduce:

#include <pybind11/pybind11.h>
#include <iostream>

struct A
{
	// right signature
	A& operator -=(A const & ) { return *this; }

	// wrong signature, should be: T& T::operator +=(const T2& b);
	void operator +=(A const & ) {}
};

namespace py = pybind11;

PYBIND11_PLUGIN(example) {
    py::module m("example", "pybind11 example plugin");

	pybind11::class_<A> cl(m, "A");
	cl.def(pybind11::init<>());
	cl.def("__iadd__", &A::operator+=, "+= operator");
	cl.def("__isub__", &A::operator-=, "-= operator");

    return m.ptr();
}

In [1]: a1, a2 = A(), A()

In [2]: a1 -= a2

In [3]: print(a1)
<example.A object at 0x36dc538>

In [4]: a1 += a2

In [5]: print(a1)
None

[eacousineau]

It's kinda surprising, but kinda not, given that's how Python operators work?

If you write the same code in Python 3, you'll get the same thing:

class A:
    def __iadd__(self, other):
        pass

    def __isub__(self, other):
        return self

a1, a2 = A(), A()

a1 -= a2
print(a1)
# <__main__.A object at 0x7f34b275aa58>

a1 += a2
print(a1)
# None

[eacousineau]

From Python docs (emphasis mine):
https://docs.python.org/3/reference/datamodel.html#object.__iadd__

[...] These methods should attempt to do the operation in-place (modifying self) and return the result (which could be, but does not have to be, self). [...] In certain situations, augmented assignment can result in unexpected errors [...], but this behavior is in fact part of the data model.

[lyskov]

Thank you @eacousineau for looking this up! I agree that it make sense to assume that C++ operator should have correct signature. In case they does not: would it be too much trouble to add a fail safe and make sure that compilation fail if return type is unexpected?

[YannickJadoul]

Thank you @eacousineau for looking this up! I agree that it make sense to assume that C++ operator should have correct signature. In case they does not: would it be too much trouble to add a fail safe and make sure that compilation fail if return type is unexpected?

I think it is, because it is in the Python docs, would break consistency with Python (and might break some exotic cases where a different type is intentionally returned).

Using py::self += py::self or similar (see operators.h and https://pybind11.readthedocs.io/en/stable/advanced/classes.html#operator-overloading) will automatically solve this problem for you.

[YannickJadoul]

(Thanks, @eacousineau for answering this!)