Skip to content

[BUG]: py::object augmented assignment doesn't work with immutable types #3812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
3 tasks done
oremanj opened this issue Mar 18, 2022 · 0 comments · Fixed by #4065
Closed
3 tasks done

[BUG]: py::object augmented assignment doesn't work with immutable types #3812

oremanj opened this issue Mar 18, 2022 · 0 comments · Fixed by #4065

Comments

@oremanj
Copy link
Contributor

oremanj commented Mar 18, 2022

Required prerequisites

Problem description

The augmented assignment operators (operator+=, etc) on py::object are implemented analogously to the regular binary operators (operator+, etc): they call one of the PyNumber_ API functions and return the result. For immutable types such as py::str, PyNumber_InPlaceAdd is implemented the same as PyNumber_Add, so you need to look at the result in order to see any change; in both C++ and Python, though, foo += bar is supposed to be sufficient to update foo on its own without observing the result of the expression.

Reproducible example code

#include <pybind11/pybind11.h>

namespace py = pybind11;
using namespace pybind11::literals;

PYBIND11_MODULE(example, m) {
    m.def("test", []() -> py::str {
        py::str text = "foo"_s;
        text += " bar"_s;
        return text;
    });
}

// import example
// assert example.test() == "foo bar"
//    --> fails: example.test() actually returns "foo"
@oremanj oremanj added the triage New bug, unverified label Mar 18, 2022
@Skylion007 Skylion007 added bug and removed triage New bug, unverified labels Mar 18, 2022
Skylion007 added a commit to Skylion007/pybind11 that referenced this issue Jul 14, 2022
Skylion007 added a commit that referenced this issue Jul 20, 2022
* Fix #3812 and fix const of inplace assignments

* Fix missing tests

* Revert operator overloading changes

* calculate answer first for tests

* Simplify tests

* Add more tests

* Add a couple more tests

* Add test_inplace_lshift, test_inplace_rshift for completeness.

* Update tests

* Shortcircuit on self assigment and address reviewer comment

* broaden skip for self assignment

* One more reviewer comment

* Document opt behavior and make consistent

* Revert unnecessary change

* Clarify comment

Co-authored-by: Ralf W. Grosse-Kunstleve <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants