Pybind11: [BUG] Move operation was incorrectly used for constant reference

[Shixiaowei02]

Issue description

I used the following operations to try to bind functions that return C++ static data objects, but found that they accidentally used rvalues. Cause the loss of global data in C++.

reference https://github.com/pybind/pybind11/issues/1561

Reproducible example code

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

#include "pybind11/stl.h"

namespace {

namespace py = pybind11;

struct Vector {
  explicit Vector(std::vector<int>&& vec) : 
    vec_(std::forward<std::vector<int>>(vec)) {}
  Vector(const Vector& other) : vec_(other.vec_) {
    std::cout << "copy ctor here!" << std::endl;
  }
  Vector(Vector&& other) : vec_(std::move(other.vec_)) {
    std::cout << "move ctor here!" << std::endl;
  }
  uint32_t size() const {
    return vec_.size();
  }
  std::vector<int> vec_;
};

static std::unordered_map<int, Vector> g_version_map;

struct FillMap {
  FillMap() {
    std::cout << "Fill Map Start..." << std::endl;
    std::pair<int, Vector> pair{1, Vector{std::vector<int>{1, 2, 3}}};
    g_version_map.insert(std::move(pair));
    std::cout << "Fill Map End..." << std::endl;
  }
};

static FillMap fill_map;

const std::unordered_map<int, Vector>& get_map() {
  return g_version_map;
}

void BindVector(py::module *m) {
  py::class_<Vector>(*m, "Vector")
      .def(py::init<const Vector&>())
      .def("size", &Vector::size, py::return_value_policy::reference);
  m->def("get_map", &get_map, py::return_value_policy::reference);
}

}
PYBIND11_MODULE(bug, m) {
  BindVector(&m);
}

g++ -Ipybind/include -Icpython-3.7.0/include/python3.7m -O3 -Wall -shared -std=c++11 -fPIC `python -m pybind11 --includes` bug.cc -o bug`python3.7m-config --extension-suffix`

Python 3.7.0 (default, Nov 24 2018, 08:00:09)
[GCC 4.8.2 20140120 (Red Hat 4.8.2-15)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import bug
Fill Map Start...
move ctor here!
move ctor here!
Fill Map End...
>>> bug.get_map()[1].size()
move ctor here!
3
>>> bug.get_map()[1].size()
move ctor here!
0

EDIT (@YannickJadoul):

After reading, remove this checklist and the template text in parentheses below.

[YannickJadoul]

@Shixiaowei02, please show us a complete minimal, so we can reproduce, including the includes.
Are you using stl.h? Those includes are important information.

[Shixiaowei02]

@Shixiaowei02, please show us a complete minimal, so we can reproduce, including the includes.
Are you using stl.h? Those includes are important information.

thank you for your reply. I encountered a problem with incremental development in a larger project. But I am sure that constant references should not be moved out. The above is a simple code that illustrates the intent.

[YannickJadoul]

The above is a simple code that illustrates the intent.

Sure, but can you please extend this and make it into a complete, self-contained example that demonstrates the issue? I want to be able to reproduce the same issue as you on my computer, so that I can debug your problem.

[Shixiaowei02]

The above is a simple code that illustrates the intent.

Sure, but can you please extend this and make it into a complete, self-contained example that demonstrates the issue? I want to be able to reproduce the same issue as you on my computer, so that I can debug your problem.

I have provided a unit test and can successfully reproduce the problem locally. Please check, thank you very much for your follow-up!

[bstaletic]

>>> bug.get_map()[1].size()
move ctor here!
3
>>> bug.get_map()[1].size()
move ctor here!
0

I definitely can't repro with your exact code:

>>> import bug
Fill Map Start...
move ctor here!
move ctor here!
Fill Map End...
>>> bug.
bug.Vector(   bug.get_map(
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3

Which version of pybind11 are you on? We've recently released 2.6.0.

[Shixiaowei02]

>>> bug.get_map()[1].size()
move ctor here!
3
>>> bug.get_map()[1].size()
move ctor here!
0

I definitely can't repro with your exact code:

>>> import bug
Fill Map Start...
move ctor here!
move ctor here!
Fill Map End...
>>> bug.
bug.Vector(   bug.get_map(
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()
{1: <bug.Vector object at 0x7f367ca685f0>}
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1]
<bug.Vector object at 0x7f367ca685f0>
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3
>>> bug.get_map()[1].size()
3

Which version of pybind11 are you on? We've recently released 2.6.0.

I'm using pybind 2.2.4, and it seems that the latest version fixes this problem, let me try.

[YannickJadoul]

Thank you. 2.2.4 is very old, by the way: https://github.com/pybind/pybind11/releases/tag/v2.2.4 says "Sep 11, 2018", so there's been quite a few fixes since, I assume.