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>

Author: wangxf123456

include/pybind11/detail/smart_holder_type_casters.h

@@ -292,8 +292,11 @@ 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;
+        if (convert && cpptype) {
+            const auto &bases = all_type_info(srctype);
+            if (bases.empty() && try_as_void_ptr_capsule(src)) {
+                return true;
+            }
         }
         return false;
     }

tests/test_class_sh_void_ptr_capsule.cpp

@@ -7,122 +7,99 @@
 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<void *>(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; }
+// 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<void *>(&just_to_have_something_to_point_to), "int", 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 *) { return 1; }
 
-struct AsAnotherObject : public HelperBase {
-    int get() const override { return 104; }
+int get_from_shared_ptr_valid_capsule(const std::shared_ptr<Valid> &) { return 2; }
 
-    PyObject *as_pybind11_tests_class_sh_void_ptr_capsule_Valid() {
-        void *vptr = dynamic_cast<void *>(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<Valid>) { return 3; }
+
+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 {
     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<Valid> &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<Valid> c) { return c->get(); }
+struct Base12 : Base1, Base2 {
+    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<HelperBase>(m, "HelperBase")
-        .def(py::init<>())
-        .def("get", &HelperBase::get)
-        .def_readonly("capsule_generated", &HelperBase::capsule_generated);
-
-    py::classh<Valid, HelperBase>(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<py::capsule>(capsule);
-        });
-
-    py::classh<NoConversion, HelperBase>(m, "NoConversion").def(py::init<>());
-
-    py::classh<NoCapsuleReturned, HelperBase>(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<py::capsule>(capsule);
-             });
-
-    py::classh<AsAnotherObject, HelperBase>(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<py::capsule>(capsule);
-        });
+    py::classh<Valid>(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<py::capsule>(capsule);
+    });
 
     py::classh<TypeWithGetattr>(m, "TypeWithGetattr")
         .def(py::init<>())
         .def("get_42", &TypeWithGetattr::get_42)
         .def("__getattr__",
              [](TypeWithGetattr &, const std::string &key) { return "GetAttr: " + key; });
+
+    py::classh<Base1>(m, "Base1");
+    py::classh<Base2>(m, "Base2");
+
+    py::classh<Base12, Base1, Base2>(m, "Base12")
+        .def(py::init<>())
+        .def("foo", &Base12::foo)
+        .def("__getattr__",
+             [](Base12 &, const std::string &key) { return "Base GetAttr: " + key; });
+
+    py::classh<Derived1, Base12>(m, "Derived1").def(py::init<>()).def("bar", &Derived1::bar);
+
+    py::classh<Derived2, Base12>(m, "Derived2").def(py::init<>()).def("bar", &Derived2::bar);
 }

tests/test_class_sh_void_ptr_capsule.py

@@ -3,26 +3,73 @@
 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"