From 6a7e9f42fe6301e2aa02d5048d8bf752236053db Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 3 Mar 2021 05:08:47 -0800 Subject: [PATCH] Changing all but one std::runtime_error to std::invalid_argument, which appears as ValueError in the Python interpreter. Adding `test_cannot_disown_use_count_ne_1`. (#2883) --- include/pybind11/detail/smart_holder_poc.h | 34 +++++++++++----------- tests/test_class_sh_basic.cpp | 9 ++++++ tests/test_class_sh_basic.py | 20 +++++++++++++ 3 files changed, 46 insertions(+), 17 deletions(-) diff --git a/include/pybind11/detail/smart_holder_poc.h b/include/pybind11/detail/smart_holder_poc.h index 99b1fa9b4..a70ffa62d 100644 --- a/include/pybind11/detail/smart_holder_poc.h +++ b/include/pybind11/detail/smart_holder_poc.h @@ -104,7 +104,7 @@ struct smart_holder { bool is_populated : 1; // Design choice: smart_holder is movable but not copyable. - smart_holder(smart_holder &&) = default; + smart_holder(smart_holder &&) = default; smart_holder(const smart_holder &) = delete; smart_holder &operator=(smart_holder &&) = default; smart_holder &operator=(const smart_holder &) = delete; @@ -124,8 +124,8 @@ struct smart_holder { template static void ensure_pointee_is_destructible(const char *context) { if (!std::is_destructible::value) - throw std::runtime_error(std::string("Pointee is not destructible (") + context - + ")."); + throw std::invalid_argument(std::string("Pointee is not destructible (") + context + + ")."); } void ensure_is_populated(const char *context) const { @@ -136,16 +136,16 @@ struct smart_holder { void ensure_vptr_is_using_builtin_delete(const char *context) const { if (vptr_is_external_shared_ptr) { - throw std::runtime_error(std::string("Cannot disown external shared_ptr (") + context - + ")."); + throw std::invalid_argument(std::string("Cannot disown external shared_ptr (") + + context + ")."); } if (vptr_is_using_noop_deleter) { - throw std::runtime_error(std::string("Cannot disown non-owning holder (") + context - + ")."); + throw std::invalid_argument(std::string("Cannot disown non-owning holder (") + context + + ")."); } if (!vptr_is_using_builtin_delete) { - throw std::runtime_error(std::string("Cannot disown custom deleter (") + context - + ")."); + throw std::invalid_argument(std::string("Cannot disown custom deleter (") + context + + ")."); } } @@ -154,25 +154,25 @@ struct smart_holder { const std::type_info *rtti_requested = &typeid(D); if (!rtti_uqp_del) { if (!is_std_default_delete(*rtti_requested)) { - throw std::runtime_error(std::string("Missing unique_ptr deleter (") + context - + ")."); + throw std::invalid_argument(std::string("Missing unique_ptr deleter (") + context + + ")."); } ensure_vptr_is_using_builtin_delete(context); } else if (!(*rtti_requested == *rtti_uqp_del)) { - throw std::runtime_error(std::string("Incompatible unique_ptr deleter (") + context - + ")."); + throw std::invalid_argument(std::string("Incompatible unique_ptr deleter (") + context + + ")."); } } void ensure_has_pointee(const char *context) const { if (!has_pointee()) { - throw std::runtime_error(std::string("Disowned holder (") + context + ")."); + throw std::invalid_argument(std::string("Disowned holder (") + context + ")."); } } void ensure_use_count_1(const char *context) const { if (vptr.get() == nullptr) { - throw std::runtime_error(std::string("Cannot disown nullptr (") + context + ")."); + throw std::invalid_argument(std::string("Cannot disown nullptr (") + context + ")."); } // In multithreaded environments accessing use_count can lead to // race conditions, but in the context of Python it is a bug (elsewhere) @@ -180,8 +180,8 @@ struct smart_holder { // is reached. // SMART_HOLDER_WIP: IMPROVABLE: assert(GIL is held). if (vptr.use_count() != 1) { - throw std::runtime_error(std::string("Cannot disown use_count != 1 (") + context - + ")."); + throw std::invalid_argument(std::string("Cannot disown use_count != 1 (") + context + + ")."); } } diff --git a/tests/test_class_sh_basic.cpp b/tests/test_class_sh_basic.cpp index f66b5623d..9ec947baf 100644 --- a/tests/test_class_sh_basic.cpp +++ b/tests/test_class_sh_basic.cpp @@ -4,6 +4,7 @@ #include #include +#include namespace pybind11_tests { namespace class_sh_basic { @@ -57,11 +58,16 @@ std::string pass_udcp(std::unique_ptr obj) { return "pass_udcp // Helpers for testing. std::string get_mtxt(atyp const &obj) { return obj.mtxt; } std::unique_ptr unique_ptr_roundtrip(std::unique_ptr obj) { return obj; } +struct SharedPtrStash { + std::vector> stash; + void Add(std::shared_ptr obj) { stash.push_back(obj); } +}; } // namespace class_sh_basic } // namespace pybind11_tests PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::atyp) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::SharedPtrStash) namespace pybind11_tests { namespace class_sh_basic { @@ -110,6 +116,9 @@ TEST_SUBMODULE(class_sh_basic, m) { // These require selected functions above to work first, as indicated: m.def("get_mtxt", get_mtxt); // pass_cref m.def("unique_ptr_roundtrip", unique_ptr_roundtrip); // pass_uqmp, rtrn_uqmp + py::classh(m, "SharedPtrStash") + .def(py::init<>()) + .def("Add", &SharedPtrStash::Add, py::arg("obj")); m.def("py_type_handle_of_atyp", []() { return py::type::handle_of(); // Exercises static_cast in this function. diff --git a/tests/test_class_sh_basic.py b/tests/test_class_sh_basic.py index 7842cfdf8..9e2813a96 100644 --- a/tests/test_class_sh_basic.py +++ b/tests/test_class_sh_basic.py @@ -85,6 +85,26 @@ def test_pass_unique_ptr_disowns(pass_f, rtrn_f, expected): ) +@pytest.mark.parametrize( + "pass_f, rtrn_f", + [ + (m.pass_uqmp, m.rtrn_uqmp), + (m.pass_uqcp, m.rtrn_uqcp), + (m.pass_udmp, m.rtrn_udmp), + (m.pass_udcp, m.rtrn_udcp), + ], +) +def test_cannot_disown_use_count_ne_1(pass_f, rtrn_f): + obj = rtrn_f() + stash = m.SharedPtrStash() + stash.Add(obj) + with pytest.raises(ValueError) as exc_info: + pass_f(obj) + assert str(exc_info.value) == ( + "Cannot disown use_count != 1 (loaded_as_unique_ptr)." + ) + + def test_unique_ptr_roundtrip(num_round_trips=1000): # Multiple roundtrips to stress-test instance registration/deregistration. recycled = m.atyp("passenger")