From 10cc0e2094ede71f34b4324f0de4b1a303cc4815 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang Date: Wed, 16 Mar 2022 12:16:01 -0700 Subject: [PATCH 01/17] Fixes issue --- .../detail/smart_holder_type_casters.h | 3 +- tests/test_class_sh_void_ptr_capsule.cpp | 135 ++++++++---------- tests/test_class_sh_void_ptr_capsule.py | 71 ++++++++- 3 files changed, 122 insertions(+), 87 deletions(-) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index d4819793cc..f20ff28462 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -292,7 +292,8 @@ class modified_type_caster_generic_load_impl { loaded_v_h = value_and_holder(); return true; } - if (convert && cpptype && try_as_void_ptr_capsule(src)) { + const auto &bases = all_type_info(srctype); + if (convert && cpptype && 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 8083a74412..fbe59e5361 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -7,53 +7,31 @@ 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; - - bool capsule_generated = false; - virtual int get() const { return 100; } -}; +struct Valid { }; -struct Valid : public HelperBase { - int get() const override { return 101; } +struct NoConversion { }; - 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 NoCapsuleReturned { }; -struct NoConversion : public HelperBase { - int get() const override { return 102; } -}; +struct AsAnotherObject { }; -struct NoCapsuleReturned : public HelperBase { - int get() const override { return 103; } +py::object create_void_ptr_capsule(py::object obj, std::string class_name) { + void *vptr = static_cast(obj.ptr()); + // We assume vptr out lives the capsule, so we use nullptr for the + // destructor. + return pybind11::reinterpret_steal( + PyCapsule_New(vptr, class_name.c_str(), nullptr)); +} - PyObject *as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned() { - capsule_generated = true; - Py_XINCREF(Py_None); - return Py_None; - } -}; +int get_from_valid_capsule(const Valid *c) { return 1; } -struct AsAnotherObject : public HelperBase { - int get() const override { return 104; } +int get_from_shared_ptr_valid_capsule(const std::shared_ptr &c) { return 2; } - 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_unique_ptr_valid_capsule(std::unique_ptr c) { return 3; } + +int get_from_no_conversion_capsule(const NoConversion *c) { return 4; } + +int get_from_no_capsule_returned(const NoCapsuleReturned *c) { return 5; } // https://github.com/pybind/pybind11/issues/3788 struct TypeWithGetattr { @@ -61,68 +39,67 @@ struct TypeWithGetattr { 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(); } +// https://github.com/pybind/pybind11/issues/3804 +struct Base1 {int a1{};}; +struct Base2 {int a2{};}; -int get_from_unique_ptr_valid_capsule(std::unique_ptr c) { return c->get(); } +struct Base12 : Base1, Base2 { + virtual ~Base12() = default; + int foo() const { return 0; } +}; -int get_from_no_conversion_capsule(const NoConversion *c) { return c->get(); } +struct Derived1 : Base12 { + int bar() const { return 1; } +}; -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_void_ptr_capsule", &create_void_ptr_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&, 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 20193b4871..212f8ebd8c 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -3,26 +3,71 @@ from pybind11_tests import class_sh_void_ptr_capsule as m +class Valid: + + capsule_generated = False + + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): + self.capsule_generated = True + return m.create_void_ptr_capsule( + self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid") + + +class NoConversion: + + capsule_generated = False + + +class NoCapsuleReturned: + + capsule_generated = False + + def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(self): + return + + +class AsAnotherObject: + + capsule_generated = False + + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): + self.capsule_generated = True + return m.create_void_ptr_capsule( + self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid") + + @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) as excinfo: + 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 +82,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' From d107a06dcbb75bf480986e417f2537dd4e951f9b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Mar 2022 19:23:22 +0000 Subject: [PATCH 02/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.cpp | 28 ++++++++--------- tests/test_class_sh_void_ptr_capsule.py | 38 +++++++++++++----------- 2 files changed, 34 insertions(+), 32 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index fbe59e5361..91c0bbfbf5 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -7,13 +7,13 @@ namespace pybind11_tests { namespace class_sh_void_ptr_capsule { -struct Valid { }; +struct Valid {}; -struct NoConversion { }; +struct NoConversion {}; -struct NoCapsuleReturned { }; +struct NoCapsuleReturned {}; -struct AsAnotherObject { }; +struct AsAnotherObject {}; py::object create_void_ptr_capsule(py::object obj, std::string class_name) { void *vptr = static_cast(obj.ptr()); @@ -40,8 +40,12 @@ struct TypeWithGetattr { }; // https://github.com/pybind/pybind11/issues/3804 -struct Base1 {int a1{};}; -struct Base2 {int a2{};}; +struct Base1 { + int a1{}; +}; +struct Base2 { + int a2{}; +}; struct Base12 : Base1, Base2 { virtual ~Base12() = default; @@ -91,15 +95,9 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { py::classh(m, "Base12") .def(py::init<>()) .def("foo", &Base12::foo) - .def("__getattr__", [](Base12&, std::string key) { - return "Base GetAttr: " + key; - }); + .def("__getattr__", [](Base12 &, std::string key) { return "Base GetAttr: " + key; }); - py::classh(m, "Derived1") - .def(py::init<>()) - .def("bar", &Derived1::bar); + py::classh(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar); - py::classh(m, "Derived2") - .def(py::init<>()) - .def("bar", &Derived2::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 212f8ebd8c..1a1e3fece0 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -5,35 +5,37 @@ class Valid: - capsule_generated = False + capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): - self.capsule_generated = True - return m.create_void_ptr_capsule( - self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid") + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): + self.capsule_generated = True + return m.create_void_ptr_capsule( + self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid" + ) class NoConversion: - capsule_generated = False + capsule_generated = False class NoCapsuleReturned: - capsule_generated = False + capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(self): - return + def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(self): + return class AsAnotherObject: - capsule_generated = False + capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): - self.capsule_generated = True - return m.create_void_ptr_capsule( - self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid") + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): + self.capsule_generated = True + return m.create_void_ptr_capsule( + self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid" + ) @pytest.mark.parametrize( @@ -56,7 +58,9 @@ def test_valid_as_void_ptr_capsule_function(ctor, caller, expected, capsule_gene (NoCapsuleReturned, m.get_from_no_capsule_returned, 3, False), ], ) -def test_invalid_as_void_ptr_capsule_function(ctor, caller, expected, capsule_generated): +def test_invalid_as_void_ptr_capsule_function( + ctor, caller, expected, capsule_generated +): obj = ctor() with pytest.raises(TypeError) as excinfo: caller(obj) @@ -88,9 +92,9 @@ def test_multiple_inheritance_getattr(): d1 = m.Derived1() assert d1.foo() == 0 assert d1.bar() == 1 - assert d1.prop1 == 'Base GetAttr: prop1' + assert d1.prop1 == "Base GetAttr: prop1" d2 = m.Derived2() assert d2.foo() == 0 assert d2.bar() == 2 - assert d2.prop2 == 'Base GetAttr: prop2' + assert d2.prop2 == "Base GetAttr: prop2" From 50bd80a188eb571d5bd50fe84df474dea2a486ad Mon Sep 17 00:00:00 2001 From: Xiaofei Wang Date: Wed, 16 Mar 2022 12:27:57 -0700 Subject: [PATCH 03/17] Fix lint error --- tests/test_class_sh_void_ptr_capsule.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index 1a1e3fece0..1b33c9b82c 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -7,7 +7,7 @@ class Valid: capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 self.capsule_generated = True return m.create_void_ptr_capsule( self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid" @@ -23,7 +23,7 @@ class NoCapsuleReturned: capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(self): + def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(self): # noqa: N802 return @@ -31,7 +31,7 @@ class AsAnotherObject: capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): + def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 self.capsule_generated = True return m.create_void_ptr_capsule( self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid" @@ -45,7 +45,7 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): (AsAnotherObject, m.get_from_valid_capsule, 1, True), ], ) -def test_valid_as_void_ptr_capsule_function(ctor, caller, expected, capsule_generated): +def test_valid_as_void_ptr_capsule_function(ctor, caller, expected, capsule_generated): # noqa: N802 obj = ctor() assert caller(obj) == expected assert obj.capsule_generated == capsule_generated @@ -62,7 +62,7 @@ def test_invalid_as_void_ptr_capsule_function( ctor, caller, expected, capsule_generated ): obj = ctor() - with pytest.raises(TypeError) as excinfo: + with pytest.raises(TypeError): caller(obj) assert obj.capsule_generated == capsule_generated From 26aa3a7cd1252de7d94a82d038548f8d97e0b9b5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Mar 2022 19:28:42 +0000 Subject: [PATCH 04/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index 1b33c9b82c..a8328e064c 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -23,7 +23,9 @@ class NoCapsuleReturned: capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned(self): # noqa: N802 + def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( + self, + ): return @@ -45,7 +47,9 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 (AsAnotherObject, m.get_from_valid_capsule, 1, True), ], ) -def test_valid_as_void_ptr_capsule_function(ctor, caller, expected, capsule_generated): # noqa: N802 +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 From febb4f2bec0447701cd17507adc67f9070940781 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang Date: Wed, 16 Mar 2022 12:30:00 -0700 Subject: [PATCH 05/17] Fix flake8 --- tests/test_class_sh_void_ptr_capsule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index a8328e064c..d8f51dad4d 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -23,7 +23,7 @@ class NoCapsuleReturned: capsule_generated = False - def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( + def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( # noqa: N802 self, ): return From ca68831fc459569be74e167d8676b91812d0e865 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Mar 2022 19:30:45 +0000 Subject: [PATCH 06/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index d8f51dad4d..71f2a6ee48 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -47,9 +47,7 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 (AsAnotherObject, m.get_from_valid_capsule, 1, True), ], ) -def test_valid_as_void_ptr_capsule_function( - 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 From ff6f41ee510c45c1749b8bd063da51ca3d2ad76c Mon Sep 17 00:00:00 2001 From: Xiaofei Wang Date: Wed, 16 Mar 2022 13:11:43 -0700 Subject: [PATCH 07/17] Fix test --- include/pybind11/detail/smart_holder_type_casters.h | 8 +++++--- tests/test_class_sh_void_ptr_capsule.cpp | 10 +++++----- tests/test_class_sh_void_ptr_capsule.py | 12 ++++++++---- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/include/pybind11/detail/smart_holder_type_casters.h b/include/pybind11/detail/smart_holder_type_casters.h index f20ff28462..4773abf787 100644 --- a/include/pybind11/detail/smart_holder_type_casters.h +++ b/include/pybind11/detail/smart_holder_type_casters.h @@ -292,9 +292,11 @@ class modified_type_caster_generic_load_impl { loaded_v_h = value_and_holder(); return true; } - const auto &bases = all_type_info(srctype); - if (convert && cpptype && bases.empty() && 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 91c0bbfbf5..8bfa181505 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -23,15 +23,15 @@ py::object create_void_ptr_capsule(py::object obj, std::string class_name) { PyCapsule_New(vptr, class_name.c_str(), nullptr)); } -int get_from_valid_capsule(const Valid *c) { return 1; } +int get_from_valid_capsule(const Valid *) { return 1; } -int get_from_shared_ptr_valid_capsule(const std::shared_ptr &c) { return 2; } +int get_from_shared_ptr_valid_capsule(std::shared_ptr) { return 2; } -int get_from_unique_ptr_valid_capsule(std::unique_ptr c) { return 3; } +int get_from_unique_ptr_valid_capsule(std::unique_ptr) { return 3; } -int get_from_no_conversion_capsule(const NoConversion *c) { return 4; } +int get_from_no_conversion_capsule(const NoConversion *) { return 4; } -int get_from_no_capsule_returned(const NoCapsuleReturned *c) { return 5; } +int get_from_no_capsule_returned(const NoCapsuleReturned *) { return 5; } // https://github.com/pybind/pybind11/issues/3788 struct TypeWithGetattr { diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index 71f2a6ee48..9001dcde2c 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -5,7 +5,8 @@ class Valid: - capsule_generated = False + def __init__(self): + self.capsule_generated = False def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 self.capsule_generated = True @@ -16,12 +17,14 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 class NoConversion: - capsule_generated = False + def __init__(self): + self.capsule_generated = False class NoCapsuleReturned: - capsule_generated = False + def __init__(self): + self.capsule_generated = False def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( # noqa: N802 self, @@ -31,7 +34,8 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( # noqa: N802 class AsAnotherObject: - capsule_generated = False + def __init__(self): + self.capsule_generated = False def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 self.capsule_generated = True From 6157d791f5c8f08a53072cb0d6184630fd315aaa Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Mar 2022 20:12:25 +0000 Subject: [PATCH 08/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index 9001dcde2c..50a8d3ccd4 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -4,7 +4,6 @@ class Valid: - def __init__(self): self.capsule_generated = False @@ -16,13 +15,11 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 class NoConversion: - def __init__(self): self.capsule_generated = False class NoCapsuleReturned: - def __init__(self): self.capsule_generated = False @@ -33,7 +30,6 @@ def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( # noqa: N802 class AsAnotherObject: - def __init__(self): self.capsule_generated = False From 3a5baa2dcc219011dcdb951f22da88305e53c696 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang Date: Wed, 16 Mar 2022 15:27:10 -0700 Subject: [PATCH 09/17] Fix clang tidy --- tests/test_class_sh_void_ptr_capsule.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index 8bfa181505..02b78bf28a 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -15,7 +15,7 @@ struct NoCapsuleReturned {}; struct AsAnotherObject {}; -py::object create_void_ptr_capsule(py::object obj, std::string class_name) { +py::object create_void_ptr_capsule(py::object obj, const std::string& class_name) { void *vptr = static_cast(obj.ptr()); // We assume vptr out lives the capsule, so we use nullptr for the // destructor. @@ -48,7 +48,6 @@ struct Base2 { }; struct Base12 : Base1, Base2 { - virtual ~Base12() = default; int foo() const { return 0; } }; From c714d58d1988f55f34ecc13d63391078d10a6670 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Mar 2022 22:28:01 +0000 Subject: [PATCH 10/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index 02b78bf28a..e942f5710b 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -15,7 +15,7 @@ struct NoCapsuleReturned {}; struct AsAnotherObject {}; -py::object create_void_ptr_capsule(py::object obj, const std::string& class_name) { +py::object create_void_ptr_capsule(py::object obj, const std::string &class_name) { void *vptr = static_cast(obj.ptr()); // We assume vptr out lives the capsule, so we use nullptr for the // destructor. From cf51ccd9da9125c7c56fa80d97a57edeb448103e Mon Sep 17 00:00:00 2001 From: Xiaofei Wang Date: Wed, 16 Mar 2022 16:24:27 -0700 Subject: [PATCH 11/17] Fix again --- tests/test_class_sh_void_ptr_capsule.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index e942f5710b..d75fd57de4 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -25,7 +25,7 @@ py::object create_void_ptr_capsule(py::object obj, const std::string &class_name int get_from_valid_capsule(const Valid *) { return 1; } -int get_from_shared_ptr_valid_capsule(std::shared_ptr) { return 2; } +int get_from_shared_ptr_valid_capsule(const std::shared_ptr &) { return 2; } int get_from_unique_ptr_valid_capsule(std::unique_ptr) { return 3; } @@ -94,7 +94,7 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { py::classh(m, "Base12") .def(py::init<>()) .def("foo", &Base12::foo) - .def("__getattr__", [](Base12 &, std::string key) { return "Base GetAttr: " + key; }); + .def("__getattr__", [](Base12 &, const std::string &key) { return "Base GetAttr: " + key; }); py::classh(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar); From 749d3cc319d3a2083bdad78426342e81a9ba645e Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 16 Mar 2022 23:25:16 +0000 Subject: [PATCH 12/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index d75fd57de4..ec68192203 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -94,7 +94,8 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { py::classh(m, "Base12") .def(py::init<>()) .def("foo", &Base12::foo) - .def("__getattr__", [](Base12 &, const std::string &key) { return "Base GetAttr: " + key; }); + .def("__getattr__", + [](Base12 &, const std::string &key) { return "Base GetAttr: " + key; }); py::classh(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar); From 79cedf944af23df9800336684a359122fe219009 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Thu, 17 Mar 2022 13:09:51 -0700 Subject: [PATCH 13/17] Fix test --- tests/test_class_sh_void_ptr_capsule.cpp | 10 +++------- tests/test_class_sh_void_ptr_capsule.py | 8 ++------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index ec68192203..4860c8b2e8 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -15,12 +15,8 @@ struct NoCapsuleReturned {}; struct AsAnotherObject {}; -py::object create_void_ptr_capsule(py::object obj, const std::string &class_name) { - void *vptr = static_cast(obj.ptr()); - // We assume vptr out lives the capsule, so we use nullptr for the - // destructor. - return pybind11::reinterpret_steal( - PyCapsule_New(vptr, class_name.c_str(), nullptr)); +py::handle create_test_void_ptr_capsule() { + return pybind11::capsule((void *) 12345, "int").release(); } int get_from_valid_capsule(const Valid *) { return 1; } @@ -80,7 +76,7 @@ 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); - m.def("create_void_ptr_capsule", &create_void_ptr_capsule); + m.def("create_test_void_ptr_capsule", &create_test_void_ptr_capsule); py::classh(m, "TypeWithGetattr") .def(py::init<>()) diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index 50a8d3ccd4..a171056370 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -9,9 +9,7 @@ def __init__(self): def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 self.capsule_generated = True - return m.create_void_ptr_capsule( - self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid" - ) + return m.create_test_void_ptr_capsule() class NoConversion: @@ -35,9 +33,7 @@ def __init__(self): def as_pybind11_tests_class_sh_void_ptr_capsule_Valid(self): # noqa: N802 self.capsule_generated = True - return m.create_void_ptr_capsule( - self, "::pybind11_tests::class_sh_void_ptr_capsule::Valid" - ) + return m.create_test_void_ptr_capsule() @pytest.mark.parametrize( From d8daf836ac7b94e93c72eed30bb1a0885bcbefe9 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Thu, 17 Mar 2022 13:16:29 -0700 Subject: [PATCH 14/17] Add comments --- tests/test_class_sh_void_ptr_capsule.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index 4860c8b2e8..03c461997c 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -15,6 +15,8 @@ struct NoCapsuleReturned {}; struct AsAnotherObject {}; +// Create a void pointer capsule for test. The encapsulated object does not +// matter for this test case. py::handle create_test_void_ptr_capsule() { return pybind11::capsule((void *) 12345, "int").release(); } From 84fb7b4f8c9599150e4ef3d085f3405626fe6df2 Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Thu, 17 Mar 2022 16:06:36 -0700 Subject: [PATCH 15/17] Try fix Valgrind --- tests/test_class_sh_void_ptr_capsule.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index 03c461997c..9f56301ac8 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -17,9 +17,7 @@ struct AsAnotherObject {}; // Create a void pointer capsule for test. The encapsulated object does not // matter for this test case. -py::handle create_test_void_ptr_capsule() { - return pybind11::capsule((void *) 12345, "int").release(); -} +PyObject *create_test_void_ptr_capsule() { return PyCapsule_New((void *) 12345, "int", nullptr); } int get_from_valid_capsule(const Valid *) { return 1; } @@ -78,7 +76,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); - m.def("create_test_void_ptr_capsule", &create_test_void_ptr_capsule); + 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<>()) From 06e56581ce5df822361cf24923402f45f581025e Mon Sep 17 00:00:00 2001 From: Xiaofei Wang <6218006+wangxf123456@users.noreply.github.com> Date: Fri, 18 Mar 2022 10:21:57 -0700 Subject: [PATCH 16/17] Resolve comments --- tests/test_class_sh_void_ptr_capsule.cpp | 11 ++++++++--- tests/test_class_sh_void_ptr_capsule.py | 2 +- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index 9f56301ac8..e46e95183b 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -17,7 +17,12 @@ struct AsAnotherObject {}; // Create a void pointer capsule for test. The encapsulated object does not // matter for this test case. -PyObject *create_test_void_ptr_capsule() { return PyCapsule_New((void *) 12345, "int", nullptr); } +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); +} int get_from_valid_capsule(const Valid *) { return 1; } @@ -77,8 +82,8 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { 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); + PyObject *capsule = create_test_void_ptr_capsule(); + return pybind11::reinterpret_steal(capsule); }); py::classh(m, "TypeWithGetattr") diff --git a/tests/test_class_sh_void_ptr_capsule.py b/tests/test_class_sh_void_ptr_capsule.py index a171056370..04ba533fdd 100644 --- a/tests/test_class_sh_void_ptr_capsule.py +++ b/tests/test_class_sh_void_ptr_capsule.py @@ -24,7 +24,7 @@ def __init__(self): def as_pybind11_tests_class_sh_void_ptr_capsule_NoCapsuleReturned( # noqa: N802 self, ): - return + pass class AsAnotherObject: From 099d1f033b143aeefccd2f0b63c6c703ece64743 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 18 Mar 2022 17:22:44 +0000 Subject: [PATCH 17/17] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/test_class_sh_void_ptr_capsule.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/test_class_sh_void_ptr_capsule.cpp b/tests/test_class_sh_void_ptr_capsule.cpp index e46e95183b..45b8d2f642 100644 --- a/tests/test_class_sh_void_ptr_capsule.cpp +++ b/tests/test_class_sh_void_ptr_capsule.cpp @@ -17,11 +17,9 @@ struct AsAnotherObject {}; // Create a void pointer capsule for test. The encapsulated object does not // matter for this test case. -PyObject* create_test_void_ptr_capsule() { +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); + return PyCapsule_New(static_cast(&just_to_have_something_to_point_to), "int", nullptr); } int get_from_valid_capsule(const Valid *) { return 1; } @@ -82,8 +80,8 @@ TEST_SUBMODULE(class_sh_void_ptr_capsule, m) { 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); + PyObject *capsule = create_test_void_ptr_capsule(); + return pybind11::reinterpret_steal(capsule); }); py::classh(m, "TypeWithGetattr")