diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index afec27bd8a..d4819793cc 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -200,9 +200,6 @@ class modified_type_caster_generic_load_impl { if (!src) { return false; } - if (cpptype && try_as_void_ptr_capsule(src)) { - return true; - } if (!typeinfo) { return try_load_foreign_module_local(src); } @@ -295,7 +292,9 @@ class modified_type_caster_generic_load_impl { loaded_v_h = value_and_holder(); return true; } - + if (convert && cpptype && try_as_void_ptr_capsule(src)) { + return true; + } return false; } diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index f6597d568f..8083a74412 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -7,17 +7,7 @@ namespace pybind11_tests { namespace class_sh_void_ptr_capsule { -// Without the helper we will run into a type_caster::load recursion. -// This is because whenever the type_caster::load is called, it checks -// whether the object defines an `as_` method that returns the void pointer -// capsule. If yes, it calls the method. But in the following testcases, those -// `as_` methods are defined with pybind11, which implicitly takes the object -// itself as the first parameter. Therefore calling those methods causes loading -// the object again, which causes infinite recursion. -// This test is unusual in the sense that the void pointer capsules are meant to -// be provided by objects wrapped with systems other than pybind11 -// (i.e. having to avoid the recursion is an artificial problem, not the norm). -// Conveniently, the helper also serves to keep track of `capsule_generated`. +// Conveniently, the helper serves to keep track of `capsule_generated`. struct HelperBase { HelperBase() = default; HelperBase(const HelperBase &) = delete; @@ -65,6 +55,12 @@ struct AsAnotherObject : public HelperBase { } }; +// https://github.com/pybind/pybind11/issues/3788 +struct TypeWithGetattr { + TypeWithGetattr() = default; + int get_42() const { return 42; } +}; + int get_from_valid_capsule(const Valid *c) { return c->get(); } int get_from_shared_ptr_valid_capsule(const std::shared_ptr &c) { return c->get(); } @@ -83,6 +79,7 @@ PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Va PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::NoConversion) PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::NoCapsuleReturned) PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::AsAnotherObject) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::TypeWithGetattr) TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { using namespace pybind11_tests::class_sh_void_ptr_capsule; @@ -94,10 +91,8 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { py::classh(m, "Valid") .def(py::init<>()) - .def("as_pybind11_tests_class_sh_void_ptr_capsule_Valid", [](HelperBase *self) { - auto *obj = dynamic_cast(self); - assert(obj != nullptr); - PyObject *capsule = obj->as_pybind11_tests_class_sh_void_ptr_capsule_Valid(); + .def("as_pybind11_tests_class_sh_void_ptr_capsule_Valid", [](Valid &self) { + PyObject *capsule = self.as_pybind11_tests_class_sh_void_ptr_capsule_Valid(); return pybind11::reinterpret_steal(capsule); }); @@ -106,20 +101,16 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { py::classh(m, "NoCapsuleReturned") .def(py::init<>()) .def("as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned", - [](HelperBase *self) { - auto *obj = dynamic_cast(self); - assert(obj != nullptr); + [](NoCapsuleReturned &self) { PyObject *capsule - = obj->as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(); + = self.as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(); return pybind11::reinterpret_steal(capsule); }); py::classh(m, "AsAnotherObject") .def(py::init<>()) - .def("as_pybind11_tests_class_sh_void_ptr_capsule_Valid", [](HelperBase *self) { - auto *obj = dynamic_cast(self); - assert(obj != nullptr); - PyObject *capsule = obj->as_pybind11_tests_class_sh_void_ptr_capsule_Valid(); + .def("as_pybind11_tests_class_sh_void_ptr_capsule_Valid", [](AsAnotherObject &self) { + PyObject *capsule = self.as_pybind11_tests_class_sh_void_ptr_capsule_Valid(); return pybind11::reinterpret_steal(capsule); }); @@ -128,4 +119,10 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { m.def("get_from_unique_ptr_valid_capsule", &get_from_unique_ptr_valid_capsule); m.def("get_from_no_conversion_capsule", &get_from_no_conversion_capsule); m.def("get_from_no_capsule_returned", &get_from_no_capsule_returned); + + py::classh(m, "TypeWithGetattr") + .def(py::init<>()) + .def("get_42", &TypeWithGetattr::get_42) + .def("__getattr__", + [](TypeWithGetattr &, const std::string &key) { return "GetAttr: " + key; }); } diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index d5bf449614..20193b4871 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -6,9 +6,9 @@ @pytest.mark.parametrize( "ctor, caller, expected, capsule_generated", [ - (m.Valid, m.get_from_valid_capsule, 101, True), + (m.Valid, m.get_from_valid_capsule, 101, False), (m.NoConversion, m.get_from_no_conversion_capsule, 102, False), - (m.NoCapsuleReturned, m.get_from_no_capsule_returned, 103, True), + (m.NoCapsuleReturned, m.get_from_no_capsule_returned, 103, False), (m.AsAnotherObject, m.get_from_valid_capsule, 104, True), ], ) @@ -31,3 +31,9 @@ def test_as_void_ptr_capsule_unsupported(ctor, caller, pointer_type, capsule_gen caller(obj) assert pointer_type in str(excinfo.value) assert obj.capsule_generated == capsule_generated + + +def test_type_with_getattr(): + obj = m.TypeWithGetattr() + assert obj.get_42() == 42 + assert obj.something == "GetAttr: something"