fix: also throw in the move-constructor added by the PYBIND11_OBJECT …

…macro, after the argument has been moved-out (if necessary) (#2701)
pybind · Dec 16, 2020 · 30eb39e · 30eb39e
1 parent d9fa705
commit 30eb39e
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 14 deletions.
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
@@ -812,18 +812,18 @@ PYBIND11_NAMESPACE_END(detail)
     : Parent(check_(o) ? o.release().ptr() : ConvertFun(o.ptr()), stolen_t{}) \
     { if (!m_ptr) throw error_already_set(); }
 
-#define PYBIND11_OBJECT_CHECK_FAILED(Name, o) \
+#define PYBIND11_OBJECT_CHECK_FAILED(Name, o_ptr) \
     ::pybind11::type_error("Object of type '" + \
-                           ::pybind11::detail::get_fully_qualified_tp_name(Py_TYPE(o.ptr())) + \
+                           ::pybind11::detail::get_fully_qualified_tp_name(Py_TYPE(o_ptr)) + \
                            "' is not an instance of '" #Name "'")
 
 #define PYBIND11_OBJECT(Name, Parent, CheckFun) \
     PYBIND11_OBJECT_COMMON(Name, Parent, CheckFun) \
     /* This is deliberately not 'explicit' to allow implicit conversion from object: */ \
     Name(const object &o) : Parent(o) \
-    { if (o && !check_(o)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, o); } \
+    { if (m_ptr && !check_(m_ptr)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, m_ptr); } \
     Name(object &&o) : Parent(std::move(o)) \
-    { if (o && !check_(o)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, o); }
+    { if (m_ptr && !check_(m_ptr)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, m_ptr); }
 
 #define PYBIND11_OBJECT_DEFAULT(Name, Parent, CheckFun) \
     PYBIND11_OBJECT(Name, Parent, CheckFun) \

diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp
@@ -254,15 +254,18 @@ TEST_SUBMODULE(pytypes, m) {
 
     m.def("convert_to_pybind11_str", [](py::object o) { return py::str(o); });
 
-    m.def("nonconverting_constructor", [](std::string type, py::object value) -> py::object {
+    m.def("nonconverting_constructor", [](std::string type, py::object value, bool move) -> py::object {
         if (type == "bytes") {
-            return py::bytes(value);
+            return move ? py::bytes(std::move(value)) : py::bytes(value);
         }
         else if (type == "none") {
-            return py::none(value);
+            return move ? py::none(std::move(value)) : py::none(value);
         }
         else if (type == "ellipsis") {
-            return py::ellipsis(value);
+            return move ? py::ellipsis(std::move(value)) : py::ellipsis(value);
+        }
+        else if (type == "type") {
+            return move ? py::type(std::move(value)) : py::type(value);
         }
         throw std::runtime_error("Invalid type");
     });

diff --git a/tests/test_pytypes.py b/tests/test_pytypes.py
@@ -268,14 +268,16 @@ def test_non_converting_constructors():
         ("bytes", range(10)),
         ("none", 42),
         ("ellipsis", 42),
+        ("type", 42),
     ]
     for t, v in non_converting_test_cases:
-        with pytest.raises(TypeError) as excinfo:
-            m.nonconverting_constructor(t, v)
-        expected_error = "Object of type '{}' is not an instance of '{}'".format(
-            type(v).__name__, t
-        )
-        assert str(excinfo.value) == expected_error
+        for move in [True, False]:
+            with pytest.raises(TypeError) as excinfo:
+                m.nonconverting_constructor(t, v, move)
+            expected_error = "Object of type '{}' is not an instance of '{}'".format(
+                type(v).__name__, t
+            )
+            assert str(excinfo.value) == expected_error
 
 
 def test_pybind11_str_raw_str():