From 7405976aa471863ab9a845b5df206f37a45c248c Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Fri, 18 Mar 2022 11:08:20 -0700 Subject: [PATCH] Fixes issue #3801 (#3807) * Fixes issue * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix lint error * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix flake8 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix test * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix clang tidy * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix again * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix test * Add comments * Try fix Valgrind * Resolve comments * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../detail/smart_holder_type_casters.h | 7 +- tests/test_class_sh_void_ptr_capsule.cpp | 133 ++++++++---------- tests/test_class_sh_void_ptr_capsule.py | 73 +++++++++- 3 files changed, 126 insertions(+), 87 deletions(-) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index d4819793c..4773abf78 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -292,8 +292,11 @@ public: loaded_v_h = value_and_holder(); return true; } - if (convert && cpptype && try_as_void_ptr_capsule(src)) { - return true; + if (convert && cpptype) { + const auto &bases = all_type_info(srctype); + if (bases.empty() && 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 8083a7441..45b8d2f64 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -7,53 +7,30 @@ namespace pybind11_tests { namespace class_sh_void_ptr_capsule { -// Conveniently, the helper serves to keep track of `capsule_generated`. -struct HelperBase { - HelperBase() = default; - HelperBase(const HelperBase &) = delete; - virtual ~HelperBase() = default; +struct Valid {}; - bool capsule_generated = false; - virtual int get() const { return 100; } -}; +struct NoConversion {}; -struct Valid : public HelperBase { - int get() const override { return 101; } +struct NoCapsuleReturned {}; - PyObject *as_pybind11_tests_class_sh_void_ptr_capsule_Valid() { - void *vptr = dynamic_cast(this); - capsule_generated = true; - // We assume vptr out lives the capsule, so we use nullptr for the - // destructor. - return PyCapsule_New(vptr, "::pybind11_tests::class_sh_void_ptr_capsule::Valid", nullptr); - } -}; +struct AsAnotherObject {}; -struct NoConversion : public HelperBase { - int get() const override { return 102; } -}; +// Create a void pointer capsule for test. The encapsulated object does not +// matter for this test case. +PyObject *create_test_void_ptr_capsule() { + static int just_to_have_something_to_point_to = 0; + return PyCapsule_New(static_cast(&just_to_have_something_to_point_to), "int", nullptr); +} -struct NoCapsuleReturned : public HelperBase { - int get() const override { return 103; } +int get_from_valid_capsule(const Valid *) { return 1; } - PyObject *as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned() { - capsule_generated = true; - Py_XINCREF(Py_None); - return Py_None; - } -}; +int get_from_shared_ptr_valid_capsule(const std::shared_ptr &) { return 2; } -struct AsAnotherObject : public HelperBase { - int get() const override { return 104; } +int get_from_unique_ptr_valid_capsule(std::unique_ptr) { return 3; } - PyObject *as_pybind11_tests_class_sh_void_ptr_capsule_Valid() { - void *vptr = dynamic_cast(this); - capsule_generated = true; - // We assume vptr out lives the capsule, so we use nullptr for the - // destructor. - return PyCapsule_New(vptr, "::pybind11_tests::class_sh_void_ptr_capsule::Valid", nullptr); - } -}; +int get_from_no_conversion_capsule(const NoConversion *) { return 4; } + +int get_from_no_capsule_returned(const NoCapsuleReturned *) { return 5; } // https://github.com/pybind/pybind11/issues/3788 struct TypeWithGetattr { @@ -61,68 +38,68 @@ struct TypeWithGetattr { int get_42() const { return 42; } }; -int get_from_valid_capsule(const Valid *c) { return c->get(); } +// https://github.com/pybind/pybind11/issues/3804 +struct Base1 { + int a1{}; +}; +struct Base2 { + int a2{}; +}; -int get_from_shared_ptr_valid_capsule(const std::shared_ptr &c) { return c->get(); } +struct Base12 : Base1, Base2 { + int foo() const { return 0; } +}; -int get_from_unique_ptr_valid_capsule(std::unique_ptr c) { return c->get(); } +struct Derived1 : Base12 { + int bar() const { return 1; } +}; -int get_from_no_conversion_capsule(const NoConversion *c) { return c->get(); } - -int get_from_no_capsule_returned(const NoCapsuleReturned *c) { return c->get(); } +struct Derived2 : Base12 { + int bar() const { return 2; } +}; } // namespace class_sh_void_ptr_capsule } // namespace pybind11_tests -PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::HelperBase) PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Valid) -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) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Base1) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Base2) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Base12) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Derived1) +PYBIND11_SMART_HOLDER_TYPE_CASTERS(pybind11_tests::class_sh_void_ptr_capsule::Derived2) TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { using namespace pybind11_tests::class_sh_void_ptr_capsule; - py::classh(m, "HelperBase") - .def(py::init<>()) - .def("get", &HelperBase::get) - .def_readonly("capsule_generated", &HelperBase::capsule_generated); - - py::classh(m, "Valid") - .def(py::init<>()) - .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); - }); - - py::classh(m, "NoConversion").def(py::init<>()); - - py::classh(m, "NoCapsuleReturned") - .def(py::init<>()) - .def("as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned", - [](NoCapsuleReturned &self) { - PyObject *capsule - = 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", [](AsAnotherObject &self) { - PyObject *capsule = self.as_pybind11_tests_class_sh_void_ptr_capsule_Valid(); - return pybind11::reinterpret_steal(capsule); - }); + py::classh(m, "Valid"); m.def("get_from_valid_capsule", &get_from_valid_capsule); m.def("get_from_shared_ptr_valid_capsule", &get_from_shared_ptr_valid_capsule); 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); + m.def("create_test_void_ptr_capsule", []() { + PyObject *capsule = create_test_void_ptr_capsule(); + return pybind11::reinterpret_steal(capsule); + }); py::classh(m, "TypeWithGetattr") .def(py::init<>()) .def("get_42", &TypeWithGetattr::get_42) .def("__getattr__", [](TypeWithGetattr &, const std::string &key) { return "GetAttr: " + key; }); + + py::classh(m, "Base1"); + py::classh(m, "Base2"); + + py::classh(m, "Base12") + .def(py::init<>()) + .def("foo", &Base12::foo) + .def("__getattr__", + [](Base12 &, const std::string &key) { return "Base GetAttr: " + key; }); + + py::classh(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar); + + py::classh(m, "Derived2").def(py::init<>()).def("bar", &Derived2::bar); } diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index 20193b487..04ba533fd 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -3,26 +3,73 @@ import pytest from pybind11_tests import class_sh_void_ptr_capsule as m +class Valid: + def __init__(self): + self.capsule_generated = False + + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 + self.capsule_generated = True + return m.create_test_void_ptr_capsule() + + +class NoConversion: + def __init__(self): + self.capsule_generated = False + + +class NoCapsuleReturned: + def __init__(self): + self.capsule_generated = False + + def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( # noqa: N802 + self, + ): + pass + + +class AsAnotherObject: + def __init__(self): + self.capsule_generated = False + + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 + self.capsule_generated = True + return m.create_test_void_ptr_capsule() + + @pytest.mark.parametrize( "ctor, caller, expected, capsule_generated", [ - (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, False), - (m.AsAnotherObject, m.get_from_valid_capsule, 104, True), + (Valid, m.get_from_valid_capsule, 1, True), + (AsAnotherObject, m.get_from_valid_capsule, 1, True), ], ) -def test_as_void_ptr_capsule(ctor, caller, expected, capsule_generated): +def test_valid_as_void_ptr_capsule_function(ctor, caller, expected, capsule_generated): obj = ctor() assert caller(obj) == expected assert obj.capsule_generated == capsule_generated +@pytest.mark.parametrize( + "ctor, caller, expected, capsule_generated", + [ + (NoConversion, m.get_from_no_conversion_capsule, 2, False), + (NoCapsuleReturned, m.get_from_no_capsule_returned, 3, False), + ], +) +def test_invalid_as_void_ptr_capsule_function( + ctor, caller, expected, capsule_generated +): + obj = ctor() + with pytest.raises(TypeError): + caller(obj) + assert obj.capsule_generated == capsule_generated + + @pytest.mark.parametrize( "ctor, caller, pointer_type, capsule_generated", [ - (m.AsAnotherObject, m.get_from_shared_ptr_valid_capsule, "shared_ptr", True), - (m.AsAnotherObject, m.get_from_unique_ptr_valid_capsule, "unique_ptr", True), + (AsAnotherObject, m.get_from_shared_ptr_valid_capsule, "shared_ptr", True), + (AsAnotherObject, m.get_from_unique_ptr_valid_capsule, "unique_ptr", True), ], ) def test_as_void_ptr_capsule_unsupported(ctor, caller, pointer_type, capsule_generated): @@ -37,3 +84,15 @@ def test_type_with_getattr(): obj = m.TypeWithGetattr() assert obj.get_42() == 42 assert obj.something == "GetAttr: something" + + +def test_multiple_inheritance_getattr(): + d1 = m.Derived1() + assert d1.foo() == 0 + assert d1.bar() == 1 + assert d1.prop1 == "Base GetAttr: prop1" + + d2 = m.Derived2() + assert d2.foo() == 0 + assert d2.bar() == 2 + assert d2.prop2 == "Base GetAttr: prop2"