From 88b99331a03afdec408fe41aad7c6ad324c5e397 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 12:41:02 -0400 Subject: [PATCH 01/15] Fix #3812 and fix const of inplace assignments --- include/pybind11/operators.h | 4 +- include/pybind11/pytypes.h | 59 +++++++++++++++++++++--------- tests/test_operator_overloading.py | 2 +- tests/test_pytypes.cpp | 10 +++++ 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index a0c3b78d65..67d547e9af 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -131,8 +131,8 @@ struct op_ { template \ struct op_impl { \ static char const *name() { return "__" #id "__"; } \ - static auto execute(L &l, const R &r) -> decltype(expr) { return expr; } \ - static B execute_cast(L &l, const R &r) { return B(expr); } \ + static auto execute(L &l, const R &r) -> decltype(expr) { return l = expr; } \ + static B execute_cast(L &l, const R &r) { return l = B(expr); } \ }; \ template \ op_ op(const self_t &, const T &) { \ diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index f9625e77ea..fd722c3b40 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -155,23 +155,23 @@ class object_api : public pyobject_tag { object operator-() const; object operator~() const; object operator+(object_api const &other) const; - object operator+=(object_api const &other) const; + object operator+=(object_api const &other); object operator-(object_api const &other) const; - object operator-=(object_api const &other) const; + object operator-=(object_api const &other); object operator*(object_api const &other) const; - object operator*=(object_api const &other) const; + object operator*=(object_api const &other); object operator/(object_api const &other) const; - object operator/=(object_api const &other) const; + object operator/=(object_api const &other); object operator|(object_api const &other) const; - object operator|=(object_api const &other) const; + object operator|=(object_api const &other); object operator&(object_api const &other) const; - object operator&=(object_api const &other) const; + object operator&=(object_api const &other); object operator^(object_api const &other) const; - object operator^=(object_api const &other) const; + object operator^=(object_api const &other); object operator<<(object_api const &other) const; - object operator<<=(object_api const &other) const; + object operator<<=(object_api const &other); object operator>>(object_api const &other) const; - object operator>>=(object_api const &other) const; + object operator>>=(object_api const &other); PYBIND11_DEPRECATED("Use py::str(obj) instead") pybind11::str str() const; @@ -334,6 +334,20 @@ class object : public handle { return *this; } +#define PYBIND11_INPLACE_OP(inp) \ + object inp(object_api const &other) { return operator=(handle::inp(other)); } + + PYBIND11_INPLACE_OP(operator+=) + PYBIND11_INPLACE_OP(operator-=) + PYBIND11_INPLACE_OP(operator*=) + PYBIND11_INPLACE_OP(operator/=) + PYBIND11_INPLACE_OP(operator|=) + PYBIND11_INPLACE_OP(operator&=) + PYBIND11_INPLACE_OP(operator^=) + PYBIND11_INPLACE_OP(operator<<=) + PYBIND11_INPLACE_OP(operator>>=) +#undef PYBIND11_INPLACE_OP + // Calling cast() on an object lvalue just copies (via handle::cast) template T cast() const &; @@ -2345,26 +2359,35 @@ bool object_api::rich_compare(object_api const &other, int value) const { return result; \ } +#define PYBIND11_MATH_OPERATOR_BINARY_INPLACE(op, fn) \ + template \ + object object_api::op(object_api const &other) { \ + object result = reinterpret_steal(fn(derived().ptr(), other.derived().ptr())); \ + if (!result.ptr()) \ + throw error_already_set(); \ + return result; \ + } + PYBIND11_MATH_OPERATOR_UNARY(operator~, PyNumber_Invert) PYBIND11_MATH_OPERATOR_UNARY(operator-, PyNumber_Negative) PYBIND11_MATH_OPERATOR_BINARY(operator+, PyNumber_Add) -PYBIND11_MATH_OPERATOR_BINARY(operator+=, PyNumber_InPlaceAdd) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator+=, PyNumber_InPlaceAdd) PYBIND11_MATH_OPERATOR_BINARY(operator-, PyNumber_Subtract) -PYBIND11_MATH_OPERATOR_BINARY(operator-=, PyNumber_InPlaceSubtract) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator-=, PyNumber_InPlaceSubtract) PYBIND11_MATH_OPERATOR_BINARY(operator*, PyNumber_Multiply) -PYBIND11_MATH_OPERATOR_BINARY(operator*=, PyNumber_InPlaceMultiply) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator*=, PyNumber_InPlaceMultiply) PYBIND11_MATH_OPERATOR_BINARY(operator/, PyNumber_TrueDivide) -PYBIND11_MATH_OPERATOR_BINARY(operator/=, PyNumber_InPlaceTrueDivide) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator/=, PyNumber_InPlaceTrueDivide) PYBIND11_MATH_OPERATOR_BINARY(operator|, PyNumber_Or) -PYBIND11_MATH_OPERATOR_BINARY(operator|=, PyNumber_InPlaceOr) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator|=, PyNumber_InPlaceOr) PYBIND11_MATH_OPERATOR_BINARY(operator&, PyNumber_And) -PYBIND11_MATH_OPERATOR_BINARY(operator&=, PyNumber_InPlaceAnd) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator&=, PyNumber_InPlaceAnd) PYBIND11_MATH_OPERATOR_BINARY(operator^, PyNumber_Xor) -PYBIND11_MATH_OPERATOR_BINARY(operator^=, PyNumber_InPlaceXor) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator^=, PyNumber_InPlaceXor) PYBIND11_MATH_OPERATOR_BINARY(operator<<, PyNumber_Lshift) -PYBIND11_MATH_OPERATOR_BINARY(operator<<=, PyNumber_InPlaceLshift) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator<<=, PyNumber_InPlaceLshift) PYBIND11_MATH_OPERATOR_BINARY(operator>>, PyNumber_Rshift) -PYBIND11_MATH_OPERATOR_BINARY(operator>>=, PyNumber_InPlaceRshift) +PYBIND11_MATH_OPERATOR_BINARY_INPLACE(operator>>=, PyNumber_InPlaceRshift) #undef PYBIND11_MATH_OPERATOR_UNARY #undef PYBIND11_MATH_OPERATOR_BINARY diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index b228da3cc3..b14c45901e 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -77,7 +77,7 @@ def test_operator_overloading(): assert cstats.default_constructions == 0 assert cstats.copy_constructions == 0 assert cstats.move_constructions >= 10 - assert cstats.copy_assignments == 0 + assert cstats.copy_assignments == 6 assert cstats.move_assignments == 0 diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index cb81007c37..da881d0c37 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -695,4 +695,14 @@ TEST_SUBMODULE(pytypes, m) { } return o; }); + + // testing immutable object augmented assignment: #issue 3812 + m.def("inplace_append", [](py::str &a, const py::str &b) -> py::str { + a += b; + return a; + }); + m.def("inplace_append", [](py::int_ &a, const py::int_ &b) { + a += b; + return a; + }); } From 42bdd3b450c822002999c91861d8586eefb6b670 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 13:12:49 -0400 Subject: [PATCH 02/15] Fix missing tests --- tests/test_pytypes.cpp | 17 +++++++++-------- tests/test_pytypes.py | 7 +++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index da881d0c37..e59be2851a 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -39,6 +39,11 @@ class float_ : public py::object { }; } // namespace external +template +T inplace_append(T &a, const T &b) { + a += b; + return a; +} TEST_SUBMODULE(pytypes, m) { // test_bool m.def("get_bool", [] { return py::bool_(false); }); @@ -697,12 +702,8 @@ TEST_SUBMODULE(pytypes, m) { }); // testing immutable object augmented assignment: #issue 3812 - m.def("inplace_append", [](py::str &a, const py::str &b) -> py::str { - a += b; - return a; - }); - m.def("inplace_append", [](py::int_ &a, const py::int_ &b) { - a += b; - return a; - }); + m.def("inplace_append", &inplace_append); + m.def("inplace_append", &inplace_append); + m.def("inplace_append", &inplace_append); + m.def("inplace_append", &inplace_append); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 3e9d51a27c..d2a63a7ea6 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -734,3 +734,10 @@ def test_populate_obj_str_attrs(): new_attrs = {k: v for k, v in new_o.__dict__.items() if not k.startswith("_")} assert all(isinstance(v, str) for v in new_attrs.values()) assert len(new_attrs) == pop + + +@pytest.mark.parametrize( + "a,b", [("foo", "bar"), (1, 2), (1.0, 2.0), (list(range(3)), list(range(3, 6)))] +) +def test_inplace_append(a, b): + assert m.inplace_append(a, b) == (a + b) From aac0c0ac6b696244acb3cdeb96eab6d243a20d17 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 13:20:42 -0400 Subject: [PATCH 03/15] Revert operator overloading changes --- include/pybind11/operators.h | 4 ++-- tests/test_operator_overloading.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index 67d547e9af..a0c3b78d65 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -131,8 +131,8 @@ struct op_ { template \ struct op_impl { \ static char const *name() { return "__" #id "__"; } \ - static auto execute(L &l, const R &r) -> decltype(expr) { return l = expr; } \ - static B execute_cast(L &l, const R &r) { return l = B(expr); } \ + static auto execute(L &l, const R &r) -> decltype(expr) { return expr; } \ + static B execute_cast(L &l, const R &r) { return B(expr); } \ }; \ template \ op_ op(const self_t &, const T &) { \ diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index b14c45901e..b228da3cc3 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -77,7 +77,7 @@ def test_operator_overloading(): assert cstats.default_constructions == 0 assert cstats.copy_constructions == 0 assert cstats.move_constructions >= 10 - assert cstats.copy_assignments == 6 + assert cstats.copy_assignments == 0 assert cstats.move_assignments == 0 From 8d70b5e91502927b57ecfbaf8b7ffa98b31c9700 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 13:32:06 -0400 Subject: [PATCH 04/15] calculate answer first for tests --- tests/test_pytypes.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 541a7f98cc..57e95a0ebd 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -745,4 +745,5 @@ def test_populate_obj_str_attrs(): "a,b", [("foo", "bar"), (1, 2), (1.0, 2.0), (list(range(3)), list(range(3, 6)))] ) def test_inplace_append(a, b): - assert m.inplace_append(a, b) == (a + b) + expected = a + b + assert m.inplace_append(a, b) == expected From 13dc61b81960cf568bae818e56427cd21e807070 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 13:43:24 -0400 Subject: [PATCH 05/15] Simplify tests --- tests/test_pytypes.cpp | 18 ++++++++---------- tests/test_pytypes.py | 6 ++++++ 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 16713ac263..f2264e426e 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -98,12 +98,6 @@ void m_defs(py::module_ &m) { } // namespace handle_from_move_only_type_with_operator_PyObject -template -T inplace_append(T &a, const T &b) { - a += b; - return a; -} - TEST_SUBMODULE(pytypes, m) { handle_from_move_only_type_with_operator_PyObject::m_defs(m); @@ -764,8 +758,12 @@ TEST_SUBMODULE(pytypes, m) { }); // testing immutable object augmented assignment: #issue 3812 - m.def("inplace_append", &inplace_append); - m.def("inplace_append", &inplace_append); - m.def("inplace_append", &inplace_append); - m.def("inplace_append", &inplace_append); + m.def("inplace_append", [](py::object &a, const py::object &b) { + a += b; + return a; + }); + m.def("inplace_subtract", [](py::object &a, const py::object &b) { + a -= b; + return a; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 57e95a0ebd..1505e5091e 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -747,3 +747,9 @@ def test_populate_obj_str_attrs(): def test_inplace_append(a, b): expected = a + b assert m.inplace_append(a, b) == expected + + +@pytest.mark.parametrize("a,b", [(3, 2), (3.0, 2.0), (set(range(3)), set(range(2)))]) +def test_inplace_subtract(a, b): + expected = a - b + assert m.inplace_subtract(a, b) == expected From 42fe40ee025645e043f8c681b74c7bbe9475746c Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 13:50:07 -0400 Subject: [PATCH 06/15] Add more tests --- tests/test_pytypes.cpp | 8 ++++++++ tests/test_pytypes.py | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index f2264e426e..1e7c69a7e5 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -766,4 +766,12 @@ TEST_SUBMODULE(pytypes, m) { a -= b; return a; }); + m.def("inplace_multiply", [](py::object &a, const py::object &b) { + a *= b; + return a; + }); + m.def("inplace_divide", [](py::object &a, const py::object &b) { + a /= b; + return a; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 1505e5091e..27088ee304 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -753,3 +753,15 @@ def test_inplace_append(a, b): def test_inplace_subtract(a, b): expected = a - b assert m.inplace_subtract(a, b) == expected + + +@pytest.mark.parametrize("a,b", [(3, 2), (3.0, 2.0), ([1], 3)]) +def test_inplace_multiply(a, b): + expected = a * b + assert m.inplace_multiply(a, b) == expected + + +@pytest.mark.parametrize("a,b", [(6, 3), (6.0, 3.0)]) +def test_inplace_divide(a, b): + expected = a / b + assert m.inplace_divide(a, b) == expected From 42c91f1d76cf6477bf0e3a37feccb8f5435817b0 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Thu, 14 Jul 2022 14:01:03 -0400 Subject: [PATCH 07/15] Add a couple more tests --- tests/test_pytypes.cpp | 8 ++++++++ tests/test_pytypes.py | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 1e7c69a7e5..9398fd8880 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -774,4 +774,12 @@ TEST_SUBMODULE(pytypes, m) { a /= b; return a; }); + m.def("inplace_or", [](py::object &a, const py::object &b) { + a |= b; + return a; + }); + m.def("inplace_and", [](py::object &a, const py::object &b) { + a &= b; + return a; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 27088ee304..6d5f05f5c1 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -765,3 +765,37 @@ def test_inplace_multiply(a, b): def test_inplace_divide(a, b): expected = a / b assert m.inplace_divide(a, b) == expected + + +@pytest.mark.parametrize( + "a,b", + [ + (False, True), + ( + set(), + { + 1, + }, + ), + ], +) +def test_inplace_or(a, b): + expected = a | b + assert m.inplace_or(a, b) == expected + + +@pytest.mark.parametrize( + "a,b", + [ + (True, False), + ( + {1, 2, 3}, + { + 1, + }, + ), + ], +) +def test_inplace_and(a, b): + expected = a & b + assert m.inplace_and(a, b) == expected From 9f2e45cfa6b0e44e4dd9fa9e8e3201d69fed368d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 15 Jul 2022 13:08:44 -0700 Subject: [PATCH 08/15] Add test_inplace_lshift, test_inplace_rshift for completeness. --- tests/test_pytypes.cpp | 8 ++++++++ tests/test_pytypes.py | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 9398fd8880..81387fd9f5 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -782,4 +782,12 @@ TEST_SUBMODULE(pytypes, m) { a &= b; return a; }); + m.def("inplace_lshift", [](py::object &a, const py::object &b) { + a <<= b; + return a; + }); + m.def("inplace_rshift", [](py::object &a, const py::object &b) { + a >>= b; + return a; + }); } diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 6d5f05f5c1..05da7572a7 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -799,3 +799,15 @@ def test_inplace_or(a, b): def test_inplace_and(a, b): expected = a & b assert m.inplace_and(a, b) == expected + + +def test_inplace_lshift(): + a, b = 2, 3 + expected = a << b + assert m.inplace_lshift(a, b) == expected + + +def test_inplace_rshift(): + a, b = 2, 3 + expected = a >> b + assert m.inplace_rshift(a, b) == expected From 8efaec1b0553216130d447c2b6ae74b3ef408fa8 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 17 Jul 2022 13:06:11 -0400 Subject: [PATCH 09/15] Update tests --- tests/test_pytypes.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py index 05da7572a7..bde8317386 100644 --- a/tests/test_pytypes.py +++ b/tests/test_pytypes.py @@ -801,13 +801,13 @@ def test_inplace_and(a, b): assert m.inplace_and(a, b) == expected -def test_inplace_lshift(): - a, b = 2, 3 +@pytest.mark.parametrize("a,b", [(8, 1), (-3, 2)]) +def test_inplace_lshift(a, b): expected = a << b assert m.inplace_lshift(a, b) == expected -def test_inplace_rshift(): - a, b = 2, 3 +@pytest.mark.parametrize("a,b", [(8, 1), (-2, 2)]) +def test_inplace_rshift(a, b): expected = a >> b assert m.inplace_rshift(a, b) == expected From fdf36ceca0087aad5cf81357896102f4a023edcd Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 17 Jul 2022 13:19:51 -0400 Subject: [PATCH 10/15] Shortcircuit on self assigment and address reviewer comment --- include/pybind11/pytypes.h | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index c297a44f03..45ab6aa8d0 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -334,12 +334,14 @@ class object : public handle { } object &operator=(const object &other) { - other.inc_ref(); - // Use temporary variable to ensure `*this` remains valid while - // `Py_XDECREF` executes, in case `*this` is accessible from Python. - handle temp(m_ptr); - m_ptr = other.m_ptr; - temp.dec_ref(); + if (this != &other) { + other.inc_ref(); + // Use temporary variable to ensure `*this` remains valid while + // `Py_XDECREF` executes, in case `*this` is accessible from Python. + handle temp(m_ptr); + m_ptr = other.m_ptr; + temp.dec_ref(); + } return *this; } @@ -353,8 +355,8 @@ class object : public handle { return *this; } -#define PYBIND11_INPLACE_OP(inp) \ - object inp(object_api const &other) { return operator=(handle::inp(other)); } +#define PYBIND11_INPLACE_OP(iop) \ + object iop(object_api const &other) { return operator=(handle::iop(other)); } PYBIND11_INPLACE_OP(operator+=) PYBIND11_INPLACE_OP(operator-=) From 35832bfae34548d7d5b40296f06b5c2d3d8cd765 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 17 Jul 2022 13:33:06 -0400 Subject: [PATCH 11/15] broaden skip for self assignment --- include/pybind11/pytypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 45ab6aa8d0..05e38d8a44 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -334,7 +334,7 @@ class object : public handle { } object &operator=(const object &other) { - if (this != &other) { + if (!this->is(other)) { other.inc_ref(); // Use temporary variable to ensure `*this` remains valid while // `Py_XDECREF` executes, in case `*this` is accessible from Python. @@ -346,7 +346,7 @@ class object : public handle { } object &operator=(object &&other) noexcept { - if (this != &other) { + if (this != &other && !this->is(other)) { handle temp(m_ptr); m_ptr = other.m_ptr; other.m_ptr = nullptr; From 352871241b3207213b3b70cfe9bbe4991f35c8c3 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Sun, 17 Jul 2022 13:37:11 -0400 Subject: [PATCH 12/15] One more reviewer comment --- include/pybind11/pytypes.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 05e38d8a44..657972a591 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -2380,9 +2380,9 @@ bool object_api::rich_compare(object_api const &other, int value) const { return result; \ } -#define PYBIND11_MATH_OPERATOR_BINARY_INPLACE(op, fn) \ +#define PYBIND11_MATH_OPERATOR_BINARY_INPLACE(iop, fn) \ template \ - object object_api::op(object_api const &other) { \ + object object_api::iop(object_api const &other) { \ object result = reinterpret_steal(fn(derived().ptr(), other.derived().ptr())); \ if (!result.ptr()) \ throw error_already_set(); \ From 89bf6ec938cb36b32fc51bab457d85e8066b43d5 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 19 Jul 2022 11:30:38 -0400 Subject: [PATCH 13/15] Document opt behavior and make consistent --- include/pybind11/pytypes.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 657972a591..df336875b3 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -334,6 +334,7 @@ class object : public handle { } object &operator=(const object &other) { + // Do not copy if src and destination object are the same if (!this->is(other)) { other.inc_ref(); // Use temporary variable to ensure `*this` remains valid while @@ -346,11 +347,11 @@ class object : public handle { } object &operator=(object &&other) noexcept { - if (this != &other && !this->is(other)) { + if (this != &other) { handle temp(m_ptr); m_ptr = other.m_ptr; - other.m_ptr = nullptr; temp.dec_ref(); + other.m_ptr = nullptr; } return *this; } From aa0853ba1c12180fcbbe90bbfc274c90282e0a91 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 19 Jul 2022 11:36:33 -0400 Subject: [PATCH 14/15] Revert unnecessary change --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index df336875b3..145a6504ac 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -350,8 +350,8 @@ class object : public handle { if (this != &other) { handle temp(m_ptr); m_ptr = other.m_ptr; - temp.dec_ref(); other.m_ptr = nullptr; + temp.dec_ref(); } return *this; } From 552a3ae0259a1c967ec9f2ca3fe357fc39eb8691 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Tue, 19 Jul 2022 11:39:56 -0400 Subject: [PATCH 15/15] Clarify comment --- include/pybind11/pytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 145a6504ac..ad5edfed9f 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -334,7 +334,7 @@ class object : public handle { } object &operator=(const object &other) { - // Do not copy if src and destination object are the same + // Skip inc_ref and dec_ref if both objects are the same if (!this->is(other)) { other.inc_ref(); // Use temporary variable to ensure `*this` remains valid while