Skip to content

Fix static_pointer_cast build failure with virtual inheritance in holder_caster_foreign_helpers.h#6014

Merged
oremanj merged 2 commits intopybind:masterfrom
rwgk:static_pointer_cast_where_dynamic_pointer_cast_is_needed
Mar 27, 2026
Merged

Fix static_pointer_cast build failure with virtual inheritance in holder_caster_foreign_helpers.h#6014
oremanj merged 2 commits intopybind:masterfrom
rwgk:static_pointer_cast_where_dynamic_pointer_cast_is_needed

Conversation

@rwgk
Copy link
Collaborator

@rwgk rwgk commented Mar 26, 2026

When a class uses virtual inheritance and its holder type is shared_ptr, any method taking a shared_ptr of the derived type as an argument fails to compile. This is because set_via_shared_from_this in holder_caster_foreign_helpers.h unconditionally uses static_pointer_cast to downcast from the enable_shared_from_this base to the target type. With virtual inheritance, static_cast from base to derived is ill-formed.

The fix introduces a SFINAE-dispatched esft_downcast helper with two overloads (using the "tag dispatch via overload priority" trick):

  • Preferred (int): uses static_pointer_cast — selected when static_cast<type*>(esft_base*) is well-formed (non-virtual inheritance). This is the common case, with zero behavioral change from before.
  • Fallback (...): uses dynamic_pointer_cast — selected via SFINAE when the static_cast is ill-formed (virtual inheritance).

Why not simpler alternatives?

  • dynamic_pointer_cast unconditionally: requires polymorphic types (at least one virtual function). Would break existing code that uses enable_shared_from_this with non-polymorphic type hierarchies.
  • std::is_polymorphic dispatch: orthogonal to virtual inheritance. A type can be polymorphic without virtual inheritance (common case), so this would unnecessarily use dynamic_pointer_cast for all polymorphic types. There is no standard is_virtual_base_of trait.

Note on the test

The test binds .def("name", &SftVirtDerived2::name) directly on the derived class rather than relying on inheritance from SftVirtBase. This works around a separate pre-existing issue where pybind11's method dispatch through virtual inheritance chains uses incorrect pointer offsets, causing a segfault. That issue is unrelated to #5989 and is flagged with a TODO comment in the test code.

CI verification

The first commit (test only, no fix) was pushed to confirm the compilation error reproduces on all CI platforms:

  • GCC: cannot convert from pointer to base class 'SftVirtBase' to pointer to derived class 'SftVirtDerived2' via virtual base 'SftVirtDerived'
  • MSVC: error C2635: cannot convert a 'SftVirtBase*' to a 'SftVirtDerived2*'; conversion from a virtual base class is implied
  • Clang: same class of error

Every platform that compiles tests failed with the same error.

See: https://github.com/pybind/pybind11/actions/runs/23617176855?pr=6014

rwgk added 2 commits March 26, 2026 13:43
…irtual inheritance

When a class uses virtual inheritance and its holder type is shared_ptr,
passing a shared_ptr of the derived type as a method argument triggers
a compilation error because static_pointer_cast cannot downcast through
a virtual base (dynamic_pointer_cast is needed instead).

Made-with: Cursor
…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
@rwgk
Copy link
Collaborator Author

rwgk commented Mar 26, 2026

Summarizing the CI results for the new test, without a fix:

https://github.com/pybind/pybind11/actions/runs/23617176855?pr=6014

Every platform that compiles the tests fails with the exact same static_pointer_cast error from holder_caster_foreign_helpers.h — exactly as expected.

  • GCC: cannot convert from pointer to base class 'SftVirtBase' to pointer to derived class 'SftVirtDerived2' via virtual base 'SftVirtDerived'
  • MSVC: error C2635: cannot convert a 'SftVirtBase*' to a 'SftVirtDerived2*'; conversion from a virtual base class is implied

@rwgk rwgk requested a review from oremanj March 26, 2026 21:38
@rwgk rwgk changed the title WIP fixing issue #5989 Fix static_pointer_cast build failure with virtual inheritance in holder_caster_foreign_helpers.h Mar 26, 2026
@oremanj oremanj merged commit 3b62426 into pybind:master Mar 27, 2026
89 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Mar 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants