From 2bab5793f75ddbd38b5639ec8c1fb822b7ad505d Mon Sep 17 00:00:00 2001
From: Dean Moldovan <dean0x7d@gmail.com>
Date: Fri, 23 Sep 2016 00:27:59 +0200
Subject: [PATCH] Later assignments to accessors should not overwrite the
 original field

`auto var = l[0]` has a strange quirk: `var` is actually an accessor and
not an object, so any later assignment of `var = ...` would modify l[0]
instead of `var`. This is surprising compared to the non-auto assignment
`py::object var = l[0]; var = ...`.

By overloading `operator=` on lvalue/rvalue, the expected behavior is
restored even for `auto` variables.
---
 include/pybind11/pybind11.h |  1 +
 include/pybind11/pytypes.h  | 11 +++++++----
 tests/test_python_types.cpp | 17 +++++++++++++++++
 tests/test_python_types.py  |  9 ++++++++-
 4 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
index 3e81d4bb..686fe536 100644
--- a/include/pybind11/pybind11.h
+++ b/include/pybind11/pybind11.h
@@ -18,6 +18,7 @@
 #  pragma warning(disable: 4800) // warning C4800: 'int': forcing value to bool 'true' or 'false' (performance warning)
 #  pragma warning(disable: 4996) // warning C4996: The POSIX name for this item is deprecated. Instead, use the ISO C and C++ conformant name
 #  pragma warning(disable: 4702) // warning C4702: unreachable code
+#  pragma warning(disable: 4522) // warning C4522: multiple assignment operators specified
 #elif defined(__INTEL_COMPILER)
 #  pragma warning(push)
 #  pragma warning(disable: 186)   // pointless comparison of unsigned integer with zero
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
index 8fa01ff8..d4c6f39a 100644
--- a/include/pybind11/pytypes.h
+++ b/include/pybind11/pytypes.h
@@ -199,16 +199,19 @@ class accessor : public object_api<accessor<Policy>> {
 public:
     accessor(handle obj, key_type key) : obj(obj), key(std::move(key)) { }
 
-    void operator=(const accessor &a) { operator=(handle(a)); }
-    void operator=(const object &o) { operator=(handle(o)); }
-    void operator=(handle value) { Policy::set(obj, key, value); }
+    void operator=(const accessor &a) && { std::move(*this).operator=(handle(a)); }
+    void operator=(const accessor &a) & { operator=(handle(a)); }
+    void operator=(const object &o) && { std::move(*this).operator=(handle(o)); }
+    void operator=(const object &o) & { operator=(handle(o)); }
+    void operator=(handle value) && { Policy::set(obj, key, value); }
+    void operator=(handle value) & { get_cache() = object(value, true); }
 
     operator object() const { return get_cache(); }
     PyObject *ptr() const { return get_cache().ptr(); }
     template <typename T> T cast() const { return get_cache().template cast<T>(); }
 
 private:
-    const object &get_cache() const {
+    object &get_cache() const {
         if (!cache) { cache = Policy::get(obj, key); }
         return cache;
     }
diff --git a/tests/test_python_types.cpp b/tests/test_python_types.cpp
index fabc78e8..678a56d1 100644
--- a/tests/test_python_types.cpp
+++ b/tests/test_python_types.cpp
@@ -272,4 +272,21 @@ test_initializer python_types([](py::module &m) {
         }
         return py::tuple();
     });
+
+    m.def("test_accessor_assignment", []() {
+        auto l = py::list(1);
+        l[0] = py::cast(0);
+
+        auto d = py::dict();
+        d["get"] = l[0];
+        auto var = l[0];
+        d["deferred_get"] = var;
+        l[0] = py::cast(1);
+        d["set"] = l[0];
+        var = py::cast(99); // this assignment should not overwrite l[0]
+        d["deferred_set"] = l[0];
+        d["var"] = var;
+
+        return d;
+    });
 });
diff --git a/tests/test_python_types.py b/tests/test_python_types.py
index a6cf1fe0..628e4612 100644
--- a/tests/test_python_types.py
+++ b/tests/test_python_types.py
@@ -251,7 +251,7 @@ def test_dict_api():
 
 
 def test_accessors():
-    from pybind11_tests import test_accessor_api, test_tuple_accessor
+    from pybind11_tests import test_accessor_api, test_tuple_accessor, test_accessor_assignment
 
     class SubTestObject:
         attr_obj = 1
@@ -280,3 +280,10 @@ def test_accessors():
     assert d["operator*"] == 7
 
     assert test_tuple_accessor(tuple()) == (0, 1, 2)
+
+    d = test_accessor_assignment()
+    assert d["get"] == 0
+    assert d["deferred_get"] == 0
+    assert d["set"] == 1
+    assert d["deferred_set"] == 1
+    assert d["var"] == 99