From 30eb39ed79d1e2eeff15219ac00773034300a5e6 Mon Sep 17 00:00:00 2001 From: Yannick Jadoul Date: Wed, 16 Dec 2020 05:22:53 +0100 Subject: [PATCH] fix: also throw in the move-constructor added by the PYBIND11_OBJECT macro, after the argument has been moved-out (if necessary) (#2701) --- include/pybind11/pytypes.h | 8 ++++---- tests/test_pytypes.cpp | 11 +++++++---- tests/test_pytypes.py | 14 ++++++++------ 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 1010ad713..d8d58b1d5 100644 --- 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 index 113cf5cbb..709611d22 100644 --- 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 index 9e5c302e5..b1509a020 100644 --- 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():