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)

Author: rwgk

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 <typename T>
     static void ensure_pointee_is_destructible(const char *context) {
         if (!std::is_destructible<T>::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,34 +154,34 @@ struct smart_holder {
         const std::type_info *rtti_requested = &typeid(D);
         if (!rtti_uqp_del) {
             if (!is_std_default_delete<T>(*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)
         // if the Global Interpreter Lock (GIL) is not being held when this code
         // 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
+                                        + ").");
         }
     }
 

tests/test_class_sh_basic.cpp

@@ -4,6 +4,7 @@
 
 #include <memory>
 #include <string>
+#include <vector>
 
 namespace pybind11_tests {
 namespace class_sh_basic {
@@ -57,11 +58,16 @@ std::string pass_udcp(std::unique_ptr<atyp const, sddc> obj) { return "pass_udcp
 // Helpers for testing.
 std::string get_mtxt(atyp const &obj) { return obj.mtxt; }
 std::unique_ptr<atyp> unique_ptr_roundtrip(std::unique_ptr<atyp> obj) { return obj; }
+struct SharedPtrStash {
+    std::vector<std::shared_ptr<const atyp>> stash;
+    void Add(std::shared_ptr<const atyp> 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<SharedPtrStash>(m, "SharedPtrStash")
+        .def(py::init<>())
+        .def("Add", &SharedPtrStash::Add, py::arg("obj"));
 
     m.def("py_type_handle_of_atyp", []() {
         return py::type::handle_of<atyp>(); // Exercises static_cast in this function.

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")