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)

This commit is contained in:
Ralf W. Grosse-Kunstleve 2021-03-03 05:08:47 -08:00 committed by GitHub
parent 3a336a2047
commit 6a7e9f42fe
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 46 additions and 17 deletions

View File

@ -104,7 +104,7 @@ struct smart_holder {
bool is_populated : 1; bool is_populated : 1;
// Design choice: smart_holder is movable but not copyable. // 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(const smart_holder &) = delete;
smart_holder &operator=(smart_holder &&) = default; smart_holder &operator=(smart_holder &&) = default;
smart_holder &operator=(const smart_holder &) = delete; smart_holder &operator=(const smart_holder &) = delete;
@ -124,8 +124,8 @@ struct smart_holder {
template <typename T> template <typename T>
static void ensure_pointee_is_destructible(const char *context) { static void ensure_pointee_is_destructible(const char *context) {
if (!std::is_destructible<T>::value) 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 { 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 { void ensure_vptr_is_using_builtin_delete(const char *context) const {
if (vptr_is_external_shared_ptr) { 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) { 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) { 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); const std::type_info *rtti_requested = &typeid(D);
if (!rtti_uqp_del) { if (!rtti_uqp_del) {
if (!is_std_default_delete<T>(*rtti_requested)) { 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); ensure_vptr_is_using_builtin_delete(context);
} else if (!(*rtti_requested == *rtti_uqp_del)) { } 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 { void ensure_has_pointee(const char *context) const {
if (!has_pointee()) { 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 { void ensure_use_count_1(const char *context) const {
if (vptr.get() == nullptr) { 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 // In multithreaded environments accessing use_count can lead to
// race conditions, but in the context of Python it is a bug (elsewhere) // race conditions, but in the context of Python it is a bug (elsewhere)
@ -180,8 +180,8 @@ struct smart_holder {
// is reached. // is reached.
// SMART_HOLDER_WIP: IMPROVABLE: assert(GIL is held). // SMART_HOLDER_WIP: IMPROVABLE: assert(GIL is held).
if (vptr.use_count() != 1) { 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
+ ")."); + ").");
} }
} }

View File

@ -4,6 +4,7 @@
#include <memory> #include <memory>
#include <string> #include <string>
#include <vector>
namespace pybind11_tests { namespace pybind11_tests {
namespace class_sh_basic { 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. // Helpers for testing.
std::string get_mtxt(atyp const &obj) { return obj.mtxt; } std::string get_mtxt(atyp const &obj) { return obj.mtxt; }
std::unique_ptr<atyp> unique_ptr_roundtrip(std::unique_ptr<atyp> obj) { return obj; } 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 class_sh_basic
} // namespace pybind11_tests } // namespace pybind11_tests
PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_basic::atyp) 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 pybind11_tests {
namespace class_sh_basic { namespace class_sh_basic {
@ -110,6 +116,9 @@ TEST_SUBMODULE(class_sh_basic, m) {
// These require selected functions above to work first, as indicated: // These require selected functions above to work first, as indicated:
m.def("get_mtxt", get_mtxt); // pass_cref m.def("get_mtxt", get_mtxt); // pass_cref
m.def("unique_ptr_roundtrip", unique_ptr_roundtrip); // pass_uqmp, rtrn_uqmp 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", []() { m.def("py_type_handle_of_atyp", []() {
return py::type::handle_of<atyp>(); // Exercises static_cast in this function. return py::type::handle_of<atyp>(); // Exercises static_cast in this function.

View File

@ -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): def test_unique_ptr_roundtrip(num_round_trips=1000):
# Multiple roundtrips to stress-test instance registration/deregistration. # Multiple roundtrips to stress-test instance registration/deregistration.
recycled = m.atyp("passenger") recycled = m.atyp("passenger")