Skip to content

Commit 5330333

Browse files
committed
Fix #5989: use dynamic_pointer_cast for virtual inheritance in esft downcast
Replace the unconditional static_pointer_cast in set_via_shared_from_this with a SFINAE-dispatched esft_downcast helper that falls back to dynamic_pointer_cast when static_cast through a virtual base is ill-formed. Also add a workaround in the test binding (.def("name") on SftVirtDerived2) for a separate pre-existing issue with inherited method dispatch through virtual bases. Made-with: Cursor
1 parent e446296 commit 5330333

2 files changed

Lines changed: 23 additions & 1 deletion

File tree

include/pybind11/detail/holder_caster_foreign_helpers.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,31 @@ struct holder_caster_foreign_helpers {
3131
PyObject *o;
3232
};
3333

34+
// Downcast shared_ptr from the enable_shared_from_this base to the target type.
35+
// SFINAE probe: use static_pointer_cast when the static downcast is valid (common case),
36+
// fall back to dynamic_pointer_cast when it isn't (virtual inheritance — issue #5989).
37+
// We can't use dynamic_pointer_cast unconditionally because it requires polymorphic types;
38+
// we can't use is_polymorphic to choose because that's orthogonal to virtual inheritance.
39+
// (The implementation uses the "tag dispatch via overload priority" trick.)
40+
template <typename type, typename esft_base>
41+
static auto esft_downcast(const std::shared_ptr<esft_base> &existing, int /*preferred*/)
42+
-> decltype(static_cast<type *>(std::declval<esft_base *>()), std::shared_ptr<type>()) {
43+
return std::static_pointer_cast<type>(existing);
44+
}
45+
46+
template <typename type, typename esft_base>
47+
static std::shared_ptr<type> esft_downcast(const std::shared_ptr<esft_base> &existing,
48+
... /*fallback*/) {
49+
return std::dynamic_pointer_cast<type>(existing);
50+
}
51+
3452
template <typename type>
3553
static auto set_via_shared_from_this(type *value, std::shared_ptr<type> *holder_out)
3654
-> decltype(value->shared_from_this(), bool()) {
3755
// object derives from enable_shared_from_this;
3856
// try to reuse an existing shared_ptr if one is known
3957
if (auto existing = try_get_shared_from_this(value)) {
40-
*holder_out = std::static_pointer_cast<type>(existing);
58+
*holder_out = esft_downcast<type>(existing, 0);
4159
return true;
4260
}
4361
return false;

tests/test_smart_ptr.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -553,6 +553,10 @@ TEST_SUBMODULE(smart_ptr, m) {
553553
py::class_<SftVirtDerived2, SftVirtDerived, std::shared_ptr<SftVirtDerived2>>(
554554
m, "SftVirtDerived2")
555555
.def(py::init<>(&SftVirtDerived2::create))
556+
// TODO: Remove this once inherited methods work through virtual bases.
557+
// Without it, d2.name() segfaults because pybind11 uses an incorrect
558+
// pointer offset when dispatching through the virtual inheritance chain.
559+
.def("name", &SftVirtDerived2::name)
556560
.def("call_name", &SftVirtDerived2::call_name, py::arg("d2"));
557561

558562
// test_move_only_holder

0 commit comments

Comments
 (0)