Fix static_pointer_cast build failure with virtual inheritance in holder_caster_foreign_helpers.h#6014
Merged
oremanj merged 2 commits intopybind:masterfrom Mar 27, 2026
Conversation
…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
Collaborator
Author
|
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
|
3 tasks
static_pointer_cast build failure with virtual inheritance in holder_caster_foreign_helpers.h
oremanj
approved these changes
Mar 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a class uses virtual inheritance and its holder type is
shared_ptr, any method taking ashared_ptrof the derived type as an argument fails to compile. This is becauseset_via_shared_from_thisinholder_caster_foreign_helpers.hunconditionally usesstatic_pointer_castto downcast from theenable_shared_from_thisbase to the target type. With virtual inheritance,static_castfrom base to derived is ill-formed.The fix introduces a SFINAE-dispatched
esft_downcasthelper with two overloads (using the "tag dispatch via overload priority" trick):int): usesstatic_pointer_cast— selected whenstatic_cast<type*>(esft_base*)is well-formed (non-virtual inheritance). This is the common case, with zero behavioral change from before....): usesdynamic_pointer_cast— selected via SFINAE when thestatic_castis ill-formed (virtual inheritance).Why not simpler alternatives?
dynamic_pointer_castunconditionally: requires polymorphic types (at least one virtual function). Would break existing code that usesenable_shared_from_thiswith non-polymorphic type hierarchies.std::is_polymorphicdispatch: orthogonal to virtual inheritance. A type can be polymorphic without virtual inheritance (common case), so this would unnecessarily usedynamic_pointer_castfor all polymorphic types. There is no standardis_virtual_base_oftrait.Note on the test
The test binds
.def("name", &SftVirtDerived2::name)directly on the derived class rather than relying on inheritance fromSftVirtBase. 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 aTODOcomment 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:
cannot convert from pointer to base class 'SftVirtBase' to pointer to derived class 'SftVirtDerived2' via virtual base 'SftVirtDerived'error C2635: cannot convert a 'SftVirtBase*' to a 'SftVirtDerived2*'; conversion from a virtual base class is impliedEvery platform that compiles tests failed with the same error.
See: https://github.com/pybind/pybind11/actions/runs/23617176855?pr=6014